# HG changeset patch # User Roland Schatz # Date 1438267134 -7200 # Node ID 9dafd1dc5ff9788a838653f3cd9d79a4b9acf18c # Parent 1825ca1a694a6d75ba341a9affa0bd074fb87900 Prevent false positives in change detection of second lowering round. diff -r 1825ca1a694a -r 9dafd1dc5ff9 graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/LoweringPhase.java --- 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 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 && alwaysReachedBlock.getDominator() == block) { * processBlock(alwaysReachedBlock); * } - * + * * // Now go for the other dominators. * for (Block dominated : block.getDominated()) { * if (dominated != alwaysReachedBlock) { diff -r 1825ca1a694a -r 9dafd1dc5ff9 graal/com.oracle.graal.phases/src/com/oracle/graal/phases/schedule/SchedulePhase.java --- 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 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 currentNodeMap = graph.createNodeMap(); - BlockMap> 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 currentNodeMap = graph.createNodeMap(); + BlockMap> earliestBlockToNodesMap = new BlockMap<>(cfg); + NodeBitMap visited = graph.createNodeBitMap(); - scheduleEarliestIterative(cfg, earliestBlockToNodesMap, currentNodeMap, visited, graph); - BlockMap> latestBlockToNodesMap = new BlockMap<>(cfg); - - for (Block b : cfg.getBlocks()) { - latestBlockToNodesMap.put(b, new ArrayList()); - } + // Assign early so we are getting a context in case of an exception. + this.blockToNodesMap = earliestBlockToNodesMap; + this.nodeToBlockMap = currentNodeMap; - BlockMap> watchListMap = calcLatestBlocks(isOutOfLoops, currentNodeMap, earliestBlockToNodesMap, visited, latestBlockToNodesMap); - sortNodesLatestWithinBlock(cfg, earliestBlockToNodesMap, latestBlockToNodesMap, currentNodeMap, watchListMap, visited); + scheduleEarliestIterative(earliestBlockToNodesMap, currentNodeMap, visited, graph); + BlockMap> latestBlockToNodesMap = new BlockMap<>(cfg); + + for (Block b : cfg.getBlocks()) { + latestBlockToNodesMap.put(b, new ArrayList()); + } - assert verifySchedule(cfg, latestBlockToNodesMap, currentNodeMap); - assert MemoryScheduleVerification.check(cfg.getStartBlock(), latestBlockToNodesMap); + BlockMap> 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> blockToNodes, NodeMap nodeToBlock, NodeBitMap visited, StructuredGraph graph) { + private void scheduleEarliestIterative(BlockMap> blockToNodes, NodeMap 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();