From d358f5f4a44aacf2d79ccdb3e362ce8ed571f6da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20H=C3=A4ssig?= Date: Wed, 2 Apr 2025 06:48:06 +0000 Subject: [PATCH] 8347449: C2: UseLoopPredicate off should also turn UseProfiledLoopPredicate off Reviewed-by: chagedorn, epeter --- src/hotspot/share/opto/c2_globals.hpp | 8 +- src/hotspot/share/opto/graphKit.cpp | 6 +- src/hotspot/share/opto/idealKit.cpp | 14 ++- src/hotspot/share/opto/loopnode.cpp | 18 ++-- src/hotspot/share/runtime/arguments.cpp | 7 ++ .../compiler/lib/ir_framework/IRNode.java | 22 +++++ .../TestDisabledLoopPredicates.java | 95 +++++++++++++++++++ 7 files changed, 148 insertions(+), 22 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/predicates/TestDisabledLoopPredicates.java diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp index 05b34bade43..ad88554fedd 100644 --- a/src/hotspot/share/opto/c2_globals.hpp +++ b/src/hotspot/share/opto/c2_globals.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 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 @@ -238,7 +238,7 @@ "Force counted loops to keep a safepoint") \ \ product(bool, UseLoopPredicate, true, \ - "Generate a predicate to select fast/slow loop versions") \ + "Move checks with uncommon trap out of loops.") \ \ develop(bool, TraceLoopPredicate, false, \ "Trace generation of loop predicates") \ @@ -785,7 +785,9 @@ range(0, max_juint) \ \ product(bool, UseProfiledLoopPredicate, true, \ - "Move predicates out of loops based on profiling data") \ + "Move checks with an uncommon trap out of loops based on " \ + "profiling data. " \ + "Requires UseLoopPredicate to be turned on (default).") \ \ develop(uintx, StressLongCountedLoop, 0, \ "if > 0, convert int counted loops to long counted loops" \ diff --git a/src/hotspot/share/opto/graphKit.cpp b/src/hotspot/share/opto/graphKit.cpp index bf6c4df626b..e10a87171fc 100644 --- a/src/hotspot/share/opto/graphKit.cpp +++ b/src/hotspot/share/opto/graphKit.cpp @@ -4037,9 +4037,9 @@ void GraphKit::add_parse_predicate(Deoptimization::DeoptReason reason, const int void GraphKit::add_parse_predicates(int nargs) { if (UseLoopPredicate) { add_parse_predicate(Deoptimization::Reason_predicate, nargs); - } - if (UseProfiledLoopPredicate) { - add_parse_predicate(Deoptimization::Reason_profile_predicate, nargs); + if (UseProfiledLoopPredicate) { + add_parse_predicate(Deoptimization::Reason_profile_predicate, nargs); + } } add_parse_predicate(Deoptimization::Reason_auto_vectorization_check, nargs); // Loop Limit Check Predicate should be near the loop. diff --git a/src/hotspot/share/opto/idealKit.cpp b/src/hotspot/share/opto/idealKit.cpp index 528a0e29411..8c26e1cc39d 100644 --- a/src/hotspot/share/opto/idealKit.cpp +++ b/src/hotspot/share/opto/idealKit.cpp @@ -163,14 +163,12 @@ void IdealKit::end_if() { // onto the stack. void IdealKit::loop(GraphKit* gkit, int nargs, IdealVariable& iv, Node* init, BoolTest::mask relop, Node* limit, float prob, float cnt) { assert((state() & (BlockS|LoopS|IfThenS|ElseS)), "bad state for new loop"); - if (UseLoopPredicate) { - // Sync IdealKit and graphKit. - gkit->sync_kit(*this); - // Add Parse Predicates. - gkit->add_parse_predicates(nargs); - // Update IdealKit memory. - sync_kit(gkit); - } + // Sync IdealKit and graphKit. + gkit->sync_kit(*this); + // Add Parse Predicates. + gkit->add_parse_predicates(nargs); + // Update IdealKit memory. + sync_kit(gkit); set(iv, init); Node* head = make_label(1); bind(head); diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index 92f678030ca..b0ed29d8567 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -1087,9 +1087,9 @@ bool PhaseIdealLoop::create_loop_nest(IdealLoopTree* loop, Node_List &old_new) { if (UseLoopPredicate) { add_parse_predicate(Deoptimization::Reason_predicate, inner_head, outer_ilt, cloned_sfpt); - } - if (UseProfiledLoopPredicate) { - add_parse_predicate(Deoptimization::Reason_profile_predicate, inner_head, outer_ilt, cloned_sfpt); + if (UseProfiledLoopPredicate) { + add_parse_predicate(Deoptimization::Reason_profile_predicate, inner_head, outer_ilt, cloned_sfpt); + } } // We only want to use the auto-vectorization check as a trap once per bci. And @@ -4298,11 +4298,13 @@ void IdealLoopTree::dump_head() { if (predicates.loop_limit_check_predicate_block()->is_non_empty()) { tty->print(" limit_check"); } - if (UseProfiledLoopPredicate && predicates.profiled_loop_predicate_block()->is_non_empty()) { - tty->print(" profile_predicated"); - } - if (UseLoopPredicate && predicates.loop_predicate_block()->is_non_empty()) { - tty->print(" predicated"); + if (UseLoopPredicate) { + if (UseProfiledLoopPredicate && predicates.profiled_loop_predicate_block()->is_non_empty()) { + tty->print(" profile_predicated"); + } + if (predicates.loop_predicate_block()->is_non_empty()) { + tty->print(" predicated"); + } } if (_head->is_CountedLoop()) { CountedLoopNode *cl = _head->as_CountedLoop(); diff --git a/src/hotspot/share/runtime/arguments.cpp b/src/hotspot/share/runtime/arguments.cpp index 2876ecaedb8..0927e87c748 100644 --- a/src/hotspot/share/runtime/arguments.cpp +++ b/src/hotspot/share/runtime/arguments.cpp @@ -3797,6 +3797,13 @@ jint Arguments::apply_ergo() { } #endif // COMPILER2_OR_JVMCI +#ifdef COMPILER2 + if (!FLAG_IS_DEFAULT(UseLoopPredicate) && !UseLoopPredicate && UseProfiledLoopPredicate) { + warning("Disabling UseProfiledLoopPredicate since UseLoopPredicate is turned off."); + FLAG_SET_ERGO(UseProfiledLoopPredicate, false); + } +#endif // COMPILER2 + if (log_is_enabled(Info, perf, class, link)) { if (!UsePerfData) { warning("Disabling -Xlog:perf+class+link since UsePerfData is turned off."); diff --git a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java index 6d4f2508d8a..46bd48eefb2 100644 --- a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java +++ b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java @@ -1539,6 +1539,21 @@ public class IRNode { CompilePhase.BEFORE_MATCHING)); } + public static final String LOOP_PARSE_PREDICATE = PREFIX + "LOOP_PARSE_PREDICATE" + POSTFIX; + static { + parsePredicateNodes(LOOP_PARSE_PREDICATE, "Loop"); + } + + public static final String LOOP_LIMIT_CHECK_PARSE_PREDICATE = PREFIX + "LOOP_LIMIT_CHECK_PARSE_PREDICATE" + POSTFIX; + static { + parsePredicateNodes(LOOP_LIMIT_CHECK_PARSE_PREDICATE, "Loop Limit Check"); + } + + public static final String PROFILED_LOOP_PARSE_PREDICATE = PREFIX + "PROFILED_LOOP_PARSE_PREDICATE" + POSTFIX; + static { + parsePredicateNodes(PROFILED_LOOP_PARSE_PREDICATE, "Profiled Loop"); + } + public static final String PREDICATE_TRAP = PREFIX + "PREDICATE_TRAP" + POSTFIX; static { trapNodes(PREDICATE_TRAP, "predicate"); @@ -2761,6 +2776,13 @@ public class IRNode { beforeMatching(irNodePlaceholder, regex); } + private static void parsePredicateNodes(String irNodePlaceholder, String label) { + String regex = START + "ParsePredicate" + MID + "#" + label + "[ ]*!jvms:" + END; + IR_NODE_MAPPINGS.put(irNodePlaceholder, new SinglePhaseRangeEntry(CompilePhase.AFTER_PARSING, regex, + CompilePhase.AFTER_PARSING, + CompilePhase.PHASEIDEALLOOP_ITERATIONS)); + } + private static void loadOfNodes(String irNodePlaceholder, String irNodeRegex) { String regex = START + irNodeRegex + MID + "@\\S*" + IS_REPLACED + LOAD_OF_CLASS_POSTFIX; beforeMatching(irNodePlaceholder, regex); diff --git a/test/hotspot/jtreg/compiler/predicates/TestDisabledLoopPredicates.java b/test/hotspot/jtreg/compiler/predicates/TestDisabledLoopPredicates.java new file mode 100644 index 00000000000..c74e22cb68a --- /dev/null +++ b/test/hotspot/jtreg/compiler/predicates/TestDisabledLoopPredicates.java @@ -0,0 +1,95 @@ +/* + * 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. + * + */ + +package compiler.predicates; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; + +/* + * @test + * @bug 8347449 + * @summary Test that profiled loop predicates are turned off if loop predicates are turned off + * @library /test/lib / + * @run driver compiler.predicates.TestDisabledLoopPredicates + */ + +public class TestDisabledLoopPredicates { + static final int SIZE = 100; + static final int MIN = 3; + + public static void main(String[] args) { + TestFramework.runWithFlags("-XX:+UseLoopPredicate", + "-XX:+UseProfiledLoopPredicate"); + TestFramework.runWithFlags("-XX:-UseLoopPredicate"); + TestFramework.runWithFlags("-XX:-UseProfiledLoopPredicate"); + } + + @Run(test = "test") + private static void check() { + int res = test(true); + Asserts.assertEQ(res, ((SIZE - 1) * SIZE - MIN * (MIN + 1)) / 2); + } + + @DontInline + private static void blackhole(int i) { + } + + @DontInline + private static int[] getArr() { + int[] arr = new int[SIZE]; + for (int i = 0; i < SIZE; i++) { + arr[i] = i; + } + + return arr; + } + + @Test + @IR(counts = { IRNode.LOOP_PARSE_PREDICATE, "1", + IRNode.PROFILED_LOOP_PARSE_PREDICATE, "1" }, + applyIfAnd = { "UseLoopPredicate", "true", + "UseProfiledLoopPredicate", "true" }) + @IR(failOn = { IRNode.LOOP_PARSE_PREDICATE, + IRNode.PROFILED_LOOP_PARSE_PREDICATE }, + applyIf = { "UseLoopPredicate", "false" }) + @IR(counts = { IRNode.LOOP_PARSE_PREDICATE, "1" }, + failOn = { IRNode.PROFILED_LOOP_PARSE_PREDICATE }, + applyIfAnd = { "UseLoopPredicate", "true", + "UseProfiledLoopPredicate", "false" }) + public static int test(boolean cond) { + int[] arr = getArr(); + int sum = 0; + for (int i = 0; i < arr.length; i++) { + if (cond) { + if (arr[i] > MIN) { + sum += arr[i]; + } + } + blackhole(arr[i]); + } + + return sum; + } +} \ No newline at end of file