diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 94c781d82c2..0c03a85d64c 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -1970,8 +1970,19 @@ void PhaseIdealLoop::do_unroll(IdealLoopTree *loop, Node_List &old_new, bool adj if (opaq == nullptr) { return; } - // Zero-trip test uses an 'opaque' node which is not shared. - assert(opaq->outcnt() == 1 && opaq->in(1) == limit, ""); + // Zero-trip test uses an 'opaque' node which is not shared, otherwise bail out. + if (opaq->outcnt() != 1 || opaq->in(1) != limit) { +#ifdef ASSERT + // In rare cases, loop cloning (as for peeling, for instance) can break this by replacing + // limit and the input of opaq by equivalent but distinct phis. + // Next IGVN should clean it up. Let's try to detect we are in such a case. + Unique_Node_List& worklist = loop->_phase->_igvn._worklist; + assert(C->major_progress(), "The operation that replaced limit and opaq->in(1) (e.g. peeling) should have set major_progress"); + assert(opaq->in(1)->is_Phi() && limit->is_Phi(), "Nodes limit and opaq->in(1) should have been replaced by PhiNodes by fix_data_uses from clone_loop."); + assert(worklist.member(opaq->in(1)) && worklist.member(limit), "Nodes limit and opaq->in(1) differ and should have been recorded for IGVN."); +#endif + return; + } } C->set_major_progress(); diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index b6c4d1b8574..54cf3e31266 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -4765,6 +4765,24 @@ void PhaseIdealLoop::eliminate_useless_zero_trip_guard() { Node* opaque = head->is_canonical_loop_entry(); if (opaque != nullptr) { useful_zero_trip_guard_opaques_nodes.push(opaque); +#ifdef ASSERT + // See PhaseIdealLoop::do_unroll + // This property is required in do_unroll, but it may not hold after cloning a loop. + // In such a case, we bail out from unrolling, and rely on IGVN to clean up the graph. + // We are here before loop cloning (before iteration_split), so if this property + // does not hold, it must come from the previous round of loop optimizations, meaning + // that IGVN failed to clean it: we will catch that here. + // On the other hand, if this assert passes, a bailout in do_unroll means that + // this property was broken in the current round of loop optimization (between here + // and do_unroll), so we give a chance to IGVN to make the property true again. + if (head->is_main_loop()) { + assert(opaque->outcnt() == 1, "opaque node should not be shared"); + assert(opaque->in(1) == head->limit(), "After IGVN cleanup, input of opaque node must be the limit."); + } + if (head->is_post_loop()) { + assert(opaque->outcnt() == 1, "opaque node should not be shared"); + } +#endif } } } diff --git a/test/hotspot/jtreg/compiler/loopopts/TooStrictAssertForUnrollAfterPeeling.java b/test/hotspot/jtreg/compiler/loopopts/TooStrictAssertForUnrollAfterPeeling.java new file mode 100644 index 00000000000..edfab8822b5 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/TooStrictAssertForUnrollAfterPeeling.java @@ -0,0 +1,194 @@ +/* + * 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 8361608 + * @summary crash during unrolling in some rare cases where loop cloning + * (typically from peeling) breaks an invariant in do_unroll + * + * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions + * -XX:+StressLoopPeeling + * -XX:CompileCommand=compileonly,compiler.loopopts.TooStrictAssertForUnrollAfterPeeling::test1 + * -XX:-TieredCompilation + * -Xbatch + * -XX:PerMethodTrapLimit=0 + * compiler.loopopts.TooStrictAssertForUnrollAfterPeeling + * + * @run main/othervm -XX:CompileOnly=compiler.loopopts.TooStrictAssertForUnrollAfterPeeling::test2 + * -XX:-TieredCompilation + * -Xbatch + * -XX:PerMethodTrapLimit=0 + * -XX:CompileCommand=inline,compiler.loopopts.TooStrictAssertForUnrollAfterPeeling::foo2 + * compiler.loopopts.TooStrictAssertForUnrollAfterPeeling + * + * @run main/othervm -XX:CompileOnly=compiler.loopopts.TooStrictAssertForUnrollAfterPeeling::test3 + * -XX:-TieredCompilation + * -Xbatch + * -XX:PerMethodTrapLimit=0 + * -XX:CompileCommand=inline,compiler.loopopts.TooStrictAssertForUnrollAfterPeeling::foo3 + * -XX:-RangeCheckElimination + * compiler.loopopts.TooStrictAssertForUnrollAfterPeeling + * + * @run main compiler.loopopts.TooStrictAssertForUnrollAfterPeeling + */ +package compiler.loopopts; + + +/* Cases 2 and 3 can use the additional flags + * -XX:+UnlockDiagnosticVMOptions + * -XX:-LoopMultiversioning + * -XX:-RangeCheckElimination + * -XX:-SplitIfBlocks + * -XX:-UseOnStackReplacement + * -XX:LoopMaxUnroll=2 + * to disable more optimizations and give a simpler graph, while still reproducing. It can be useful to debug, investigate... + */ +public class TooStrictAssertForUnrollAfterPeeling { + static int iArr[] = new int[400]; + static boolean flag; + + public static void main(String[] args) { + run1(); + run2(); + run3(); + } + + // Case 1 + + public static void run1() { + for (int i = 1; i < 1000; i++) { + test1(); + } + } + + static long test1() { + int s = 0; + int iArr[] = new int[400]; + for (int i = 0; i < 70; i++) {} + + for (int i = 0; i < 36; i++) { + for (int j = 0; j < 3; j++) { + s += iArr[0] = 7; + if (s != 0) { + return s + foo1(iArr); + } + } + } + return 0; + } + + public static long foo1(int[] a) { + long sum = 0; + for (int i = 0; i < a.length; i++) { + sum += a[i]; + } + return sum; + } + + // Case 2 + + public static void run2() { + for (int i = 1; i < 10000; i++) { + test2(); + } + } + + // Lx: Optimized in loop opts round x. + + static int test2() { + int x = 5; + for (int i = 1; i < 37; i++) { // L3: Peeled + for (int a = 0; a < 2; a++) { // L2: Max unrolled + for (int b = 0; b < 300; b++) { + } // L1: Empty -> removed + } + int j = 1; + x *= 12; + while (++j < 5) { // L1: Max unrolled: peel + unroll + iArr[0] += 2; + if (iArr[0] > 0) { + // foo(): everything outside loop. + return foo2(iArr); + } + } + } + return 3; + } + + public static int foo2(int[] a) { + int sum = 0; + for (int i = 0; i < a.length; i++) { // L2: Pre/main/post, L3: Unrolled -> hit assert! + for (int j = 0; j < 34; j++) { + } // L1: Empty -> removed + if (flag) { + // Ensure not directly unrolled in L2 but only in L3. + return 3; + } + sum += a[i]; + } + return sum; + } + + public static void run3() { + for (int i = 1; i < 10000; i++) { + test3(); + } + } + + // Case 3 + + static int test3() { + int x = 5; + for (int i = 1; i < 37; i++) { // L3: Peeled + for (int a = 0; a < 2; a++) { // L2: Max unrolled + for (int b = 0; b < 300; b++) { + } // L1: Empty -> removed + } + int j = 1; + x *= 12; + while (++j < 5) { // L1: Max unrolled: peel + unroll + iArr[0] += 2; + if (iArr[0] > 0) { + // foo(): everything outside loop. + return foo3(iArr, x); + } + } + } + return 3; + } + + public static int foo3(int[] a, int limit) { + int sum = 0; + for (int i = 0; i < limit; i++) { // L2: Pre/main/post, L3: Unrolled -> hit assert! + for (int j = 0; j < 34; j++) { + } // L1: Empty -> removed + if (flag) { + // Ensure not directly unrolled in L2 but only in L3. + return 3; + } + sum += a[i]; + } + return sum; + } +}