8361702: C2: assert(is_dominator(compute_early_ctrl(limit, limit_ctrl), pre_end)) failed: node pinned on loop exit test?

Reviewed-by: epeter, chagedorn, mhaessig

Author: rwestrel

src/hotspot/share/opto/loopnode.hpp

@@ -1859,6 +1859,8 @@ class PhaseIdealLoop : public PhaseTransform {
 
   bool ctrl_of_all_uses_out_of_loop(const Node* n, Node* n_ctrl, IdealLoopTree* n_loop);
 
+  bool would_sink_below_pre_loop_exit(IdealLoopTree* n_loop, Node* ctrl);
+
   Node* compute_early_ctrl(Node* n, Node* n_ctrl);
 
   void try_sink_out_of_loop(Node* n);

src/hotspot/share/opto/loopopts.cpp

@@ -1710,18 +1710,20 @@ void PhaseIdealLoop::try_sink_out_of_loop(Node* n) {
         // Rewire control of n to right outside of the loop, regardless if its input(s) are later sunk or not.
         Node* maybe_pinned_n = n;
         Node* outside_ctrl = place_outside_loop(n_ctrl, loop_ctrl);
-        if (n->depends_only_on_test()) {
-          Node* pinned_clone = n->pin_array_access_node();
-          if (pinned_clone != nullptr) {
-            // Pin array access nodes: if this is an array load, it's going to be dependent on a condition that's not a
-            // range check for that access. If that condition is replaced by an identical dominating one, then an
-            // unpinned load would risk floating above its range check.
-            register_new_node(pinned_clone, n_ctrl);
-            maybe_pinned_n = pinned_clone;
-            _igvn.replace_node(n, pinned_clone);
+        if (!would_sink_below_pre_loop_exit(loop_ctrl, outside_ctrl)) {
+          if (n->depends_only_on_test()) {
+            Node* pinned_clone = n->pin_array_access_node();
+            if (pinned_clone != nullptr) {
+              // Pin array access nodes: if this is an array load, it's going to be dependent on a condition that's not a
+              // range check for that access. If that condition is replaced by an identical dominating one, then an
+              // unpinned load would risk floating above its range check.
+              register_new_node(pinned_clone, n_ctrl);
+              maybe_pinned_n = pinned_clone;
+              _igvn.replace_node(n, pinned_clone);
+            }
           }
+          _igvn.replace_input_of(maybe_pinned_n, 0, outside_ctrl);
         }
-        _igvn.replace_input_of(maybe_pinned_n, 0, outside_ctrl);
       }
     }
     if (n_loop != _ltree_root && n->outcnt() > 1) {
@@ -1929,6 +1931,21 @@ bool PhaseIdealLoop::ctrl_of_all_uses_out_of_loop(const Node* n, Node* n_ctrl, I
   return true;
 }
 
+// Sinking a node from a pre loop to its main loop pins the node between the pre and main loops. If that node is input
+// to a check that's eliminated by range check elimination, it becomes input to an expression that feeds into the exit
+// test of the pre loop above the point in the graph where it's pinned. This results in a broken graph. One way to avoid
+// it would be to not eliminate the check in the main loop. Instead, we prevent sinking of the node here so better code
+// is generated for the main loop.
+bool PhaseIdealLoop::would_sink_below_pre_loop_exit(IdealLoopTree* n_loop, Node* ctrl) {
+  if (n_loop->_head->is_CountedLoop() && n_loop->_head->as_CountedLoop()->is_pre_loop()) {
+    CountedLoopNode* pre_loop = n_loop->_head->as_CountedLoop();
+    if (is_dominator(pre_loop->loopexit(), ctrl)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 bool PhaseIdealLoop::ctrl_of_use_out_of_loop(const Node* n, Node* n_ctrl, IdealLoopTree* n_loop, Node* ctrl) {
   if (n->is_Load()) {
     ctrl = get_late_ctrl_with_anti_dep(n->as_Load(), n_ctrl, ctrl);
@@ -1940,14 +1957,8 @@ bool PhaseIdealLoop::ctrl_of_use_out_of_loop(const Node* n, Node* n_ctrl, IdealL
   if (n_loop->is_member(u_loop)) {
     return false; // Found use in inner loop
   }
-  // Sinking a node from a pre loop to its main loop pins the node between the pre and main loops. If that node is input
-  // to a check that's eliminated by range check elimination, it becomes input to an expression that feeds into the exit
-  // test of the pre loop above the point in the graph where it's pinned.
-  if (n_loop->_head->is_CountedLoop() && n_loop->_head->as_CountedLoop()->is_pre_loop()) {
-    CountedLoopNode* pre_loop = n_loop->_head->as_CountedLoop();
-    if (is_dominator(pre_loop->loopexit(), ctrl)) {
-      return false;
-    }
+  if (would_sink_below_pre_loop_exit(n_loop, ctrl)) {
+    return false;
   }
   return true;
 }

test/hotspot/jtreg/compiler/rangechecks/TestSunkRangeFromPreLoopRCE2.java

@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2025, Red Hat, Inc. 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8361702
+ * @summary C2: assert(is_dominator(compute_early_ctrl(limit, limit_ctrl), pre_end)) failed: node pinned on loop exit test?
+  *
+ * @run main/othervm -XX:CompileCommand=compileonly,*TestSunkRangeFromPreLoopRCE2*::* -Xbatch TestSunkRangeFromPreLoopRCE2
+ * @run main TestSunkRangeFromPreLoopRCE2
+ */
+
+public class TestSunkRangeFromPreLoopRCE2 {
+    static int iFld;
+    static long lArr[] = new long[400];
+
+    public static void main(String[] strArr) {
+        for (int i = 0; i < 1000; i++) {
+            test();
+        }
+    }
+
+    static void test() {
+        int iArr[] = new int[400];
+        for (int i = 8; i < 128; i++) {
+            for (int j = 209; j > 9; j--) {
+                switch ((j % 5) + 58) {
+                    case 58:
+                        iArr[i] = 194;
+                        break;
+                    case 59:
+                        iFld = 3;
+                    case 62:
+                    default:
+                        iArr[1] = i;
+                }
+                for (int k = 2; k > 1; --k) {
+                    lArr[k] -= iFld;
+                }
+            }
+        }
+    }
+}
+

test/hotspot/jtreg/compiler/rangechecks/TestSunkRangeFromPreLoopRCE3.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2025, Red Hat, Inc. 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8361702
+ * @summary C2: assert(is_dominator(compute_early_ctrl(limit, limit_ctrl), pre_end)) failed: node pinned on loop exit test?
+ * @requires vm.flavor == "server"
+ * @run main/othervm -XX:-BackgroundCompilation -XX:LoopUnrollLimit=100 -XX:-UseLoopPredicate -XX:-UseProfiledLoopPredicate TestSunkRangeFromPreLoopRCE3
+ */
+
+/**
+ * @test
+ * @bug 8361702
+ * @summary C2: assert(is_dominator(compute_early_ctrl(limit, limit_ctrl), pre_end)) failed: node pinned on loop exit test?
+ * @run main TestSunkRangeFromPreLoopRCE3
+ */
+
+import java.util.Arrays;
+
+public class TestSunkRangeFromPreLoopRCE3 {
+
+    static final int nbIterations = 100;
+
+    public static void main(String[] args) {
+        for (int i = 0; i < 20_000; i++) {
+            test1(0);
+            test1(0);
+        }
+    }
+
+    private static float test1(int k) {
+        float v = 0;
+        int j = 0;
+        int[] lengths = new int[2];
+        test1Helper(lengths);
+        int constantFoldedTo4AfterCCP = 2;
+        for (; constantFoldedTo4AfterCCP < 4; constantFoldedTo4AfterCCP *= 2);
+
+        int constantFoldedTo0AfterCCPAnd1RoundLoopOpts;
+        for (constantFoldedTo0AfterCCPAnd1RoundLoopOpts = 0; constantFoldedTo0AfterCCPAnd1RoundLoopOpts < 40; constantFoldedTo0AfterCCPAnd1RoundLoopOpts += constantFoldedTo4AfterCCP) {
+        }
+        constantFoldedTo0AfterCCPAnd1RoundLoopOpts -= 40;
+        for (int i = 0; i < nbIterations; i++) {
+            int arrayLength2 = Integer.max(Integer.min(lengths[j * k], 1000), 0);
+            float[] array = new float[arrayLength2];
+            v += array[(constantFoldedTo0AfterCCPAnd1RoundLoopOpts + 1) * i];
+
+            int arrayLength = Integer.max(Integer.min(lengths[k], 1000), 0);
+
+            v += arrayLength & constantFoldedTo0AfterCCPAnd1RoundLoopOpts;
+
+            j = 1;
+        }
+        return v;
+    }
+
+    private static void test1Helper(int[] lengths) {
+        lengths[0] = nbIterations+1;
+        lengths[1] = nbIterations+1;
+    }
+}