[GR-48248] [GR-38949] Don't try to incorrectly move virtual state inp…

…uts out of loops

PullRequest: graal/15590

compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/phases/schedule/SchedulePhase.java

@@ -585,7 +585,7 @@ protected void calcLatestBlock(HIRBlock earliestBlock, SchedulingStrategy strate
                          */
                         continue;
                     }
-                    latestBlock = calcBlockForUsage(currentNode, usage, latestBlock, currentNodeMap, moveInputsIntoDominator);
+                    latestBlock = calcLatestBlockForUsage(currentNode, usage, earliestBlock, latestBlock, currentNodeMap, moveInputsIntoDominator);
                     checkLatestEarliestRelation(currentNode, earliestBlock, latestBlock, usage);
                 }
 
@@ -687,9 +687,10 @@ private static Node getUnproxifiedUncompressed(Node node) {
             return result;
         }
 
-        private static HIRBlock calcBlockForUsage(Node node, Node usage, HIRBlock startBlock, NodeMap<HIRBlock> currentNodeMap, NodeBitMap moveInputsToDominator) {
+        private static HIRBlock calcLatestBlockForUsage(Node node, Node usage, HIRBlock earliestBlock, HIRBlock initialLatestBlock, NodeMap<HIRBlock> currentNodeMap,
+                        NodeBitMap moveInputsToDominator) {
             assert !(node instanceof PhiNode);
-            HIRBlock currentBlock = startBlock;
+            HIRBlock currentBlock = initialLatestBlock;
             if (usage instanceof PhiNode) {
                 // An input to a PhiNode is used at the end of the predecessor block that
                 // corresponds to the PhiNode input. One PhiNode can use an input multiple times.
@@ -711,7 +712,16 @@ private static HIRBlock calcBlockForUsage(Node node, Node usage, HIRBlock startB
                 }
 
                 if (!(node instanceof VirtualState) && !moveInputsToDominator.isNew(usage) && moveInputsToDominator.isMarked(usage)) {
-                    otherBlock = otherBlock.getDominator();
+                    /*
+                     * The usage is marked as forcing its inputs into the dominator. Respect that if
+                     * we can, but the dominator might not be a legal position for the node. This is
+                     * the case for loop-variant floating nodes between a loop phi and a virtual
+                     * state on the loop begin.
+                     */
+                    HIRBlock dominator = otherBlock.getDominator();
+                    if (AbstractControlFlowGraph.dominates(earliestBlock, dominator)) {
+                        otherBlock = dominator;
+                    }
                     GraalError.guarantee(otherBlock != null, "Dominators need to be computed in the CFG");
                 }
 

compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/phases/util/GraphOrder.java

@@ -29,6 +29,7 @@
 
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.Equivalence;
+import org.graalvm.compiler.debug.Assertions;
 import org.graalvm.compiler.debug.DebugContext;
 import org.graalvm.compiler.debug.GraalError;
 import org.graalvm.compiler.graph.GraalGraphError;
@@ -150,12 +151,24 @@ private static void visitForward(ArrayList<Node> nodes, NodeBitMap visited, Node
         }
     }
 
+    public static boolean assertSchedulableGraph(StructuredGraph g) {
+        assert GraphOrder.assertNonCyclicGraph(g);
+        assert g.getGuardsStage() == GuardsStage.AFTER_FSA || GraphOrder.assertScheduleableBeforeFSA(g);
+        if (g.getGuardsStage() == GuardsStage.AFTER_FSA && Assertions.detailedAssertionsEnabled(g.getOptions())) {
+            // we still want to do a memory verification of the schedule even if we can
+            // no longer use assertSchedulableGraph after the floating reads phase
+            SchedulePhase.runWithoutContextOptimizations(g, SchedulePhase.SchedulingStrategy.LATEST_OUT_OF_LOOPS, true);
+        }
+        assert g.verify();
+        return true;
+    }
+
     /**
      * This method schedules the graph and makes sure that, for every node, all inputs are available
      * at the position where it is scheduled. This is a very expensive assertion.
      */
     @SuppressWarnings("try")
-    public static boolean assertSchedulableGraph(final StructuredGraph graph) {
+    private static boolean assertScheduleableBeforeFSA(final StructuredGraph graph) {
         assert graph.getGuardsStage() != GuardsStage.AFTER_FSA : "Cannot use the BlockIteratorClosure after FrameState Assignment, HIR Loop Data Structures are no longer valid.";
         try (DebugContext.Scope s = graph.getDebug().scope("AssertSchedulableGraph")) {
             SchedulePhase.runWithoutContextOptimizations(graph, getSchedulingPolicy(graph), true);