changeset 11798:2fbb9fd55dde

made lowering recursive instead of iterative
author Doug Simon <doug.simon@oracle.com>
date Wed, 25 Sep 2013 21:49:39 +0200
parents 65dbed1fdf46
children 4c96ccce3772
files graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/MemoryScheduleTest.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotRuntime.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/nodes/WriteBarrier.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/CallSiteTargetNode.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ReflectionGetCallerClassNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GuardingPiNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/debug/DynamicCounterNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/CheckCastNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/ExceptionObjectNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/Lowerable.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/LoweringPhase.java graal/com.oracle.graal.replacements.amd64/src/com/oracle/graal/replacements/amd64/AMD64ConvertNode.java graal/com.oracle.graal.replacements.amd64/src/com/oracle/graal/replacements/amd64/AMD64ConvertSnippets.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/asserts/NeverInlineMacroNode.java graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/frame/FrameAccessNode.java graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/frame/FrameGetNode.java graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/frame/FrameSetNode.java
diffstat 20 files changed, 104 insertions(+), 39 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/MemoryScheduleTest.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/MemoryScheduleTest.java	Wed Sep 25 21:49:39 2013 +0200
@@ -447,7 +447,7 @@
     @Test
     public void testLoop4() {
         SchedulePhase schedule = getFinalSchedule("testLoop4Snippet", TestMode.WITHOUT_FRAMESTATES, MemoryScheduling.OPTIMAL);
-        assertReadWithinStartBlock(schedule, false);
+        assertReadWithinStartBlock(schedule, true);
         assertReadWithinReturnBlock(schedule, false);
     }
 
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotRuntime.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotRuntime.java	Wed Sep 25 21:49:39 2013 +0200
@@ -750,6 +750,7 @@
                     for (int lockDepth : commit.getLocks().get(objIndex)) {
                         MonitorEnterNode enter = graph.add(new MonitorEnterNode(allocations[objIndex], lockDepth));
                         graph.addBeforeFixed(commit, enter);
+                        enter.lower(tool);
                     }
                 }
                 for (Node usage : commit.usages().snapshot()) {
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/nodes/WriteBarrier.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/nodes/WriteBarrier.java	Wed Sep 25 21:49:39 2013 +0200
@@ -60,8 +60,8 @@
     }
 
     @Override
-    public void lower(LoweringTool generator) {
+    public void lower(LoweringTool tool) {
         assert graph().getGuardsStage() == StructuredGraph.GuardsStage.AFTER_FSA;
-        generator.getRuntime().lower(this, generator);
+        tool.getRuntime().lower(this, tool);
     }
 }
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/CallSiteTargetNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/CallSiteTargetNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -72,7 +72,9 @@
         if (target != null) {
             graph().replaceFixedWithFloating(this, target);
         } else {
-            graph().replaceFixedWithFixed(this, createInvoke());
+            InvokeNode invoke = createInvoke();
+            graph().replaceFixedWithFixed(this, invoke);
+            invoke.lower(tool);
         }
     }
 }
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ReflectionGetCallerClassNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ReflectionGetCallerClassNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -53,7 +53,9 @@
         if (callerClassNode != null) {
             graph().replaceFixedWithFloating(this, callerClassNode);
         } else {
-            graph().replaceFixedWithFixed(this, createInvoke());
+            InvokeNode invoke = createInvoke();
+            graph().replaceFixedWithFixed(this, invoke);
+            invoke.lower(tool);
         }
     }
 
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GuardingPiNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GuardingPiNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -95,13 +95,13 @@
     @Override
     public ValueNode canonical(CanonicalizerTool tool) {
         if (stamp() == StampFactory.illegal(object.kind())) {
-            // The condition always fails
+            // The guard always fails
             return graph().add(new DeoptimizeNode(action, reason));
         }
         if (condition instanceof LogicConstantNode) {
             LogicConstantNode c = (LogicConstantNode) condition;
             if (c.getValue() == negated) {
-                // The condition always fails
+                // The guard always fails
                 return graph().add(new DeoptimizeNode(action, reason));
             }
 
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/debug/DynamicCounterNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/debug/DynamicCounterNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -183,6 +183,8 @@
 
             graph().addBeforeFixed(this, load);
             graph().addBeforeFixed(this, store);
+            load.lower(tool);
+            store.lower(tool);
         }
         graph().removeFixed(this);
     }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/CheckCastNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/CheckCastNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -115,6 +115,7 @@
                 graph().addBeforeFixed(this, nullGuard);
                 condition = typeTest;
                 stamp = stamp.join(StampFactory.objectNonNull());
+                nullGuard.lower(tool);
             } else {
                 // TODO (ds) replace with probability of null-seen when available
                 double shortCircuitProbability = NOT_FREQUENT_PROBABILITY;
@@ -123,6 +124,7 @@
         }
         GuardingPiNode checkedObject = graph().add(new GuardingPiNode(object, condition, false, forStoreCheck ? ArrayStoreException : ClassCastException, InvalidateReprofile, stamp));
         graph().replaceFixedWithFixed(this, checkedObject);
+        checkedObject.lower(tool);
     }
 
     @Override
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/ExceptionObjectNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/ExceptionObjectNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -89,6 +89,7 @@
         graph().addAfterFixed(this, loadException);
         setStateAfter(null);
         setStamp(StampFactory.forVoid());
+        loadException.lower(tool);
     }
 
     @Override
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/Lowerable.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/Lowerable.java	Wed Sep 25 21:49:39 2013 +0200
@@ -24,8 +24,16 @@
 
 import com.oracle.graal.nodes.*;
 
+/**
+ * Interface implemented by nodes that can replace themselves with lower level nodes during a phase
+ * that transforms a graph to replace higher level nodes with lower level nodes.
+ */
 public interface Lowerable {
 
+    /**
+     * Expand this node into lower level nodes expressing the same semantics. If the introduced
+     * nodes are themselves lowerable, they should be recursively lowered as part of this call.
+     */
     void lower(LoweringTool tool);
 
     ValueNode asNode();
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java	Wed Sep 25 21:49:39 2013 +0200
@@ -267,7 +267,7 @@
 // @formatter:off
 //     cases:                                           original node:
 //                                         |Floating|Fixed-unconnected|Fixed-connected|
-//                                         -------------------------------------------|
+//                                         --------------------------------------------
 //                                     null|   1    |        X        |       3       |
 //                                         --------------------------------------------
 //                                 Floating|   2    |        X        |       4       |
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/LoweringPhase.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/LoweringPhase.java	Wed Sep 25 21:49:39 2013 +0200
@@ -142,31 +142,55 @@
         this.canonicalizer = canonicalizer;
     }
 
-    private static boolean containsLowerable(NodeIterable<Node> nodes) {
-        for (Node n : nodes) {
-            if (n instanceof Lowerable) {
-                return true;
-            }
-        }
-        return false;
+    /**
+     * Checks that second lowering of a given graph did not introduce any new nodes.
+     * 
+     * @param graph a graph that was just {@linkplain #lower lowered}
+     * @throws AssertionError if the check fails
+     */
+    private boolean checkPostLowering(StructuredGraph graph, PhaseContext context) {
+        int expectedMark = graph.getMark();
+        lower(graph, context, 1);
+        int mark = graph.getMark();
+        assert mark == expectedMark : graph + ": a second round in the current lowering phase introduced these new nodes: " + graph.getNewNodes(mark).snapshot();
+        return true;
     }
 
     @Override
     protected void run(final StructuredGraph graph, PhaseContext context) {
-        int i = 0;
-        while (true) {
-            int mark = graph.getMark();
+        lower(graph, context, 0);
+        assert checkPostLowering(graph, context);
+    }
+
+    private void lower(StructuredGraph graph, PhaseContext context, int i) {
+        IncrementalCanonicalizerPhase<PhaseContext> incrementalCanonicalizer = new IncrementalCanonicalizerPhase<>(canonicalizer);
+        incrementalCanonicalizer.appendPhase(new Round(i, context));
+        incrementalCanonicalizer.apply(graph, context);
+        assert graph.verify();
+    }
 
-            IncrementalCanonicalizerPhase<PhaseContext> incrementalCanonicalizer = new IncrementalCanonicalizerPhase<>(canonicalizer);
-            incrementalCanonicalizer.appendPhase(new Round(i++, context));
-            incrementalCanonicalizer.apply(graph, context);
-
-            if (!containsLowerable(graph.getNewNodes(mark))) {
-                // No new lowerable nodes - done!
-                break;
+    /**
+     * Checks that lowering of a given node did not introduce any new {@link Lowerable} nodes that
+     * could be lowered in the current {@link LoweringPhase}. Such nodes must be recursively lowered
+     * as part of lowering {@code node}.
+     * 
+     * @param node a node that was just lowered
+     * @param preLoweringMark the graph mark before {@code node} was lowered
+     * @throws AssertionError if the check fails
+     */
+    private static boolean checkPostNodeLowering(Node node, LoweringToolImpl loweringTool, int preLoweringMark) {
+        StructuredGraph graph = (StructuredGraph) node.graph();
+        int postLoweringMark = graph.getMark();
+        NodeIterable<Node> newNodesAfterLowering = graph.getNewNodes(preLoweringMark);
+        for (Node n : newNodesAfterLowering) {
+            if (n instanceof Lowerable) {
+                ((Lowerable) n).lower(loweringTool);
+                int mark = graph.getMark();
+                assert postLoweringMark == mark : graph + ": lowering of " + node + " produced lowerable " + n + " that should have been recursively lowered as it introduces these new nodes: " +
+                                graph.getNewNodes(postLoweringMark).snapshot();
             }
-            assert graph.verify();
         }
+        return true;
     }
 
     private final class Round extends Phase {
@@ -241,12 +265,14 @@
 
                 if (node instanceof Lowerable) {
                     assert checkUsagesAreScheduled(node);
+                    int preLoweringMark = node.graph().getMark();
                     ((Lowerable) node).lower(loweringTool);
+                    assert checkPostNodeLowering(node, loweringTool, preLoweringMark);
                 }
 
                 if (!nextNode.isAlive()) {
-                    // can happen when the rest of the block is killed by lowering (e.g. by a
-                    // unconditional deopt)
+                    // can happen when the rest of the block is killed by lowering
+                    // (e.g. by an unconditional deopt)
                     break;
                 } else {
                     Node nextLastFixed = nextNode.predecessor();
--- a/graal/com.oracle.graal.replacements.amd64/src/com/oracle/graal/replacements/amd64/AMD64ConvertNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.replacements.amd64/src/com/oracle/graal/replacements/amd64/AMD64ConvertNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -30,9 +30,8 @@
 
 /**
  * This node has the semantics of the AMD64 conversions. It is used in the lowering of the
- * ConvertNode which, on AMD64 needs a AMD64ConvertNode plus some fixup code that handles the corner
- * cases that differ between AMD64 and Java.
- * 
+ * {@link ConvertNode} which, on AMD64 needs a {@link AMD64ConvertNode} plus some fixup code that
+ * handles the corner cases that differ between AMD64 and Java.
  */
 public class AMD64ConvertNode extends FloatingNode implements ArithmeticLIRLowerable {
 
--- a/graal/com.oracle.graal.replacements.amd64/src/com/oracle/graal/replacements/amd64/AMD64ConvertSnippets.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.replacements.amd64/src/com/oracle/graal/replacements/amd64/AMD64ConvertSnippets.java	Wed Sep 25 21:49:39 2013 +0200
@@ -181,6 +181,7 @@
             SnippetTemplate template = template(args);
             Debug.log("Lowering %s in %s: node=%s, template=%s, arguments=%s", convert.opcode, graph, convert, template, args);
             template.instantiate(runtime, replacee, DEFAULT_REPLACER, tool, args);
+            graph.removeFloating(convert);
         }
     }
 }
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java	Wed Sep 25 21:49:39 2013 +0200
@@ -391,7 +391,7 @@
     /**
      * Creates a snippet template.
      */
-    protected SnippetTemplate(MetaAccessProvider runtime, Replacements replacements, Arguments args) {
+    protected SnippetTemplate(final MetaAccessProvider runtime, final Replacements replacements, Arguments args) {
         StructuredGraph snippetGraph = replacements.getSnippet(args.info.method);
 
         ResolvedJavaMethod method = snippetGraph.method();
@@ -400,7 +400,7 @@
         PhaseContext context = new PhaseContext(runtime, replacements.getAssumptions(), replacements);
 
         // Copy snippet graph, replacing constant parameters with given arguments
-        StructuredGraph snippetCopy = new StructuredGraph(snippetGraph.name, snippetGraph.method());
+        final StructuredGraph snippetCopy = new StructuredGraph(snippetGraph.name, snippetGraph.method());
         IdentityHashMap<Node, Node> nodeReplacements = new IdentityHashMap<>();
         nodeReplacements.put(snippetGraph.start(), snippetCopy.start());
 
@@ -504,8 +504,13 @@
 
         // Perform lowering on the snippet
         snippetCopy.setGuardsStage(args.cacheKey.guardsStage);
-        PhaseContext c = new PhaseContext(runtime, new Assumptions(false), replacements);
-        new LoweringPhase(new CanonicalizerPhase(true)).apply(snippetCopy, c);
+        Debug.scope("LoweringSnippetTemplate", snippetCopy, new Runnable() {
+
+            public void run() {
+                PhaseContext c = new PhaseContext(runtime, new Assumptions(false), replacements);
+                new LoweringPhase(new CanonicalizerPhase(true)).apply(snippetCopy, c);
+            }
+        });
 
         // Remove all frame states from snippet graph. Snippets must be atomic (i.e. free
         // of side-effects that prevent deoptimizing to a point before the snippet).
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -22,6 +22,8 @@
  */
 package com.oracle.graal.replacements.nodes;
 
+import static java.lang.reflect.Modifier.*;
+
 import java.lang.reflect.*;
 
 import com.oracle.graal.api.meta.*;
@@ -106,18 +108,27 @@
 
     @Override
     public void lower(LoweringTool tool) {
-        boolean nullCheck = false;
         StructuredGraph replacementGraph = getLoweredSnippetGraph(tool);
         if (replacementGraph == null) {
             replacementGraph = getLoweredSubstitutionGraph(tool);
-            nullCheck = true;
         }
 
         InvokeNode invoke = replaceWithInvoke();
         assert invoke.verify();
 
         if (replacementGraph != null) {
-            InliningUtil.inline(invoke, replacementGraph, nullCheck);
+            // Pull out the receiver null check so that a replaced
+            // receiver can be lowered if necessary
+            if (!isStatic(targetMethod.getModifiers())) {
+                ValueNode nonNullReceiver = InliningUtil.nonNullReceiver(invoke);
+                if (nonNullReceiver instanceof Lowerable) {
+                    ((Lowerable) nonNullReceiver).lower(tool);
+                }
+            }
+            InliningUtil.inline(invoke, replacementGraph, false);
+            Debug.dump(graph(), "After inlining replacement %s", replacementGraph);
+        } else {
+            invoke.lower(tool);
         }
     }
 
--- a/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/asserts/NeverInlineMacroNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/asserts/NeverInlineMacroNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -37,5 +37,6 @@
         InvokeNode invoke = createInvoke();
         graph().replaceFixedWithFixed(this, invoke);
         invoke.setUseForInlining(false);
+        invoke.lower(tool);
     }
 }
--- a/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/frame/FrameAccessNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/frame/FrameAccessNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -89,7 +89,7 @@
     @Override
     public String toString(Verbosity verbosity) {
         if (verbosity == Verbosity.Name) {
-            return super.toString(verbosity) + getSlotKind().name() + (isConstantFrameSlot() ? " " + getConstantFrameSlot() : "");
+            return super.toString(verbosity) + getSlotKind().name() + (slot != null && isConstantFrameSlot() ? " " + getConstantFrameSlot() : "");
         } else {
             return super.toString(verbosity);
         }
--- a/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/frame/FrameGetNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/frame/FrameGetNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -88,6 +88,8 @@
             loadNode = graph().add(new UnsafeLoadNode(loadFieldNode, Unsafe.ARRAY_LONG_BASE_OFFSET, slotOffset, getSlotKind()));
         }
         structuredGraph.replaceFixedWithFixed(this, loadNode);
+        loadFieldNode.lower(tool);
+        ((Lowerable) loadNode).lower(tool);
     }
 
     @NodeIntrinsic
--- a/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/frame/FrameSetNode.java	Wed Sep 25 21:48:38 2013 +0200
+++ b/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/nodes/frame/FrameSetNode.java	Wed Sep 25 21:49:39 2013 +0200
@@ -86,6 +86,8 @@
             storeNode = graph().add(new StoreIndexedNode(loadFieldNode, slotIndex, Kind.Long, value));
         }
         structuredGraph.replaceFixedWithFixed(this, storeNode);
+        loadFieldNode.lower(tool);
+        ((Lowerable) storeNode).lower(tool);
     }
 
     @NodeIntrinsic