8349139: C2: Div looses dependency on condition that guarantees divisor not zero in counted loop

Reviewed-by: chagedorn, epeter, qamai

Author: rwestrel

src/hotspot/share/opto/loopTransform.cpp

@@ -1299,6 +1299,22 @@ Node *PhaseIdealLoop::clone_up_backedge_goo(Node *back_ctrl, Node *preheader_ctr
   return n;
 }
 
+// When a counted loop is created, the loop phi type may be narrowed down. As a consequence, the control input of some
+// nodes may be cleared: in particular in the case of a division by the loop iv, the Div node would lose its control
+// dependency if the loop phi is never zero. After pre/main/post loops are created (and possibly unrolling), the
+// loop phi type is only correct if the loop is indeed reachable: there's an implicit dependency between the loop phi
+// type and the zero trip guard for the main or post loop and as a consequence a dependency between the Div node and the
+// zero trip guard. This makes the dependency explicit by adding a CastII for the loop entry input of the loop phi. If
+// the backedge of the main or post loop is removed, a Div node won't be able to float above the zero trip guard of the
+// loop and can't execute even if the loop is not reached.
+void PhaseIdealLoop::cast_incr_before_loop(Node* incr, Node* ctrl, CountedLoopNode* loop) {
+  Node* castii = new CastIINode(ctrl, incr, TypeInt::INT, ConstraintCastNode::UnconditionalDependency);
+  register_new_node(castii, ctrl);
+  Node* phi = loop->phi();
+  assert(phi->in(LoopNode::EntryControl) == incr, "replacing wrong input?");
+  _igvn.replace_input_of(phi, LoopNode::EntryControl, castii);
+}
+
 #ifdef ASSERT
 void PhaseIdealLoop::ensure_zero_trip_guard_proj(Node* node, bool is_main_loop) {
   assert(node->is_IfProj(), "must be the zero trip guard If node");
@@ -1453,6 +1469,8 @@ void PhaseIdealLoop::insert_pre_post_loops(IdealLoopTree *loop, Node_List &old_n
   initialize_assertion_predicates_for_main_loop(pre_head, main_head, first_node_index_in_pre_loop_body,
                                                 last_node_index_in_pre_loop_body,
                                                 DEBUG_ONLY(last_node_index_from_backedge_goo COMMA) old_new);
+  // CastII for the main loop:
+  cast_incr_before_loop(pre_incr, min_taken, main_head);
 
   // Step B4: Shorten the pre-loop to run only 1 iteration (for now).
   // RCE and alignment may change this later.
@@ -1593,7 +1611,7 @@ void PhaseIdealLoop::insert_vector_post_loop(IdealLoopTree *loop, Node_List &old
 // Insert post loops.  Add a post loop to the given loop passed.
 Node *PhaseIdealLoop::insert_post_loop(IdealLoopTree* loop, Node_List& old_new,
                                        CountedLoopNode* main_head, CountedLoopEndNode* main_end,
-                                       Node*& incr, Node* limit, CountedLoopNode*& post_head) {
+                                       Node* incr, Node* limit, CountedLoopNode*& post_head) {
   IfNode* outer_main_end = main_end;
   IdealLoopTree* outer_loop = loop;
   if (main_head->is_strip_mined()) {
@@ -1682,6 +1700,7 @@ Node *PhaseIdealLoop::insert_post_loop(IdealLoopTree* loop, Node_List& old_new,
 
   DEBUG_ONLY(ensure_zero_trip_guard_proj(post_head->in(LoopNode::EntryControl), false);)
   initialize_assertion_predicates_for_post_loop(main_head, post_head, first_node_index_in_cloned_loop_body);
+  cast_incr_before_loop(zer_opaq->in(1), zer_taken, post_head);
   return new_main_exit;
 }
 
@@ -1777,8 +1796,8 @@ void PhaseIdealLoop::create_assertion_predicates_at_main_or_post_loop(CountedLoo
 // to do because these control dependent nodes on the old target loop entry created by clone_up_backedge_goo() were
 // pinned on the loop backedge before. The Assertion Predicates are not control dependent on these nodes in any way.
 void PhaseIdealLoop::rewire_old_target_loop_entry_dependency_to_new_entry(
-    LoopNode* target_loop_head, const Node* old_target_loop_entry,
-    const uint node_index_before_new_assertion_predicate_nodes) {
+  CountedLoopNode* target_loop_head, const Node* old_target_loop_entry,
+  const uint node_index_before_new_assertion_predicate_nodes) {
   Node* new_main_loop_entry = target_loop_head->skip_strip_mined()->in(LoopNode::EntryControl);
   if (new_main_loop_entry == old_target_loop_entry) {
     // No Assertion Predicates added.
@@ -1788,6 +1807,7 @@ void PhaseIdealLoop::rewire_old_target_loop_entry_dependency_to_new_entry(
   for (DUIterator_Fast imax, i = old_target_loop_entry->fast_outs(imax); i < imax; i++) {
     Node* out = old_target_loop_entry->fast_out(i);
     if (!out->is_CFG() && out->_idx < node_index_before_new_assertion_predicate_nodes) {
+      assert(out != target_loop_head->init_trip(), "CastII on loop entry?");
       _igvn.replace_input_of(out, 0, new_main_loop_entry);
       set_ctrl(out, new_main_loop_entry);
       --i;
@@ -2677,7 +2697,8 @@ void PhaseIdealLoop::do_range_check(IdealLoopTree* loop) {
         if (b_test._test == BoolTest::lt) { // Range checks always use lt
           // The underflow and overflow limits: 0 <= scale*I+offset < limit
           add_constraint(stride_con, lscale_con, offset, zero, limit, next_limit_ctrl, &pre_limit, &main_limit);
-          Node* init = cl->init_trip();
+          Node* init = cl->uncasted_init_trip(true);
+
           Node* opaque_init = new OpaqueLoopInitNode(C, init);
           register_new_node(opaque_init, loop_entry);
 

src/hotspot/share/opto/loopnode.cpp

@@ -6058,6 +6058,24 @@ CountedLoopEndNode* CountedLoopNode::find_pre_loop_end() {
   return pre_end;
 }
 
+Node* CountedLoopNode::uncasted_init_trip(bool uncast) {
+  Node* init = init_trip();
+  if (uncast && init->is_CastII()) {
+    // skip over the cast added by PhaseIdealLoop::cast_incr_before_loop() when pre/post/main loops are created because
+    // it can get in the way of type propagation. For instance, the index tested by an Assertion Predicate, if the cast
+    // is not skipped over, could be (1):
+    // (AddI (CastII (AddI pre_loop_iv -2) int) 1)
+    // while without the cast, it is (2):
+    // (AddI (AddI pre_loop_iv -2) 1)
+    // which is be transformed to (3):
+    // (AddI pre_loop_iv -1)
+    // The compiler may be able to constant fold the Assertion Predicate condition for (3) but not (1)
+    assert(init->as_CastII()->carry_dependency() && skip_assertion_predicates_with_halt() == init->in(0), "casted iv phi from pre loop expected");
+    init = init->in(1);
+  }
+  return init;
+}
+
 //------------------------------get_late_ctrl----------------------------------
 // Compute latest legal control.
 Node *PhaseIdealLoop::get_late_ctrl( Node *n, Node *early ) {

src/hotspot/share/opto/loopnode.hpp

@@ -365,6 +365,8 @@ class CountedLoopNode : public BaseCountedLoopNode {
   Node* is_canonical_loop_entry();
   CountedLoopEndNode* find_pre_loop_end();
 
+  Node* uncasted_init_trip(bool uncasted);
+
 #ifndef PRODUCT
   virtual void dump_spec(outputStream *st) const;
 #endif
@@ -973,6 +975,8 @@ class PhaseIdealLoop : public PhaseTransform {
     return ctrl;
   }
 
+  void cast_incr_before_loop(Node* incr, Node* ctrl, CountedLoopNode* loop);
+
 #ifdef ASSERT
   static void ensure_zero_trip_guard_proj(Node* node, bool is_main_loop);
 #endif
@@ -998,7 +1002,7 @@ class PhaseIdealLoop : public PhaseTransform {
                                                         CountedLoopNode* target_loop_head,
                                                         const NodeInLoopBody& _node_in_loop_body,
                                                         bool kill_old_template);
-  void rewire_old_target_loop_entry_dependency_to_new_entry(LoopNode* target_loop_head,
+  void rewire_old_target_loop_entry_dependency_to_new_entry(CountedLoopNode* target_loop_head,
                                                             const Node* old_target_loop_entry,
                                                             uint node_index_before_new_assertion_predicate_nodes);
   void insert_loop_limit_check_predicate(ParsePredicateSuccessProj* loop_limit_check_parse_proj, Node* cmp_limit,
@@ -1352,7 +1356,7 @@ class PhaseIdealLoop : public PhaseTransform {
   // Add post loop after the given loop.
   Node *insert_post_loop(IdealLoopTree* loop, Node_List& old_new,
                          CountedLoopNode* main_head, CountedLoopEndNode* main_end,
-                         Node*& incr, Node* limit, CountedLoopNode*& post_head);
+                         Node* incr, Node* limit, CountedLoopNode*& post_head);
 
   // Add a vector post loop between a vector main loop and the current post loop
   void insert_vector_post_loop(IdealLoopTree *loop, Node_List &old_new);
@@ -1580,8 +1584,6 @@ class PhaseIdealLoop : public PhaseTransform {
   // Attempt to use a conditional move instead of a phi/branch
   Node *conditional_move( Node *n );
 
-  bool split_thru_phi_could_prevent_vectorization(Node* n, Node* n_blk);
-
   // Check for aggressive application of 'split-if' optimization,
   // using basic block level info.
   void  split_if_with_blocks     ( VectorSet &visited, Node_Stack &nstack);

src/hotspot/share/opto/loopopts.cpp

@@ -1089,25 +1089,6 @@ void PhaseIdealLoop::try_move_store_after_loop(Node* n) {
   }
 }
 
-// Split some nodes that take a counted loop phi as input at a counted
-// loop can cause vectorization of some expressions to fail
-bool PhaseIdealLoop::split_thru_phi_could_prevent_vectorization(Node* n, Node* n_blk) {
-  if (!n_blk->is_CountedLoop()) {
-    return false;
-  }
-
-  int opcode = n->Opcode();
-
-  if (opcode != Op_AndI &&
-      opcode != Op_MulI &&
-      opcode != Op_RotateRight &&
-      opcode != Op_RShiftI) {
-    return false;
-  }
-
-  return n->in(1) == n_blk->as_BaseCountedLoop()->phi();
-}
-
 //------------------------------split_if_with_blocks_pre-----------------------
 // Do the real work in a non-recursive function.  Data nodes want to be
 // cloned in the pre-order so they can feed each other nicely.
@@ -1194,10 +1175,6 @@ Node *PhaseIdealLoop::split_if_with_blocks_pre( Node *n ) {
     return n;
   }
 
-  if (split_thru_phi_could_prevent_vectorization(n, n_blk)) {
-    return n;
-  }
-
   // Check for having no control input; not pinned.  Allow
   // dominating control.
   if (n->in(0)) {

src/hotspot/share/opto/predicates.cpp

@@ -758,7 +758,7 @@ void AssertionPredicateIfCreator::create_halt_node(IfFalseNode* fail_proj, Ideal
 }
 
 OpaqueLoopInitNode* TemplateAssertionPredicateCreator::create_opaque_init(Node* new_control) const {
-  OpaqueLoopInitNode* opaque_init = new OpaqueLoopInitNode(_phase->C, _loop_head->init_trip());
+  OpaqueLoopInitNode* opaque_init = new OpaqueLoopInitNode(_phase->C, _loop_head->uncasted_init_trip(_loop_head->is_main_loop()));
   _phase->register_new_node(opaque_init, new_control);
   return opaque_init;
 }

test/hotspot/jtreg/compiler/controldependency/TestDivDependentOnMainLoopGuard.java

@@ -0,0 +1,73 @@
+/*
+ * 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 8349139
+ * @summary C2: Div looses dependency on condition that guarantees divisor not null in counted loop
+ * @library /test/lib /
+ * @run main/othervm -Xcomp -XX:-TieredCompilation -XX:CompileOnly=TestDivDependentOnMainLoopGuard::*
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+StressGCM -XX:StressSeed=35878193 TestDivDependentOnMainLoopGuard
+ * @run main/othervm -Xcomp -XX:-TieredCompilation -XX:CompileOnly=TestDivDependentOnMainLoopGuard::*
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+StressGCM TestDivDependentOnMainLoopGuard
+ * @run main/othervm -Xcomp -XX:CompileOnly=TestDivDependentOnMainLoopGuard::* TestDivDependentOnMainLoopGuard
+ */
+
+import jdk.test.lib.Utils;
+import java.util.Random;
+
+public class TestDivDependentOnMainLoopGuard {
+
+    public static final int N = 400;
+    private static final Random RANDOM = Utils.getRandomInstance();
+    public static final int stop = RANDOM.nextInt(0, 68);
+
+    public int iArrFld[]=new int[N];
+
+    public void mainTest(String[] strArr1, int otherPhi) {
+
+        int i=57657, i1=577, i2=6, i3=157, i4=12, i23=61271;
+        boolean bArr[]=new boolean[N];
+
+        for (i = 9; 379 > i; i++) {
+            i2 = 1;
+            do {
+                i1 <<= i3;
+            } while (++i2 < 68);
+            for (i23 = 68; i23 > stop; otherPhi=i23-1, i23--) {
+                bArr[i23 + 1] = true;
+                try {
+                    i1 = (-42360 / i23);
+                    iArrFld[i + 1] = otherPhi;
+                } catch (ArithmeticException a_e) {}
+            }
+        }
+    }
+
+    public static void main(String[] strArr) {
+        TestDivDependentOnMainLoopGuard _instance = new TestDivDependentOnMainLoopGuard();
+        for (int i = 0; i < 10; i++ ) {
+            _instance.mainTest(strArr, 0);
+        }
+    }
+}

test/hotspot/jtreg/compiler/controldependency/TestMainLoopNoBackedgeFloatingDiv.java

@@ -0,0 +1,58 @@
+/*
+ * 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 8349139
+ * @summary C2: Div looses dependency on condition that guarantees divisor not null in counted loop
+ * @run main/othervm -XX:-BackgroundCompilation TestMainLoopNoBackedgeFloatingDiv
+ */
+
+public class TestMainLoopNoBackedgeFloatingDiv {
+    private static int field;
+
+    public static void main(String[] args) {
+        for (int i = 0; i < 20_000; i++) {
+           test1(1000, 0, false);
+           test1Helper(1000, 0, false, false);
+        }
+       test1(1, 0, false);
+    }
+
+    private static int test1(int stop, int res, boolean alwaysTrueInMain) {
+        stop = Integer.max(stop, 1);
+        res = test1Helper(stop, res, alwaysTrueInMain, true);
+        return res;
+    }
+
+    private static int test1Helper(int stop, int res, boolean alwaysTrueInMain, boolean flag) {
+        for (int i = stop; i >= 1; i--) {
+            res = res / i;
+            if (alwaysTrueInMain) {
+                break;
+            }
+            alwaysTrueInMain = flag;
+        }
+        return res;
+    }
+}