infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-01-28 12:09:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

8356084: C2: Data is wrongly rewired to Initialized Assertion Predicates instead of Template Assertion Predicates

Browse Source 
Reviewed-by: epeter, kvn
This commit is contained in:
Christian Hagedorn 2025-05-08 11:34:46 +00:00
parent b47b2062a2
commit ad07426fab
3 changed files with 93 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/hotspot/share/opto/predicates.cpp
View File

@ -984,15 +984,16 @@ void CreateAssertionPredicatesVisitor::visit(const TemplateAssertionPredicate& t
// Create an Initialized Assertion Predicate from the provided Template Assertion Predicate.
InitializedAssertionPredicate CreateAssertionPredicatesVisitor::initialize_from_template(
const TemplateAssertionPredicate& template_assertion_predicate, Node* new_control) const {
const TemplateAssertionPredicate& template_assertion_predicate, IfTrueNode* cloned_template_predicate_tail) const {
DEBUG_ONLY(template_assertion_predicate.verify();)
IfNode* template_head = template_assertion_predicate.head();
InitializedAssertionPredicateCreator initialized_assertion_predicate_creator(_phase);
InitializedAssertionPredicate initialized_assertion_predicate =
initialized_assertion_predicate_creator.create_from_template(template_head, new_control, _init, _stride);
initialized_assertion_predicate_creator.create_from_template(template_head, cloned_template_predicate_tail, _init,
_stride);
DEBUG_ONLY(initialized_assertion_predicate.verify();)
template_assertion_predicate.rewire_loop_data_dependencies(initialized_assertion_predicate.tail(),
template_assertion_predicate.rewire_loop_data_dependencies(cloned_template_predicate_tail,
_node_in_loop_body, _phase);
rewire_to_old_predicate_chain_head(initialized_assertion_predicate.tail());
return initialized_assertion_predicate;

2
src/hotspot/share/opto/predicates.hpp
View File

@ -1098,7 +1098,7 @@ class CreateAssertionPredicatesVisitor : public PredicateVisitor {
clone_template_and_replace_init_input(const TemplateAssertionPredicate& template_assertion_predicate) const;
InitializedAssertionPredicate initialize_from_template(const TemplateAssertionPredicate& template_assertion_predicate,
Node* new_control) const;
IfTrueNode* cloned_template_predicate_tail) const;
void rewire_to_old_predicate_chain_head(Node* initialized_assertion_predicate_success_proj) const;
public:

99
test/hotspot/jtreg/compiler/predicates/assertion/TestAssertionPredicates.java
View File

@ -179,6 +179,18 @@
* compiler.predicates.assertion.TestAssertionPredicates Stress
*/
/*
* @test id=StressXcompMaxUnroll0
* @key randomness
* @bug 8288981 8356084
* @requires vm.compiler2.enabled
* @run main/othervm -Xcomp -XX:+UnlockDiagnosticVMOptions -XX:+StressGCM -XX:+AbortVMOnCompilationFailure
* -XX:LoopMaxUnroll=0 -XX:+IgnoreUnrecognizedVMOptions -XX:-KillPathsReachableByDeadTypeNode
* -XX:CompileCommand=compileonly,compiler.predicates.assertion.TestAssertionPredicates::*
* -XX:CompileCommand=dontinline,compiler.predicates.assertion.TestAssertionPredicates::*
* compiler.predicates.assertion.TestAssertionPredicates StressXcompMaxUnroll0
*/
/*
* @test id=NoLoopPredicationXcomp
* @bug 8288981 8350577
@ -226,6 +238,7 @@ public class TestAssertionPredicates {
static boolean flagFalse, flagFalse2;
static int iFld = 34;
static int iFld2, iFld3;
static int two = 2;
static long lFld, lFldOne = 1;
static float fFld;
static short sFld;
@ -238,7 +251,7 @@ public class TestAssertionPredicates {
}
static Foo foo = new Foo();
static int fooArrSize = 10000001;
public static void main(String[] args) {
switch (args[0]) {
@ -315,7 +328,7 @@ public class TestAssertionPredicates {
test8308504No2();
}
}
case "DataUpdate" -> {
case "DataUpdate", "StressXcompMaxUnroll0" -> {
for (int i = 0; i < 10; i++) {
// The following tests create large arrays. Limit the number of invocations to reduce the time spent.
flag = !flag;
@ -323,6 +336,7 @@ public class TestAssertionPredicates {
testDataUpdateUnswitchUnroll();
testDataUpdateUnroll();
testDataUpdatePeelingUnroll();
testPeelingThreeTimesDataUpdate();
}
}
case "CloneDown" -> {
@ -1064,18 +1078,18 @@ public class TestAssertionPredicates {
for (int i = 0; i < limit; i++) {
fooArr[10000000 * i] = foo;
}
// 9) This loop is not optimized in any way because we have a call inside the loop. This loop is only required
// 10) This loop is not optimized in any way because we have a call inside the loop. This loop is only required
// to trigger a crash.
// 10) During GCM with StressGCM, we could schedule the LoadN from the main loop before checking if we should
// 11) During GCM with StressGCM, we could schedule the LoadN from the main loop before checking if we should
// enter the main loop. When 'flag' is true, we only have an array of size 20000001. We then perform
// the LoadN[3*10000000] and crash when the memory is unmapped.
for (float f = 0; f < 1.6f; f += 0.5f) {
// 2) Loop is unswitched
// 3) Both loop are peeled (we focus on one of those since both are almost identical except for the
// 4) Both loop are peeled (we focus on one of those since both are almost identical except for the
// unswitched condition):
// Peeled iteration [i = 0]
// Loop [i = 1..4, stride = 1]
// 4) Loop unroll policy now returns true.
// 5) Loop unroll policy now returns true.
// Peeled iteration [i = 0]
// - Loop is pre-main-posted
// Loop-pre[i = 1..4, stride = 1]
@ -1085,16 +1099,16 @@ public class TestAssertionPredicates {
// Loop-pre[i = 1..4, stride = 1]
// Loop-main[i = 2..4, stride = 2]
// Loop-post[i = 2..4, stride = 1]
// 5) During IGVN, we find that the backedge is never taken for main loop (we would over-iteratre) and it
// 6) During IGVN, we find that the backedge is never taken for main loop (we would over-iteratre) and it
// collapses to a single iteration.
// 6) After loop opts, the pre-loop is removed.
// 7) After loop opts, the pre-loop is removed.
for (int i = 0; i < limit; i++) {
// 1) Hoisted with a Hoisted Range Check Predicate
// 7) The 'i = 1' value is propagated to the single main loop iteration and we have the following
// 8) The 'i = 1' value is propagated to the single main loop iteration and we have the following
// fixed-index accesses:
// LoadN[2*10000000];
// LoadN[3*10000000];
// 8) Without explicitly pinning the LoadN from the main loop at the main loop entry (i.e. below the
// 9) Without explicitly pinning the LoadN from the main loop at the main loop entry (i.e. below the
// zero trip guard), they are still pinned below the Hoisted Range Check Predicate before the loop.
fooArr[i * 10000000].iFld += 34;
if (flagFalse) {
@ -1327,11 +1341,74 @@ public class TestAssertionPredicates {
}
}
// -Xcomp -XX:LoopMaxUnroll=0 -XX:+StressGCM -XX:CompileCommand=compileonly,Test*::*
private static void testPeelingThreeTimesDataUpdate() {
Foo[] fooArr = new Foo[fooArrSize];
for (int i = 0; i < two; i++) {
fooArr[10000000 * i] = foo;
}
int x = 0;
// 2) The Hoisted Range Check Predicate is accompanied by two Template Assertion Predicates. The LoadN node,
// previously pinned at the hoisted range check, is now pinned at the Template Assertion Predicate. Note
// that the LoadN is still inside the loop body.
// 3) The loop is now peeled 3 times which also peels 3 loads from 'fooArr' out of the loop:
// // Peeled section from 1st Loop Peeling
// ...
// LoadN[0]
// ...
// <loop entry guard>
// // Peeled section from 2nd Loop Peeling
// ...
// LoadN[10000000]
// Initialized Assertion Predicate (***)
// <loop entry guard>
// // Peeled section from 3rd Loop Peeling
// ...
// LoadN[20000000]
// ...
// Initialized Assertion Predicate
// <loop entry guard>
// Template Assertion Predicate
// Initialized Assertion Predicate
// Loop:
// LoadN[i*10000000]
//
// To avoid that the peeled LoadN nodes float above the corresponding loop entry guards, we need to pin them
// below. That is done by updating the dependency of the peeled LoadN to the new Template Assertion Predicate.
// This is currently broken: We wrongly set the dependency to the Initialized Assertion Predicate instead of the
// Template Assertion Predicate. We can then no longer find the dependency and miss to update it in the next
// Loop Peeling application. As a result, all the LoadN pile up at the originally added Initialized Assertion
// Predicate of the first Loop Peeling application at (***).
//
// With GCM, we could schedule LoadN[20000000] at (***), before the loop entry corresponding loop entry guard
// for this load. We then crash during runtime because we are accessing an out-of-range index. The fix is to
// properly update the data dependencies to the Template Assertion Predicates and not the Initialized Assertion
// Predicates.
for (int i = 0; i < two; i++) {
// 1) Hoisted with a Hoisted Range Check Predicate
x += fooArr[i * 10000000].iFld;
if (iFld2 == 4) {
return;
}
if (i > 0 && iFld2 == 3) {
iFld2 = 42;
return;
}
if (i > 1 && iFld2 == 2) {
return;
}
}
}
/*
* Tests collected in JBS and duplicated issues
*/
// -Xbatch -XX:CompileCommand=compileonly,Test*::*
static void test8305428() {
int j = 1;