diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index d04eb34acee..3b398b053a3 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -4165,35 +4165,44 @@ void IdealLoopTree::allpaths_check_safepts(VectorSet &visited, Node_List &stack) // backedge (arc 3->2). So it deletes the ncsfpt (non-call safepoint) // in block 2, _but_ this leaves the outer loop without a safepoint. // -// entry 0 -// | -// v -// outer 1,2 +->1 -// | | -// | v -// | 2<---+ ncsfpt in 2 -// |_/|\ | -// | v | -// inner 2,3 / 3 | call in 3 -// / | | -// v +--+ -// exit 4 +// entry 0 +// | +// v +// outer 1,2,4 +-> 1 +// | \ +// | v +// inner 2,3 | 2 <---+ ncsfpt in 2 +// | / \ | +// | v v | +// | 4 3 | call in 3 +// |_/ \ \_| +// | +// v +// exit 5 // +// This method maintains a list (_required_safept) of ncsfpts that must +// be protected for each loop. It only marks ncsfpts for prevervation, +// and does not actually delete any of them. // -// This method creates a list (_required_safept) of ncsfpt nodes that must -// be protected is created for each loop. When a ncsfpt maybe deleted, it -// is first looked for in the lists for the outer loops of the current loop. +// If some other method needs to delete a ncsfpt later, it will make sure +// the ncsfpt is not in the list of all outer loops of the current loop. +// See `PhaseIdealLoop::is_deleteable_safept`. // // The insights into the problem: -// A) counted loops are okay -// B) innermost loops are okay (only an inner loop can delete -// a ncsfpt needed by an outer loop) -// C) a loop is immune from an inner loop deleting a safepoint -// if the loop has a call on the idom-path -// D) a loop is also immune if it has a ncsfpt (non-call safepoint) on the -// idom-path that is not in a nested loop -// E) otherwise, an ncsfpt on the idom-path that is nested in an inner -// loop needs to be prevented from deletion by an inner loop +// A) Counted loops are okay (i.e. do not need to preserve ncsfpts), +// they will be handled in `IdealLoopTree::counted_loop` +// B) Innermost loops are okay because there's no inner loops that can +// delete their ncsfpts. Only outer loops need to mark safepoints for +// protection, because only loops further in can accidentally delete +// their ncsfpts +// C) If an outer loop has a call that's guaranteed to execute (on the +// idom-path), then that loop is okay. Because the call will always +// perform a safepoint poll, regardless of what safepoints are deleted +// from its inner loops +// D) Similarly, if an outer loop has a ncsfpt on the idom-path that isn't +// inside any nested loop, then that loop is okay +// E) Otherwise, if an outer loop's ncsfpt on the idom-path is nested in +// an inner loop, we need to prevent the inner loop from deleting it // // There are two analyses: // 1) The first, and cheaper one, scans the loop body from @@ -4227,10 +4236,10 @@ void IdealLoopTree::check_safepts(VectorSet &visited, Node_List &stack) { break; } else if (n->Opcode() == Op_SafePoint) { if (_phase->get_loop(n) == this) { + // We found a local ncsfpt. + // Continue searching for a call that is guaranteed to be a safepoint. has_local_ncsfpt = true; - break; - } - if (nonlocal_ncsfpt == nullptr) { + } else if (nonlocal_ncsfpt == nullptr) { nonlocal_ncsfpt = n; // save the one closest to the tail } } else { diff --git a/test/hotspot/jtreg/compiler/loopopts/TestRedundantSafepointElimination.java b/test/hotspot/jtreg/compiler/loopopts/TestRedundantSafepointElimination.java new file mode 100644 index 00000000000..69f86a2bf1d --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/TestRedundantSafepointElimination.java @@ -0,0 +1,177 @@ +/* + * Copyright (c) 2025 Alibaba Group Holding Limited. 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. + */ + +package compiler.loopopts; + +import compiler.lib.ir_framework.*; + +/* + * @test + * @bug 8347499 + * @summary Tests that redundant safepoints can be eliminated in loops. + * @library /test/lib / + * @run main compiler.loopopts.TestRedundantSafepointElimination + */ +public class TestRedundantSafepointElimination { + public static void main(String[] args) { + TestFramework.run(); + } + + static int someInts0 = 1; + static int someInts1 = 2; + + @DontInline + private void empty() {} + + // Test for a top-level counted loop. + // There should be a non-call safepoint in the loop. + @Test + @IR(counts = {IRNode.SAFEPOINT, "1"}, + phase = CompilePhase.AFTER_LOOP_OPTS) + public int testTopLevelCountedLoop() { + int sum = 0; + for (int i = 0; i < 100000; i++) { + sum += someInts0; + } + return sum; + } + + // Test for a top-level counted loop with a call that dominates + // the tail of the loop. + // There should be no safepoint in the loop, because the call is + // guaranteed to have a safepoint. + @Test + @IR(counts = {IRNode.SAFEPOINT, "0"}, + phase = CompilePhase.AFTER_LOOP_OPTS) + public int testTopLevelCountedLoopWithDomCall() { + int sum = 0; + for (int i = 0; i < 100000; i++) { + empty(); + sum += someInts0; + } + return sum; + } + + // Test for a top-level uncounted loop. + // There should be a non-call safepoint in the loop. + @Test + @IR(counts = {IRNode.SAFEPOINT, "1"}, + phase = CompilePhase.AFTER_LOOP_OPTS) + public int testTopLevelUncountedLoop() { + int sum = 0; + for (int i = 0; i < 100000; i += someInts0) { + sum += someInts1; + } + return sum; + } + + // Test for a top-level uncounted loop with a call that dominates + // the tail of the loop. + // There should be no safepoint in the loop, because the call is + // guaranteed to have a safepoint. + // Before JDK-8347499, this test would fail due to C2 exiting + // prematurely when encountering the local non-call safepoint. + @Test + @IR(counts = {IRNode.SAFEPOINT, "0"}, + phase = CompilePhase.AFTER_LOOP_OPTS) + public int testTopLevelUncountedLoopWithDomCall() { + int sum = 0; + for (int i = 0; i < 100000; i += someInts0) { + empty(); + sum += someInts1; + } + return sum; + } + + // Test for nested loops, where the outer loop has a call that + // dominates its own tail. + // There should be only one safepoint in the inner loop. + // Before JDK-8347499, this test would fail due to C2 exiting + // prematurely when encountering the local non-call safepoint. + @Test + @IR(counts = {IRNode.SAFEPOINT, "1"}, + phase = CompilePhase.AFTER_LOOP_OPTS) + public int testOuterLoopWithDomCall() { + int sum = 0; + for (int i = 0; i < 100; i += someInts0) { + empty(); + for (int j = 0; j < 1000; j++) { + sum += someInts1; + } + } + return sum; + } + + // Test for nested loops, where both the outer and inner loops + // have a call that dominates their tails. + // There should be no safepoint in both loops, because calls + // within them are guaranteed to have a safepoint. + // Before JDK-8347499, this test would fail due to C2 exiting + // prematurely when encountering the local non-call safepoint. + @Test + @IR(counts = {IRNode.SAFEPOINT, "0"}, + phase = CompilePhase.AFTER_LOOP_OPTS) + public int testOuterAndInnerLoopWithDomCall() { + int sum = 0; + for (int i = 0; i < 100; i += someInts0) { + empty(); + for (int j = 0; j < 1000; j++) { + empty(); + sum += someInts1; + } + } + return sum; + } + + // Test for nested loops, where the outer loop has a local + // non-call safepoint. + // There should be a safepoint in both loops. + @Test + @IR(counts = {IRNode.SAFEPOINT, "2"}, + phase = CompilePhase.AFTER_LOOP_OPTS) + public int testOuterLoopWithLocalNonCallSafepoint() { + int sum = 0; + for (int i = 0; i < 100; i += someInts0) { + for (int j = 0; j < 1000; j++) { + sum += someInts1; + } + } + return sum; + } + + // Test for nested loops, where the outer loop has no local + // safepoints, and it must preserve a non-local safepoint. + // There should be two safepoints in the loop tree. + @Test + @IR(counts = {IRNode.SAFEPOINT, "2"}, + phase = CompilePhase.AFTER_LOOP_OPTS) + public void testLoopNeedsToPreserveSafepoint() { + int i = 0, stop; + while (i < 1000) { + stop = i + 10; + while (i < stop) { + i += 1; + } + } + } +} diff --git a/test/micro/org/openjdk/bench/vm/compiler/LoopSafepoint.java b/test/micro/org/openjdk/bench/vm/compiler/LoopSafepoint.java new file mode 100644 index 00000000000..fa69550948e --- /dev/null +++ b/test/micro/org/openjdk/bench/vm/compiler/LoopSafepoint.java @@ -0,0 +1,131 @@ +/* + * Copyright (c) 2025 Alibaba Group Holding Limited. 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. + */ + +package org.openjdk.bench.vm.compiler; + +import org.openjdk.jmh.annotations.*; + +import java.util.concurrent.TimeUnit; + +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(value = 3) +@Warmup(iterations = 5, time = 2) +@Measurement(iterations = 5, time = 3) +@State(Scope.Thread) +public class LoopSafepoint { + static int someInts0 = 1; + static int someInts1 = 2; + + @CompilerControl(CompilerControl.Mode.DONT_INLINE) + private void empty() {} + + // All benchmarks below are in sync with the IR test + // `compiler.loopopts.TestRedundantSafepointElimination.java`. + // Check the comments in the IR test for more details. + + @Benchmark + public int topLevelCountedLoop() { + int sum = 0; + for (int i = 0; i < 100000; i++) { + sum += someInts0; + } + return sum; + } + + @Benchmark + public int topLevelCountedLoopWithDomCall() { + int sum = 0; + for (int i = 0; i < 100000; i++) { + empty(); + sum += someInts0; + } + return sum; + } + + @Benchmark + public int topLevelUncountedLoop() { + int sum = 0; + for (int i = 0; i < 100000; i += someInts0) { + sum += someInts1; + } + return sum; + } + + @Benchmark + public int topLevelUncountedLoopWithDomCall() { + int sum = 0; + for (int i = 0; i < 100000; i += someInts0) { + empty(); + sum += someInts1; + } + return sum; + } + + @Benchmark + public int outerLoopWithDomCall() { + int sum = 0; + for (int i = 0; i < 100; i += someInts0) { + empty(); + for (int j = 0; j < 1000; j++) { + sum += someInts1; + } + } + return sum; + } + + @Benchmark + public int outerAndInnerLoopWithDomCall() { + int sum = 0; + for (int i = 0; i < 100; i += someInts0) { + empty(); + for (int j = 0; j < 1000; j++) { + empty(); + sum += someInts1; + } + } + return sum; + } + + @Benchmark + public int outerLoopWithLocalNonCallSafepoint() { + int sum = 0; + for (int i = 0; i < 100; i += someInts0) { + for (int j = 0; j < 1000; j++) { + sum += someInts1; + } + } + return sum; + } + + @Benchmark + public void loopNeedsToPreserveSafepoint() { + int i = 0, stop; + while (i < 1000) { + stop = i + 10; + while (i < stop) { + i += 1; + } + } + } +}