diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp index d4b55ec2d8d12..caa0474c4755a 100644 --- a/src/hotspot/share/opto/c2_globals.hpp +++ b/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") \ \ diff --git a/src/hotspot/share/opto/cfgnode.cpp b/src/hotspot/share/opto/cfgnode.cpp index 14afecd2550df..01cbacd72f3f6 100644 --- a/src/hotspot/share/opto/cfgnode.cpp +++ b/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: diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index 1d287c2f43df8..899e3c9bb8586 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -119,7 +119,7 @@ class RegionNode : public Node { #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); } diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index 40c29384c4379..3329d4dadc6ac 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/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) { diff --git a/test/hotspot/jtreg/compiler/loopopts/TestSplitIfNewIrreducibleLoop.java b/test/hotspot/jtreg/compiler/loopopts/TestSplitIfNewIrreducibleLoop.java new file mode 100644 index 0000000000000..edf0a298214b2 --- /dev/null +++ b/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! +} +