8348572: C2 compilation asserts due to unexpected irreducible loop

Reviewed-by: thartmann, kvn

src/hotspot/share/opto/c2_globals.hpp

@@ -633,6 +633,9 @@
   develop(bool, VerifyLoopOptimizations, false,                             \
           "verify major loop optimizations")                                \
                                                                             \
+  develop(bool, VerifyNoNewIrreducibleLoops, false,                         \
+          "Verify that no new irreducible loops are created after parsing") \
+                                                                            \
   product(bool, ProfileDynamicTypes, true, DIAGNOSTIC,                      \
           "do extra type profiling and use it more aggressively")           \
                                                                             \

src/hotspot/share/opto/cfgnode.cpp

@@ -436,15 +436,18 @@ bool RegionNode::are_all_nodes_in_infinite_subgraph(Unique_Node_List& worklist)
 
 void RegionNode::set_loop_status(RegionNode::LoopStatus status) {
   assert(loop_status() == RegionNode::LoopStatus::NeverIrreducibleEntry, "why set our status again?");
+  assert(status != RegionNode::LoopStatus::MaybeIrreducibleEntry || !is_Loop(), "LoopNode is never irreducible entry.");
   _loop_status = status;
 }
 
-#ifdef ASSERT
-void RegionNode::verify_can_be_irreducible_entry() const {
-  assert(loop_status() == RegionNode::LoopStatus::MaybeIrreducibleEntry, "must be marked irreducible");
-  assert(!is_Loop(), "LoopNode cannot be irreducible loop entry");
+// A Region can only be an irreducible entry if:
+// - It is marked as "maybe irreducible entry". Any other loop status would guarantee
+//   that it is never an irreducible loop entry.
+// - And it is not a LoopNode, those are guaranteed to be reducible loop entries.
+bool RegionNode::can_be_irreducible_entry() const {
+  return loop_status() == RegionNode::LoopStatus::MaybeIrreducibleEntry &&
+         !is_Loop();
 }
-#endif //ASSERT
 
 void RegionNode::try_clean_mem_phis(PhaseIterGVN* igvn) {
   // Incremental inlining + PhaseStringOpts sometimes produce:

src/hotspot/share/opto/cfgnode.hpp

@@ -119,7 +119,7 @@ public:
 #endif //ASSERT
   LoopStatus loop_status() const { return _loop_status; };
   void set_loop_status(LoopStatus status);
-  DEBUG_ONLY(void verify_can_be_irreducible_entry() const;)
+  bool can_be_irreducible_entry() const;
 
   virtual int Opcode() const;
   virtual uint size_of() const { return sizeof(*this); }

src/hotspot/share/opto/loopnode.cpp

@@ -5631,14 +5631,23 @@ int PhaseIdealLoop::build_loop_tree_impl(Node* n, int pre_order) {
       // l is irreducible: we just found a second entry m.
       _has_irreducible_loops = true;
       RegionNode* secondary_entry = m->as_Region();
-      DEBUG_ONLY(secondary_entry->verify_can_be_irreducible_entry();)
+
+      if (!secondary_entry->can_be_irreducible_entry()) {
+        assert(!VerifyNoNewIrreducibleLoops, "A new irreducible loop was created after parsing.");
+        C->record_method_not_compilable("A new irreducible loop was created after parsing.");
+        return pre_order;
+      }
 
       // Walk up the loop-tree, mark all loops that are already post-visited as irreducible
       // Since m is a secondary entry to them all.
       while( is_postvisited(l->_head) ) {
         l->_irreducible = 1; // = true
         RegionNode* head = l->_head->as_Region();
-        DEBUG_ONLY(head->verify_can_be_irreducible_entry();)
+        if (!head->can_be_irreducible_entry()) {
+          assert(!VerifyNoNewIrreducibleLoops, "A new irreducible loop was created after parsing.");
+          C->record_method_not_compilable("A new irreducible loop was created after parsing.");
+          return pre_order;
+        }
         l = l->_parent;
         // Check for bad CFG here to prevent crash, and bailout of compile
         if (l == nullptr) {

test/hotspot/jtreg/compiler/loopopts/TestSplitIfNewIrreducibleLoop.java

@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2025, 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
+ * 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 8348572
+ * @summary Tests where new irreducible loop is introduced by split_if.
+ *
+ * @run driver compiler.loopopts.TestSplitIfNewIrreducibleLoop
+ *
+ * @run main/othervm -Xcomp -XX:PerMethodTrapLimit=0
+ *                   -XX:CompileCommand=compileonly,compiler.loopopts.TestSplitIfNewIrreducibleLoop::test
+ *                   compiler.loopopts.TestSplitIfNewIrreducibleLoop
+ */
+
+package compiler.loopopts;
+
+public class TestSplitIfNewIrreducibleLoop {
+
+    static class A {
+        public A parent;
+    }
+
+    static class B extends A {}
+
+    public static void main(String[] args) {
+        // Instantiate one each: classes are loaded.
+        A a = new A();
+        B b = new B();
+        test(b);
+    }
+
+    static int test(A parent) {
+        do {
+            if (parent instanceof B b) { return 1; }
+            if (parent != null) { parent = parent.parent; }
+            if (parent == null) { return 0; }
+        } while (true);
+    }
+
+    // Before split_if it looks like this (the instanceof check has already been partial peeled):
+    //
+    // if (parent instanceof B b) { return 1; }
+    // do {
+    //     if (parent != null) { parent = parent.parent; }
+    //     if (parent == null) { return 0; }
+    //     if (parent instanceof B b) { return 1; }
+    // } while (true);
+    //
+    //
+    // Now, we want to split_if the first if in the loop body, like this:
+    //
+    // if (parent instanceof B b) { return 1; }
+    // if (parent != null) { goto LOOP2; } else { goto LOOP1; }
+    // do {
+    //     :LOOP1
+    //     parent = parent.parent;
+    //     :LOOP2
+    //     if (parent == null) { return 0; }
+    //     if (parent instanceof B b) { return 1; }
+    //     if (parent != null) { goto LOOP2; } else { goto LOOP1; }
+    // } while (true);
+    //
+    // As the comment in ifnode.cpp / split_if says: we know that on the backedge
+    // "parent" cannot be null, and so we would be able to split the if through
+    // the region, and on the backedge it would constant fold away, and we would
+    // only have to check the split if in the loop entry.
+    //
+    // Problem: we have introduced an irreducible loop!
+}
+