diff --git a/src/hotspot/share/opto/predicates.cpp b/src/hotspot/share/opto/predicates.cpp index d6078e50d69ae..137d16712d80b 100644 --- a/src/hotspot/share/opto/predicates.cpp +++ b/src/hotspot/share/opto/predicates.cpp @@ -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; diff --git a/src/hotspot/share/opto/predicates.hpp b/src/hotspot/share/opto/predicates.hpp index f19b2d73c5668..2181e498bd42a 100644 --- a/src/hotspot/share/opto/predicates.hpp +++ b/src/hotspot/share/opto/predicates.hpp @@ -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: diff --git a/test/hotspot/jtreg/compiler/predicates/assertion/TestAssertionPredicates.java b/test/hotspot/jtreg/compiler/predicates/assertion/TestAssertionPredicates.java index f7f2f1f20f68c..8f30dce15bc45 100644 --- a/test/hotspot/jtreg/compiler/predicates/assertion/TestAssertionPredicates.java +++ b/test/hotspot/jtreg/compiler/predicates/assertion/TestAssertionPredicates.java @@ -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 @@ static class Foo { } static Foo foo = new Foo(); - + static int fooArrSize = 10000001; public static void main(String[] args) { switch (args[0]) { @@ -315,7 +328,7 @@ public static void main(String[] args) { 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 static void main(String[] args) { testDataUpdateUnswitchUnroll(); testDataUpdateUnroll(); testDataUpdatePeelingUnroll(); + testPeelingThreeTimesDataUpdate(); } } case "CloneDown" -> { @@ -1064,18 +1078,18 @@ static void testDataUpdateUnswitchingPeelingUnroll() { 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 @@ static void testDataUpdateUnswitchingPeelingUnroll() { // 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 @@ static void testDataUpdatePeelingUnroll() { } } + + // -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] + // ... + // + // // Peeled section from 2nd Loop Peeling + // ... + // LoadN[10000000] + // Initialized Assertion Predicate (***) + // + // // Peeled section from 3rd Loop Peeling + // ... + // LoadN[20000000] + // ... + // Initialized Assertion Predicate + // + // 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;