Mercurial > hg > graal-compiler

changeset 21083:71af9afd7ff6

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Test the GraphDecoder by encoding and decoding every graph after parsing; fix bugs found by that testing
author Christian Wimmer <christian.wimmer@oracle.com>
date Wed, 22 Apr 2015 13:05:36 -0700
parents 266eef1c29e0
children 814bd3dbc615
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotSuitesProvider.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GraphDecoder.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/LoadFieldNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/util/GraphUtil.java
diffstat 4 files changed, 45 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotSuitesProvider.java	Wed Apr 22 11:38:25 2015 -0700
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotSuitesProvider.java	Wed Apr 22 13:05:36 2015 -0700
@@ -31,6 +31,8 @@
 import com.oracle.graal.hotspot.phases.*;
 import com.oracle.graal.java.*;
 import com.oracle.graal.lir.phases.*;
+import com.oracle.graal.nodes.*;
+import com.oracle.graal.nodes.StructuredGraph.*;
 import com.oracle.graal.options.*;
 import com.oracle.graal.options.DerivedOptionValue.OptionSupplier;
 import com.oracle.graal.phases.*;
@@ -104,10 +106,34 @@
         PhaseSuite<HighTierContext> suite = new PhaseSuite<>();
         GraphBuilderConfiguration config = GraphBuilderConfiguration.getDefault(plugins);
         suite.appendPhase(new GraphBuilderPhase(config));
+        assert appendGraphEncoderTest(suite);
         return suite;
     }
 
     /**
+     * When assertions are enabled, we encode and decode every parsed graph, to ensure that the
+     * encoding and decoding process work correctly. The decoding performs canonicalization during
+     * decoding, so the decoded graph can be different than the encoded graph - we cannot check them
+     * for equality here. However, the encoder {@link GraphEncoder#verifyEncoding verifies the
+     * encoding itself}, i.e., performs a decoding without canoncialization and checks the graphs
+     * for equality.
+     */
+    private boolean appendGraphEncoderTest(PhaseSuite<HighTierContext> suite) {
+        suite.appendPhase(new BasePhase<HighTierContext>() {
+            @Override
+            protected void run(StructuredGraph graph, HighTierContext context) {
+                EncodedGraph encodedGraph = GraphEncoder.encodeSingleGraph(graph, runtime.getTarget().arch);
+
+                SimplifyingGraphDecoder graphDecoder = new SimplifyingGraphDecoder(context.getMetaAccess(), context.getConstantReflection(), context.getStampProvider(), !ImmutableCode.getValue(),
+                                runtime.getTarget().arch);
+                StructuredGraph targetGraph = new StructuredGraph(graph.method(), AllowAssumptions.YES);
+                graphDecoder.decode(targetGraph, encodedGraph);
+            }
+        });
+        return true;
+    }
+
+    /**
      * Modifies the {@link GraphBuilderConfiguration} to build extra
      * {@linkplain DebugInfoMode#Simple debug info} if the VM
      * {@linkplain CompilerToVM#shouldDebugNonSafepoints() requests} it.
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GraphDecoder.java	Wed Apr 22 11:38:25 2015 -0700
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GraphDecoder.java	Wed Apr 22 13:05:36 2015 -0700
@@ -303,7 +303,6 @@
              */
             FixedNode detectLoopsStart = startNode.predecessor() != null ? (FixedNode) startNode.predecessor() : startNode;
             cleanupGraph(methodScope, start);
-            Debug.dump(methodScope.graph, "Before loop detection");
             detectLoops(methodScope.graph, detectLoopsStart);
         }
     }
@@ -1020,7 +1019,6 @@
             }
         }
 
-        Debug.dump(currentGraph, "After loops detected");
         insertLoopEnds(currentGraph, startInstruction);
     }
 
@@ -1145,7 +1143,6 @@
     protected void cleanupGraph(MethodScope methodScope, Graph.Mark start) {
         assert verifyEdges(methodScope);
 
-        Debug.dump(methodScope.graph, "Before removing redundant merges");
         for (Node node : methodScope.graph.getNewNodes(start)) {
             if (node instanceof MergeNode) {
                 MergeNode mergeNode = (MergeNode) node;
@@ -1155,7 +1152,6 @@
             }
         }
 
-        Debug.dump(methodScope.graph, "Before removing redundant begins");
         for (Node node : methodScope.graph.getNewNodes(start)) {
             if (node instanceof BeginNode || node instanceof KillingBeginNode) {
                 if (!(node.predecessor() instanceof ControlSplitNode) && node.hasNoUsages()) {
@@ -1165,7 +1161,6 @@
             }
         }
 
-        Debug.dump(methodScope.graph, "Before removing unused non-fixed nodes");
         for (Node node : methodScope.graph.getNewNodes(start)) {
             if (!(node instanceof FixedNode) && node.hasNoUsages()) {
                 GraphUtil.killCFG(node);
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/LoadFieldNode.java	Wed Apr 22 11:38:25 2015 -0700
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/LoadFieldNode.java	Wed Apr 22 13:05:36 2015 -0700
@@ -70,9 +70,11 @@
             if (constant != null) {
                 return constant;
             }
-            PhiNode phi = asPhi(metaAccess, constantReflection, forObject);
-            if (phi != null) {
-                return phi;
+            if (tool.allUsagesAvailable()) {
+                PhiNode phi = asPhi(metaAccess, constantReflection, forObject);
+                if (phi != null) {
+                    return phi;
+                }
             }
         }
         if (!isStatic() && forObject.isNullConstant()) {
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/util/GraphUtil.java	Wed Apr 22 11:38:25 2015 -0700
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/util/GraphUtil.java	Wed Apr 22 13:05:36 2015 -0700
@@ -238,14 +238,28 @@
      * </pre>
      */
     public static void normalizeLoops(StructuredGraph graph) {
+        boolean loopRemoved = false;
         for (LoopBeginNode begin : graph.getNodes(LoopBeginNode.TYPE)) {
             if (begin.loopEnds().isEmpty()) {
                 assert begin.forwardEndCount() == 1;
                 graph.reduceDegenerateLoopBegin(begin);
+                loopRemoved = true;
             } else {
                 normalizeLoopBegin(begin);
             }
         }
+
+        if (loopRemoved) {
+            /*
+             * Removing a degenerated loop can make non-loop phi functions unnecessary. Therefore,
+             * we re-check all phi functions and remove redundant ones.
+             */
+            for (Node node : graph.getNodes()) {
+                if (node instanceof PhiNode) {
+                    checkRedundantPhi((PhiNode) node);
+                }
+            }
+        }
     }
 
     private static void normalizeLoopBegin(LoopBeginNode begin) {