Stricter handling of LOOP_OVERFLOWS_CHECKED in the phase plan

Author: gergo-

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -64,13 +64,16 @@ public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
         return NotApplicable.ifAny(
                         super.notApplicableTo(graphState),
                         // keep in sync with stateAllowsPeeling()
+                        NotApplicable.unlessRunAfter(this, StageFlag.LOOP_OVERFLOWS_CHECKED, graphState),
                         NotApplicable.unlessRunBefore(this, StageFlag.FSA, graphState),
                         NotApplicable.unlessRunBefore(this, StageFlag.VALUE_PROXY_REMOVAL, graphState));
     }
 
     private static boolean stateAllowsPeeling(GraphState graphState) {
         // keep in sync with notApplicableTo()
-        return graphState.isBeforeStage(StageFlag.FSA) && graphState.isBeforeStage(StageFlag.VALUE_PROXY_REMOVAL);
+        return graphState.isAfterStage(StageFlag.LOOP_OVERFLOWS_CHECKED) &&
+                        graphState.isBeforeStage(StageFlag.FSA) &&
+                        graphState.isBeforeStage(StageFlag.VALUE_PROXY_REMOVAL);
     }
 
     @Override

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -122,7 +122,7 @@ public static boolean loopIsIn32BitRange(Loop loop) {
 
     @Override
     public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
-        return ALWAYS_APPLICABLE;
+        return NotApplicable.unlessRunAfter(this, GraphState.StageFlag.LOOP_OVERFLOWS_CHECKED, graphState);
     }
 
     @Override

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/AbstractInliningPhase.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -26,8 +26,10 @@
 
 import java.util.Optional;
 
+import jdk.graal.compiler.graph.Graph;
 import jdk.graal.compiler.nodes.GraphState;
 import jdk.graal.compiler.nodes.GraphState.StageFlag;
+import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.phases.BasePhase;
 import jdk.graal.compiler.phases.tiers.HighTierContext;
 
@@ -41,4 +43,24 @@ public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
                         NotApplicable.unlessRunBefore(this, StageFlag.HIGH_TIER_LOWERING, graphState),
                         NotApplicable.unlessRunBefore(this, StageFlag.FINAL_CANONICALIZATION, graphState));
     }
+
+    @Override
+    protected final void run(StructuredGraph graph, HighTierContext context) {
+        Graph.Mark mark = graph.getMark();
+        runInlining(graph, context);
+        if (!mark.isCurrent() && graph.getSpeculationLog() != null && graph.hasLoops()) {
+            /*
+             * We may have inlined new loops. We must make sure that counted loops are checked for
+             * overflow before we apply the next loop optimization phase. Inlining may run multiple
+             * times in different versions, possibly after some loop optimizations, and even on
+             * demand (i.e., without explicitly appearing in the phase plan). For such phases the
+             * stage flag mechanism isn't strong enough to express the constraint that we must run
+             * DisableOverflownCountedLoops before the next loop phase. Therefore, run it
+             * explicitly.
+             */
+            new DisableOverflownCountedLoopsPhase().run(graph);
+        }
+    }
+
+    protected abstract void runInlining(StructuredGraph graph, HighTierContext context);
 }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/DisableOverflownCountedLoopsPhase.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -64,9 +64,12 @@ protected void run(StructuredGraph graph) {
     @Override
     public void updateGraphState(GraphState graphState) {
         super.updateGraphState(graphState);
+        graphState.removeRequirementToStage(LOOP_OVERFLOWS_CHECKED);
         /*
          * This phase can run multiple times which is fine as long as we run it before all loop
-         * phases.
+         * phases. Note that newly inlined loops after this phase essentially invalidate this flag,
+         * but we have no formal invalidation mechanism. Inlining must therefore run this phase
+         * explicitly if it introduces new loops.
          */
         if (graphState.isBeforeStage(LOOP_OVERFLOWS_CHECKED)) {
             graphState.setAfterStage(LOOP_OVERFLOWS_CHECKED);

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/inlining/InliningPhase.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -87,7 +87,7 @@ public float codeSizeIncrease() {
      *
      */
     @Override
-    protected void run(final StructuredGraph graph, final HighTierContext context) {
+    protected void runInlining(final StructuredGraph graph, final HighTierContext context) {
         final InliningData data = new InliningData(graph, context, maxMethodPerInlining, canonicalizer, inliningPolicy, rootInvokes);
 
         int count = 0;

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/host/HostInliningPhase.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -179,7 +179,7 @@ private boolean isTransferToInterpreterMethod(InliningPhaseContext context, Reso
     }
 
     @Override
-    protected final void run(StructuredGraph graph, HighTierContext highTierContext) {
+    protected final void runInlining(StructuredGraph graph, HighTierContext highTierContext) {
         ResolvedJavaMethod method = graph.method();
         TruffleHostEnvironment env = TruffleHostEnvironment.get(method);
         if (env == null) {