[GR-65873] More aggressive CFG caching

PullRequest: graal/21091

Author: rmosaner

compiler/src/jdk.graal.compiler.microbenchmarks/src/jdk/graal/compiler/microbenchmarks/lir/GraalCompilerState.java

@@ -393,7 +393,7 @@ protected DebugContext getGraphDebug() {
     }
 
     private static ControlFlowGraph deepCopy(ControlFlowGraph cfg) {
-        return ControlFlowGraph.newBuilder(cfg.graph).backendBlocks(true).connectBlocks(true).computeFrequency(true).computeLoops(true).computeDominators(true).computePostdominators(
+        return ControlFlowGraph.newBuilder(cfg.graph).modifiableBlocks(true).connectBlocks(true).computeFrequency(true).computeLoops(true).computeDominators(true).computePostdominators(
                         true).build();
     }
 

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/GraphScheduleTest.java

@@ -26,13 +26,14 @@
 
 import java.util.List;
 
+import org.junit.Assert;
+
 import jdk.graal.compiler.graph.Node;
 import jdk.graal.compiler.graph.NodeMap;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.nodes.StructuredGraph.ScheduleResult;
 import jdk.graal.compiler.nodes.cfg.HIRBlock;
 import jdk.graal.compiler.phases.schedule.SchedulePhase;
-import org.junit.Assert;
 
 public class GraphScheduleTest extends GraalCompilerTest {
 
@@ -50,7 +51,7 @@ protected void assertOrderedAfterLastSchedule(StructuredGraph graph, Node a, Nod
     }
 
     protected void assertOrderedAfterSchedule(ScheduleResult ibp, Node a, Node b) {
-        NodeMap<HIRBlock> nodeToBlock = ibp.getCFG().getNodeToBlock();
+        NodeMap<HIRBlock> nodeToBlock = ibp.getNodeToBlockMap();
         HIRBlock bBlock = nodeToBlock.get(b);
         HIRBlock aBlock = nodeToBlock.get(a);
 

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/MemoryScheduleTest.java

@@ -641,7 +641,7 @@ private void assertReadWithinAllReturnBlocks(ScheduleResult schedule, boolean wi
         int withRead = 0;
         int returnBlocks = 0;
         for (ReturnNode returnNode : graph.getNodes(ReturnNode.TYPE)) {
-            HIRBlock block = schedule.getCFG().getNodeToBlock().get(returnNode);
+            HIRBlock block = schedule.getNodeToBlockMap().get(returnNode);
             for (Node node : schedule.getBlockToNodesMap().get(block)) {
                 if (node instanceof FloatingReadNode) {
                     withRead++;
@@ -667,7 +667,7 @@ private static void assertReadAndWriteInSameBlock(ScheduleResult schedule, boole
         StructuredGraph graph = schedule.getCFG().graph;
         FloatingReadNode read = graph.getNodes().filter(FloatingReadNode.class).first();
         WriteNode write = graph.getNodes().filter(WriteNode.class).first();
-        assertTrue(!(inSame ^ schedule.getCFG().blockFor(read) == schedule.getCFG().blockFor(write)));
+        assertTrue(!(inSame ^ schedule.blockFor(read) == schedule.blockFor(write)));
     }
 
     private static void assertReadBeforeAllWritesInStartBlock(ScheduleResult schedule) {
@@ -712,6 +712,11 @@ private ScheduleResult getFinalSchedule(final String snippet, final TestMode mod
                 graph.clearAllStateAfterForTestingOnly();
                 // disable state split verification
                 graph.getGraphState().setAfterFSA();
+                if (graph.hasLoops() && graph.isLastCFGValid()) {
+                    // CFGLoops are computed differently after FSA, see CFGLoop#getLoopExits(). The
+                    // cached cfg needs to have its loop information invalidated.
+                    graph.getLastCFG().invalidateLoopInformation();
+                }
             }
             debug.dump(DebugContext.BASIC_LEVEL, graph, "after removal of framestates");
 

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/SchedulingTest.java

@@ -26,6 +26,8 @@
 
 import java.util.List;
 
+import org.junit.Test;
+
 import jdk.graal.compiler.graph.Node;
 import jdk.graal.compiler.graph.NodeMap;
 import jdk.graal.compiler.nodes.FrameState;
@@ -39,7 +41,6 @@
 import jdk.graal.compiler.nodes.util.GraphUtil;
 import jdk.graal.compiler.phases.schedule.SchedulePhase;
 import jdk.graal.compiler.phases.schedule.SchedulePhase.SchedulingStrategy;
-import org.junit.Test;
 
 public class SchedulingTest extends GraphScheduleTest {
 
@@ -65,7 +66,7 @@ public void testValueProxyInputs() {
         SchedulePhase schedulePhase = new SchedulePhase(SchedulingStrategy.LATEST);
         schedulePhase.apply(graph, getDefaultHighTierContext());
         ScheduleResult schedule = graph.getLastSchedule();
-        NodeMap<HIRBlock> nodeToBlock = schedule.getCFG().getNodeToBlock();
+        NodeMap<HIRBlock> nodeToBlock = schedule.getNodeToBlockMap();
         assertTrue(graph.getNodes().filter(LoopExitNode.class).count() == 1);
         LoopExitNode loopExit = graph.getNodes().filter(LoopExitNode.class).first();
         List<Node> list = schedule.nodesFor(nodeToBlock.get(loopExit));

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/common/GraalOptions.java

@@ -350,4 +350,7 @@ public final class GraalOptions {
 
     @Option(help = "Enables target-specific lowering and legalization of SIMD operations. Required for SIMD code generation.", type = OptionType.Debug)
     public static final OptionKey<Boolean> TargetVectorLowering = new OptionKey<>(true);
+
+    @Option(help = "Enables caching of data structures like control flow graph or schedule across compiler phases.", type = OptionType.Debug)
+    public static final OptionKey<Boolean> CacheCompilerDataStructures = new OptionKey<>(true);
 }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/gen/LIRCompilerBackend.java

@@ -171,7 +171,7 @@ private static LIRGenerationResult emitLIR0(Backend backend,
                  * LIRGeneration may have changed the CFG, so in case a schedule is needed afterward
                  * we make sure it will be recomputed.
                  */
-                graph.clearLastSchedule();
+                graph.clearLastCFG();
                 return result;
             } catch (Throwable e) {
                 throw debug.handle(e);

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graph/Graph.java

@@ -669,6 +669,11 @@ public enum NodeEvent {
          */
         INPUT_CHANGED,
 
+        /**
+         * A node's control flow edge (i.e., predecessor or any successor) has changed.
+         */
+        CONTROL_FLOW_CHANGED,
+
         /**
          * A node's {@linkplain Node#usages() usages} count dropped to zero.
          */
@@ -737,6 +742,9 @@ public abstract static class NodeEventListener {
          */
         final void event(NodeEvent e, Node node) {
             switch (e) {
+                case CONTROL_FLOW_CHANGED:
+                    controlFlowChanged(node);
+                    break;
                 case INPUT_CHANGED:
                     inputChanged(node);
                     break;
@@ -773,7 +781,17 @@ public void changed(NodeEvent e, Node node) {
         }
 
         /**
-         * Notifies this listener about a change in a node's inputs.
+         * Notifies this listener about a change in a node's control flow edges, i.e., its
+         * predecessor or any of its successor edges.
+         *
+         * @param node a node who has had its predecessor or one of its successors changed
+         */
+        public void controlFlowChanged(Node node) {
+        }
+
+        /**
+         * Notifies this listener about a change in a node's inputs (successor or predecessor edges
+         * not included).
          *
          * @param node a node who has had one of its inputs changed
          */

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graph/Node.java

@@ -819,9 +819,9 @@ protected void updatePredecessor(Node oldSuccessor, Node newSuccessor) {
             if (newSuccessor != null) {
                 assertTrue(newSuccessor.predecessor == null, "unexpected non-null predecessor in new successor (%s): %s, this=%s", newSuccessor, newSuccessor.predecessor, this);
                 newSuccessor.predecessor = this;
-                maybeNotifyInputChanged(newSuccessor);
+                maybeNotifyControlFlowChanged(newSuccessor);
             }
-            maybeNotifyInputChanged(this);
+            maybeNotifyControlFlowChanged(this);
         }
     }
 
@@ -1261,9 +1261,17 @@ public void replaceAtUsages(Node replacement, InputType... inputTypes) {
         }
     }
 
+    private void maybeNotifyControlFlowChanged(Node node) {
+        if (graph != null) {
+            assert !graph.isFrozen() : "Frozen graph must not change!";
+            graph.fireNodeEvent(Graph.NodeEvent.CONTROL_FLOW_CHANGED, node);
+            graph.edgeModificationCount++;
+        }
+    }
+
     private void maybeNotifyInputChanged(Node node) {
         if (graph != null) {
-            assert !graph.isFrozen();
+            assert !graph.isFrozen() : "Frozen graph must not change!";
             graph.fireNodeEvent(Graph.NodeEvent.INPUT_CHANGED, node);
             graph.edgeModificationCount++;
         }
@@ -1276,7 +1284,7 @@ private void maybeNotifyInputChanged(Node node) {
      */
     public void maybeNotifyZeroUsages(Node node) {
         if (graph != null && node.isAlive()) {
-            assert !graph.isFrozen();
+            assert !graph.isFrozen() : "Frozen graph must not change!";
             graph.fireNodeEvent(Graph.NodeEvent.ZERO_USAGES, node);
         }
     }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graph/NodeMap.java

@@ -67,6 +67,12 @@ private void checkAndGrow(Node node) {
         assert check(node);
     }
 
+    public void growToSize(int newSize) {
+        if (newSize > this.values.length) {
+            this.values = Arrays.copyOf(values, Math.max(MIN_REALLOC_SIZE, newSize));
+        }
+    }
+
     @Override
     public boolean isEmpty() {
         throw new UnsupportedOperationException("isEmpty() is not supported for performance reasons");

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/LoopTransformations.java

@@ -73,6 +73,7 @@
 import jdk.graal.compiler.nodes.calc.AddNode;
 import jdk.graal.compiler.nodes.calc.CompareNode;
 import jdk.graal.compiler.nodes.calc.ConditionalNode;
+import jdk.graal.compiler.nodes.cfg.ControlFlowGraph;
 import jdk.graal.compiler.nodes.extended.OpaqueNode;
 import jdk.graal.compiler.nodes.extended.SwitchNode;
 import jdk.graal.compiler.nodes.loop.CountedLoopInfo;
@@ -496,6 +497,7 @@ private static void setSingleVisitedLoopFrequencySplitProbability(AbstractBeginN
      * if the given loop exit is the only exit of the loop.
      */
     public static void adaptCountedLoopExitProbability(AbstractBeginNode lex, double newExitCheckFrequency) {
+        invalidateCFGFrequencies(lex.graph().getLastCFG());
         double probability = 1.0D - 1.0D / newExitCheckFrequency;
         if (probability <= 0D) {
             setSingleVisitedLoopFrequencySplitProbability(lex);
@@ -506,6 +508,12 @@ public static void adaptCountedLoopExitProbability(AbstractBeginNode lex, double
         ifNode.setTrueSuccessorProbability(BranchProbabilityData.injected(probability, trueSucc));
     }
 
+    private static void invalidateCFGFrequencies(ControlFlowGraph cfg) {
+        if (cfg != null) {
+            cfg.invalidateFrequencies();
+        }
+    }
+
     public static class PreMainPostResult {
         private final LoopBeginNode preLoop;
         private final LoopBeginNode mainLoop;