Mercurial > hg > graal-compiler

changeset 22392:9dafd1dc5ff9

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Prevent false positives in change detection of second lowering round.
author Roland Schatz <roland.schatz@oracle.com>
date Thu, 30 Jul 2015 16:38:54 +0200
parents 1825ca1a694a
children 9a5a7d9a23aa
files graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/LoweringPhase.java graal/com.oracle.graal.phases/src/com/oracle/graal/phases/schedule/SchedulePhase.java
diffstat 2 files changed, 94 insertions(+), 46 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/LoweringPhase.java	Wed Jul 29 15:43:28 2015 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/LoweringPhase.java	Thu Jul 30 16:38:54 2015 +0200
@@ -180,7 +180,7 @@
      */
     private boolean checkPostLowering(StructuredGraph graph, PhaseContext context) {
         Mark expectedMark = graph.getMark();
-        lower(graph, context, 1);
+        lower(graph, context, LoweringMode.VERIFY_LOWERING);
         Mark mark = graph.getMark();
         assert mark.equals(expectedMark) : graph + ": a second round in the current lowering phase introduced these new nodes: " + graph.getNewNodes(expectedMark).snapshot();
         return true;
@@ -188,13 +188,13 @@
 
     @Override
     protected void run(final StructuredGraph graph, PhaseContext context) {
-        lower(graph, context, 0);
+        lower(graph, context, LoweringMode.LOWERING);
         assert checkPostLowering(graph, context);
     }
 
-    private void lower(StructuredGraph graph, PhaseContext context, int i) {
+    private void lower(StructuredGraph graph, PhaseContext context, LoweringMode mode) {
         IncrementalCanonicalizerPhase<PhaseContext> incrementalCanonicalizer = new IncrementalCanonicalizerPhase<>(canonicalizer);
-        incrementalCanonicalizer.appendPhase(new Round(i, context));
+        incrementalCanonicalizer.appendPhase(new Round(context, mode));
         incrementalCanonicalizer.apply(graph, context);
         assert graph.verify();
     }
@@ -233,21 +233,41 @@
         return true;
     }
 
+    private enum LoweringMode {
+        LOWERING,
+        VERIFY_LOWERING
+    }
+
     private final class Round extends Phase {
 
         private final PhaseContext context;
+        private final LoweringMode mode;
         private final SchedulePhase schedule;
-        private final int iteration;
+
+        private Round(PhaseContext context, LoweringMode mode) {
+            this.context = context;
+            this.mode = mode;
 
-        private Round(int iteration, PhaseContext context) {
-            this.iteration = iteration;
-            this.context = context;
-            this.schedule = new SchedulePhase();
+            /*
+             * In VERIFY_LOWERING, we want to verify whether the lowering itself changes the graph.
+             * Make sure we're not detecting spurious changes because the SchedulePhase modifies the
+             * graph.
+             */
+            boolean immutableSchedule = mode == LoweringMode.VERIFY_LOWERING;
+
+            this.schedule = new SchedulePhase(immutableSchedule);
         }
 
         @Override
         protected CharSequence createName() {
-            return "LoweringIteration" + iteration;
+            switch (mode) {
+                case LOWERING:
+                    return "LoweringRound";
+                case VERIFY_LOWERING:
+                    return "VerifyLoweringRound";
+                default:
+                    throw JVMCIError.shouldNotReachHere();
+            }
         }
 
         @Override
@@ -404,7 +424,7 @@
      *     if (alwaysReachedBlock != null &amp;&amp; alwaysReachedBlock.getDominator() == block) {
      *         processBlock(alwaysReachedBlock);
      *     }
-     * 
+     *
      *     // Now go for the other dominators.
      *     for (Block dominated : block.getDominated()) {
      *         if (dominated != alwaysReachedBlock) {
--- a/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/schedule/SchedulePhase.java	Wed Jul 29 15:43:28 2015 +0200
+++ b/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/schedule/SchedulePhase.java	Thu Jul 30 16:38:54 2015 +0200
@@ -27,11 +27,12 @@
 
 import java.util.*;
 
-import com.oracle.graal.debug.*;
 import jdk.internal.jvmci.meta.*;
 
 import com.oracle.graal.compiler.common.*;
 import com.oracle.graal.compiler.common.cfg.*;
+import com.oracle.graal.debug.*;
+import com.oracle.graal.graph.Graph.*;
 import com.oracle.graal.graph.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.cfg.*;
@@ -80,54 +81,81 @@
     private final SchedulingStrategy selectedStrategy;
     private NodeMap<Block> nodeToBlockMap;
 
+    private final boolean immutableGraph;
+
     public SchedulePhase() {
-        this(OptScheduleOutOfLoops.getValue() ? SchedulingStrategy.LATEST_OUT_OF_LOOPS : SchedulingStrategy.LATEST);
+        this(false);
+    }
+
+    public SchedulePhase(boolean immutableGraph) {
+        this(OptScheduleOutOfLoops.getValue() ? SchedulingStrategy.LATEST_OUT_OF_LOOPS : SchedulingStrategy.LATEST, immutableGraph);
     }
 
     public SchedulePhase(SchedulingStrategy strategy) {
+        this(strategy, false);
+    }
+
+    public SchedulePhase(SchedulingStrategy strategy, boolean immutableGraph) {
         this.selectedStrategy = strategy;
+        this.immutableGraph = immutableGraph;
+    }
+
+    private NodeEventScope verifyImmutableGraph(StructuredGraph graph) {
+        boolean assertionsEnabled = false;
+        assert (assertionsEnabled = true) == true;
+        if (immutableGraph && assertionsEnabled) {
+            return graph.trackNodeEvents(new NodeEventListener() {
+                public void event(NodeEvent e, Node node) {
+                    assert false : "graph changed: " + e + " on node " + node;
+                }
+            });
+        } else {
+            return null;
+        }
     }
 
     @Override
     protected void run(StructuredGraph graph) {
-        // assert GraphOrder.assertNonCyclicGraph(graph);
-        cfg = ControlFlowGraph.compute(graph, true, true, true, false);
+        try (NodeEventScope scope = verifyImmutableGraph(graph)) {
+            // assert GraphOrder.assertNonCyclicGraph(graph);
+            cfg = ControlFlowGraph.compute(graph, true, true, true, false);
 
-        if (selectedStrategy == SchedulingStrategy.EARLIEST) {
-            // Assign early so we are getting a context in case of an exception.
-            this.nodeToBlockMap = graph.createNodeMap();
-            this.blockToNodesMap = new BlockMap<>(cfg);
-            NodeBitMap visited = graph.createNodeBitMap();
-            scheduleEarliestIterative(cfg, blockToNodesMap, nodeToBlockMap, visited, graph);
-            return;
-        } else {
-            boolean isOutOfLoops = selectedStrategy == SchedulingStrategy.LATEST_OUT_OF_LOOPS;
-            if (selectedStrategy == SchedulingStrategy.LATEST || isOutOfLoops) {
-                NodeMap<Block> currentNodeMap = graph.createNodeMap();
-                BlockMap<List<Node>> earliestBlockToNodesMap = new BlockMap<>(cfg);
-                NodeBitMap visited = graph.createNodeBitMap();
-
+            if (selectedStrategy == SchedulingStrategy.EARLIEST) {
                 // Assign early so we are getting a context in case of an exception.
-                this.blockToNodesMap = earliestBlockToNodesMap;
-                this.nodeToBlockMap = currentNodeMap;
+                this.nodeToBlockMap = graph.createNodeMap();
+                this.blockToNodesMap = new BlockMap<>(cfg);
+                NodeBitMap visited = graph.createNodeBitMap();
+                scheduleEarliestIterative(blockToNodesMap, nodeToBlockMap, visited, graph);
+                return;
+            } else {
+                boolean isOutOfLoops = selectedStrategy == SchedulingStrategy.LATEST_OUT_OF_LOOPS;
+                if (selectedStrategy == SchedulingStrategy.LATEST || isOutOfLoops) {
+                    NodeMap<Block> currentNodeMap = graph.createNodeMap();
+                    BlockMap<List<Node>> earliestBlockToNodesMap = new BlockMap<>(cfg);
+                    NodeBitMap visited = graph.createNodeBitMap();
 
-                scheduleEarliestIterative(cfg, earliestBlockToNodesMap, currentNodeMap, visited, graph);
-                BlockMap<List<Node>> latestBlockToNodesMap = new BlockMap<>(cfg);
-
-                for (Block b : cfg.getBlocks()) {
-                    latestBlockToNodesMap.put(b, new ArrayList<Node>());
-                }
+                    // Assign early so we are getting a context in case of an exception.
+                    this.blockToNodesMap = earliestBlockToNodesMap;
+                    this.nodeToBlockMap = currentNodeMap;
 
-                BlockMap<ArrayList<FloatingReadNode>> watchListMap = calcLatestBlocks(isOutOfLoops, currentNodeMap, earliestBlockToNodesMap, visited, latestBlockToNodesMap);
-                sortNodesLatestWithinBlock(cfg, earliestBlockToNodesMap, latestBlockToNodesMap, currentNodeMap, watchListMap, visited);
+                    scheduleEarliestIterative(earliestBlockToNodesMap, currentNodeMap, visited, graph);
+                    BlockMap<List<Node>> latestBlockToNodesMap = new BlockMap<>(cfg);
+
+                    for (Block b : cfg.getBlocks()) {
+                        latestBlockToNodesMap.put(b, new ArrayList<Node>());
+                    }
 
-                assert verifySchedule(cfg, latestBlockToNodesMap, currentNodeMap);
-                assert MemoryScheduleVerification.check(cfg.getStartBlock(), latestBlockToNodesMap);
+                    BlockMap<ArrayList<FloatingReadNode>> watchListMap = calcLatestBlocks(isOutOfLoops, currentNodeMap, earliestBlockToNodesMap, visited, latestBlockToNodesMap);
+                    sortNodesLatestWithinBlock(cfg, earliestBlockToNodesMap, latestBlockToNodesMap, currentNodeMap, watchListMap, visited);
+
+                    assert verifySchedule(cfg, latestBlockToNodesMap, currentNodeMap);
+                    assert MemoryScheduleVerification.check(cfg.getStartBlock(), latestBlockToNodesMap);
 
-                this.blockToNodesMap = latestBlockToNodesMap;
-                this.nodeToBlockMap = currentNodeMap;
+                    this.blockToNodesMap = latestBlockToNodesMap;
+                    this.nodeToBlockMap = currentNodeMap;
 
-                cfg.setNodeToBlock(currentNodeMap);
+                    cfg.setNodeToBlock(currentNodeMap);
+                }
             }
         }
     }
@@ -464,7 +492,7 @@
         return currentBlock;
     }
 
-    private static void scheduleEarliestIterative(ControlFlowGraph cfg, BlockMap<List<Node>> blockToNodes, NodeMap<Block> nodeToBlock, NodeBitMap visited, StructuredGraph graph) {
+    private void scheduleEarliestIterative(BlockMap<List<Node>> blockToNodes, NodeMap<Block> nodeToBlock, NodeBitMap visited, StructuredGraph graph) {
 
         BitSet floatingReads = new BitSet(cfg.getBlocks().size());
 
@@ -532,7 +560,7 @@
         } while (unmarkedPhi && changed);
 
         // Check for dead nodes.
-        if (visited.getCounter() < graph.getNodeCount()) {
+        if (!immutableGraph && visited.getCounter() < graph.getNodeCount()) {
             for (Node n : graph.getNodes()) {
                 if (!visited.isMarked(n)) {
                     n.clearInputs();