From 2edb6d98133d8bd6dc4527c7497c460283fdc53e Mon Sep 17 00:00:00 2001 From: Christian Hagedorn Date: Tue, 28 May 2024 08:12:36 +0000 Subject: [PATCH] 8330386: Replace Opaque4Node of Initialized Assertion Predicate with new OpaqueInitializedAssertionPredicateNode Reviewed-by: kvn, roland --- src/hotspot/share/opto/classes.hpp | 1 + src/hotspot/share/opto/loopPredicate.cpp | 7 +- src/hotspot/share/opto/loopTransform.cpp | 95 ++-- src/hotspot/share/opto/loopnode.cpp | 7 +- src/hotspot/share/opto/loopnode.hpp | 3 +- src/hotspot/share/opto/loopopts.cpp | 21 +- src/hotspot/share/opto/macro.cpp | 16 +- src/hotspot/share/opto/node.cpp | 2 +- src/hotspot/share/opto/node.hpp | 9 +- src/hotspot/share/opto/opaquenode.cpp | 4 + src/hotspot/share/opto/opaquenode.hpp | 19 +- src/hotspot/share/opto/predicates.cpp | 10 +- src/hotspot/share/opto/predicates.hpp | 2 +- src/hotspot/share/opto/split_if.cpp | 6 +- ...aqueInitializedAssertionPredicateNode.java | 407 ++++++++++++++++++ 15 files changed, 542 insertions(+), 67 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/predicates/assertion/TestOpaqueInitializedAssertionPredicateNode.java diff --git a/src/hotspot/share/opto/classes.hpp b/src/hotspot/share/opto/classes.hpp index f1da18f464e..db6cb817d77 100644 --- a/src/hotspot/share/opto/classes.hpp +++ b/src/hotspot/share/opto/classes.hpp @@ -271,6 +271,7 @@ macro(OpaqueLoopStride) macro(OpaqueZeroTripGuard) macro(Opaque3) macro(Opaque4) +macro(OpaqueInitializedAssertionPredicate) macro(ProfileBoolean) macro(OrI) macro(OrL) diff --git a/src/hotspot/share/opto/loopPredicate.cpp b/src/hotspot/share/opto/loopPredicate.cpp index 9fcfbaf5e64..8d2cf9dc936 100644 --- a/src/hotspot/share/opto/loopPredicate.cpp +++ b/src/hotspot/share/opto/loopPredicate.cpp @@ -348,10 +348,13 @@ void PhaseIdealLoop::get_assertion_predicates(Node* predicate, Unique_Node_List& if (uncommon_proj->unique_ctrl_out() != rgn) { break; } - if (iff->in(1)->Opcode() == Op_Opaque4 && assertion_predicate_has_loop_opaque_node(iff)) { + Node* bol = iff->in(1); + assert(!bol->is_OpaqueInitializedAssertionPredicate(), "should not find an Initialized Assertion Predicate"); + if (bol->is_Opaque4()) { + assert(assertion_predicate_has_loop_opaque_node(iff), "must find OpaqueLoop* nodes"); if (get_opaque) { // Collect the predicate Opaque4 node. - list.push(iff->in(1)); + list.push(bol); } else { // Collect the predicate projection. list.push(predicate); diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index f2022d35cf6..b2482edbf7a 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -1201,11 +1201,12 @@ bool IdealLoopTree::policy_range_check(PhaseIdealLoop* phase, bool provisional, // Comparing trip+off vs limit Node *bol = iff->in(1); - if (bol->req() != 2) { + if (bol->req() < 2) { continue; // dead constant test } if (!bol->is_Bool()) { - assert(bol->Opcode() == Op_Conv2B, "predicate check only"); + assert(bol->is_Opaque4() || bol->is_OpaqueInitializedAssertionPredicate(), + "Opaque node of non-null-check or of Initialized Assertion Predicate"); continue; } if (bol->as_Bool()->_test._test == BoolTest::ne) { @@ -1385,8 +1386,10 @@ void PhaseIdealLoop::copy_assertion_predicates_to_main_loop_helper(const Predica uncommon_proj = iff->proj_out(1 - predicate_proj->as_Proj()->_con); if (uncommon_proj->unique_ctrl_out() != rgn) break; - if (iff->in(1)->Opcode() == Op_Opaque4) { - assert(assertion_predicate_has_loop_opaque_node(iff), "unexpected"); + Node* bol = iff->in(1); + assert(!bol->is_OpaqueInitializedAssertionPredicate(), "should not find an Initialized Assertion Predicate"); + if (bol->is_Opaque4()) { + assert(assertion_predicate_has_loop_opaque_node(iff), "must find OpaqueLoop* nodes"); // Clone the Assertion Predicate twice and initialize one with the initial // value of the loop induction variable. Leave the other predicate // to be initialized when increasing the stride during loop unrolling. @@ -1496,20 +1499,25 @@ Node* PhaseIdealLoop::clone_assertion_predicate_and_initialize(Node* iff, Node* Node* uncommon_proj, Node* control, IdealLoopTree* outer_loop, Node* input_proj) { TemplateAssertionPredicateExpression template_assertion_predicate_expression(iff->in(1)->as_Opaque4()); - Opaque4Node* new_opaque4_node; + Node* new_opaque_node; if (new_stride == nullptr) { - // Only set a new OpaqueLoopInitNode node and clone the existing OpaqueLoopStrideNode without modification. + // Clone the Template Assertion Predicate and set a new OpaqueLoopInitNode to create a new Template Assertion Predicate. // This is done when creating a new Template Assertion Predicate for the main loop which requires a new init node. + // We keep the Opaque4 node since it's still a template. assert(new_init->is_OpaqueLoopInit(), "only for creating new Template Assertion Predicates"); - new_opaque4_node = template_assertion_predicate_expression.clone_and_replace_init(new_init, control, this); + new_opaque_node = template_assertion_predicate_expression.clone_and_replace_init(new_init, control, this); } else { - new_opaque4_node = template_assertion_predicate_expression.clone_and_replace_init_and_stride(new_init, new_stride, + // Create an Initialized Assertion Predicate from the Template Assertion Predicate. + new_opaque_node = template_assertion_predicate_expression.clone_and_replace_init_and_stride(new_init, new_stride, control, this); + // Since this is an Initialized Assertion Predicate, we use the dedicated opaque node. + new_opaque_node = new OpaqueInitializedAssertionPredicateNode(new_opaque_node->in(1)->as_Bool(), C); + register_new_node(new_opaque_node, control); } Node* proj = predicate->clone(); Node* other_proj = uncommon_proj->clone(); Node* new_iff = iff->clone(); - new_iff->set_req(1, new_opaque4_node); + new_iff->set_req(1, new_opaque_node); proj->set_req(0, new_iff); other_proj->set_req(0, new_iff); Node* frame = new ParmNode(C->start(), TypeFunc::FramePtr); @@ -1965,25 +1973,30 @@ void PhaseIdealLoop::update_main_loop_assertion_predicates(Node* ctrl, CountedLo while (entry != nullptr && entry->is_Proj() && entry->in(0)->is_If()) { IfNode* iff = entry->in(0)->as_If(); ProjNode* proj = iff->proj_out(1 - entry->as_Proj()->_con); - if (proj->unique_ctrl_out()->Opcode() != Op_Halt) { + if (!proj->unique_ctrl_out()->is_Halt()) { break; } - if (iff->in(1)->Opcode() == Op_Opaque4) { - if (!assertion_predicate_has_loop_opaque_node(iff)) { - // No OpaqueLoop* node? Then it's one of the two Initialized Assertion Predicates: - // - For the initial access a[init] - // - For the last access a[init+old_stride-orig_stride] - // We could keep the one for the initial access but we do not know which one we currently have here. Just kill both. - // We will create new Initialized Assertion Predicates from the Template Assertion Predicates below: + Node* bol = iff->in(1); + if (bol->is_Opaque4()) { + if (assertion_predicate_has_loop_opaque_node(iff)) { + // This is a Template Assertion Predicate for the initial or last access. + // Create an Initialized Assertion Predicates for it accordingly: // - For the initial access a[init] (same as before) // - For the last access a[init+new_stride-orig_stride] (with the new unroll stride) - _igvn.replace_input_of(iff, 1, iff->in(1)->in(2)); - } else { - // Template Assertion Predicate: Clone it to create initialized version with new stride. prev_proj = clone_assertion_predicate_and_initialize(iff, init, max_value, entry, proj, ctrl, outer_loop, prev_proj); assert(!assertion_predicate_has_loop_opaque_node(prev_proj->in(0)->as_If()), "unexpected"); + } else { + // Ignore Opaque4 from a non-null-check for an intrinsic or unsafe access. This could happen when we maximally + // unroll a non-main loop with such an If with an Opaque4 node directly above the loop entry. + assert(!loop_head->is_main_loop(), "Opaque4 node from a non-null check - should not be at main loop"); } + } else if (bol->is_OpaqueInitializedAssertionPredicate()) { + // This is one of the two Initialized Assertion Predicates: + // - For the initial access a[init] + // - For the last access a[init+old_stride-orig_stride] + // We could keep the one for the initial access but we do not know which one we currently have here. Just kill both. + _igvn.replace_input_of(iff, 1, _igvn.intcon(1)); } entry = entry->in(0)->in(0); } @@ -2006,13 +2019,15 @@ void PhaseIdealLoop::copy_assertion_predicates_to_post_loop(LoopNode* main_loop_ while (ctrl != nullptr && ctrl->is_Proj() && ctrl->in(0)->is_If()) { IfNode* iff = ctrl->in(0)->as_If(); ProjNode* proj = iff->proj_out(1 - ctrl->as_Proj()->_con); - if (proj->unique_ctrl_out()->Opcode() != Op_Halt) { + if (!proj->unique_ctrl_out()->is_Halt()) { break; } - if (iff->in(1)->Opcode() == Op_Opaque4 && assertion_predicate_has_loop_opaque_node(iff)) { + if (iff->in(1)->is_Opaque4()) { + // Initialize from Template Assertion Predicate. + assert(assertion_predicate_has_loop_opaque_node(iff), "must find OpaqueLoop* nodes"); prev_proj = clone_assertion_predicate_and_initialize(iff, init, stride, ctrl, proj, post_loop_entry, post_loop, prev_proj); - assert(!assertion_predicate_has_loop_opaque_node(prev_proj->in(0)->as_If()), "unexpected"); + assert(!assertion_predicate_has_loop_opaque_node(prev_proj->in(0)->as_If()), "must not find OpaqueLoop* nodes"); } ctrl = ctrl->in(0)->in(0); } @@ -2043,8 +2058,11 @@ void PhaseIdealLoop::initialize_assertion_predicates_for_peeled_loop(const Predi // Does not belong to this Predicate Block anymore. break; } - if (iff->in(1)->Opcode() == Op_Opaque4) { - assert(assertion_predicate_has_loop_opaque_node(iff), "unexpected"); + Node* bol = iff->in(1); + assert(!bol->is_OpaqueInitializedAssertionPredicate(), "should not find an Initialized Assertion Predicate"); + if (bol->is_Opaque4()) { + // Initialize from Template Assertion Predicate. + assert(assertion_predicate_has_loop_opaque_node(iff), "must find OpaqueLoop* nodes"); input_proj = clone_assertion_predicate_and_initialize(iff, init, stride, next_regular_predicate_proj, uncommon_proj, control, outer_loop, input_proj); @@ -2767,20 +2785,25 @@ bool PhaseIdealLoop::is_scaled_iv_plus_extra_offset(Node* exp1, Node* offset3, N // Same as PhaseIdealLoop::duplicate_predicates() but for range checks // eliminated by iteration splitting. -Node* PhaseIdealLoop::add_range_check_elimination_assertion_predicate(IdealLoopTree* loop, - Node* ctrl, const int scale_con, - Node* offset, Node* limit, jint stride_con, - Node* value) { +Node* PhaseIdealLoop::add_range_check_elimination_assertion_predicate(IdealLoopTree* loop, Node* ctrl, + const int scale_con, Node* offset, Node* limit, + jint stride_con, Node* value, + const bool is_template) { bool overflow = false; BoolNode* bol = rc_predicate(ctrl, scale_con, offset, value, nullptr, stride_con, limit, (stride_con > 0) != (scale_con > 0), overflow); - Node* opaque_bol = new Opaque4Node(C, bol, _igvn.intcon(1)); - register_new_node(opaque_bol, ctrl); + Node* opaque_assertion_predicate; + if (is_template) { + opaque_assertion_predicate = new Opaque4Node(C, bol, _igvn.intcon(1)); + } else { + opaque_assertion_predicate = new OpaqueInitializedAssertionPredicateNode(bol, C); + } + register_new_node(opaque_assertion_predicate, ctrl); IfNode* new_iff = nullptr; if (overflow) { - new_iff = new IfNode(ctrl, opaque_bol, PROB_MAX, COUNT_UNKNOWN); + new_iff = new IfNode(ctrl, opaque_assertion_predicate, PROB_MAX, COUNT_UNKNOWN); } else { - new_iff = new RangeCheckNode(ctrl, opaque_bol, PROB_MAX, COUNT_UNKNOWN); + new_iff = new RangeCheckNode(ctrl, opaque_assertion_predicate, PROB_MAX, COUNT_UNKNOWN); } register_control(new_iff, loop->_parent, ctrl); Node* iffalse = new IfFalseNode(new_iff); @@ -2981,13 +3004,13 @@ void PhaseIdealLoop::do_range_check(IdealLoopTree *loop, Node_List &old_new) { // Initialized Assertion Predicate for the value of the initial main-loop. loop_entry = add_range_check_elimination_assertion_predicate(loop, loop_entry, scale_con, int_offset, - int_limit, stride_con, init); + int_limit, stride_con, init, false); assert(!assertion_predicate_has_loop_opaque_node(loop_entry->in(0)->as_If()), "unexpected"); // Add two Template Assertion Predicates to create new Initialized Assertion Predicates from when either // unrolling or splitting this main-loop further. loop_entry = add_range_check_elimination_assertion_predicate(loop, loop_entry, scale_con, int_offset, - int_limit, stride_con, opaque_init); + int_limit, stride_con, opaque_init, true); assert(assertion_predicate_has_loop_opaque_node(loop_entry->in(0)->as_If()), "unexpected"); Node* opaque_stride = new OpaqueLoopStrideNode(C, cl->stride()); @@ -3000,7 +3023,7 @@ void PhaseIdealLoop::do_range_check(IdealLoopTree *loop, Node_List &old_new) { max_value = new CastIINode(max_value, loop->_head->as_CountedLoop()->phi()->bottom_type()); register_new_node(max_value, loop_entry); loop_entry = add_range_check_elimination_assertion_predicate(loop, loop_entry, scale_con, int_offset, - int_limit, stride_con, max_value); + int_limit, stride_con, max_value, true); assert(assertion_predicate_has_loop_opaque_node(loop_entry->in(0)->as_If()), "unexpected"); } else { diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index fac7cf32f60..b877888fccb 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -4380,10 +4380,9 @@ void PhaseIdealLoop::collect_useful_template_assertion_predicates_for_loop(Ideal void PhaseIdealLoop::eliminate_useless_template_assertion_predicates(Unique_Node_List& useful_predicates) { for (int i = C->template_assertion_predicate_count(); i > 0; i--) { - Node* opaque4 = C->template_assertion_predicate_opaq_node(i - 1); - assert(opaque4->Opcode() == Op_Opaque4, "must be"); - if (!useful_predicates.member(opaque4)) { // not in the useful list - _igvn.replace_node(opaque4, opaque4->in(2)); + Opaque4Node* opaque4_node = C->template_assertion_predicate_opaq_node(i - 1)->as_Opaque4(); + if (!useful_predicates.member(opaque4_node)) { // not in the useful list + _igvn.replace_node(opaque4_node, opaque4_node->in(2)); } } } diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index f7ba4578a51..ad603439e59 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1380,7 +1380,8 @@ public: IfProjNode* upper_bound_proj, int scale, Node* offset, Node* init, Node* limit, jint stride, Node* rng, bool& overflow, Deoptimization::DeoptReason reason); Node* add_range_check_elimination_assertion_predicate(IdealLoopTree* loop, Node* predicate_proj, int scale_con, - Node* offset, Node* limit, jint stride_con, Node* value); + Node* offset, Node* limit, int stride_con, Node* value, + bool is_template); // Helper function to collect predicate for eliminating the useless ones void eliminate_useless_predicates(); diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 25000f10000..200adc34494 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -779,8 +779,12 @@ Node *PhaseIdealLoop::conditional_move( Node *region ) { } }//for Node* bol = iff->in(1); - if (bol->Opcode() == Op_Opaque4) { - return nullptr; // Ignore loop predicate checks (the Opaque4 ensures they will go away) + assert(!bol->is_OpaqueInitializedAssertionPredicate(), "Initialized Assertion Predicates cannot form a diamond with Halt"); + if (bol->is_Opaque4()) { + // Ignore Template Assertion Predicates with Opaque4 nodes. + assert(assertion_predicate_has_loop_opaque_node(iff), + "must be Template Assertion Predicate, non-null-check with Opaque4 cannot form a diamond with Halt"); + return nullptr; } assert(bol->Opcode() == Op_Bool, "Unexpected node"); int cmp_op = bol->in(1)->Opcode(); @@ -1663,7 +1667,8 @@ void PhaseIdealLoop::try_sink_out_of_loop(Node* n) { !n->is_Proj() && !n->is_MergeMem() && !n->is_CMove() && - n->Opcode() != Op_Opaque4 && + !n->is_Opaque4() && + !n->is_OpaqueInitializedAssertionPredicate() && !n->is_Type()) { Node *n_ctrl = get_ctrl(n); IdealLoopTree *n_loop = get_loop(n_ctrl); @@ -1976,18 +1981,18 @@ Node* PhaseIdealLoop::clone_iff(PhiNode* phi) { // Convert this Phi into a Phi merging Bools uint i; for (i = 1; i < phi->req(); i++) { - Node *b = phi->in(i); + Node* b = phi->in(i); if (b->is_Phi()) { _igvn.replace_input_of(phi, i, clone_iff(b->as_Phi())); } else { - assert(b->is_Bool() || b->Opcode() == Op_Opaque4, ""); + assert(b->is_Bool() || b->is_Opaque4() || b->is_OpaqueInitializedAssertionPredicate(), + "bool, non-null check with Opaque4 node or Initialized Assertion Predicate with its Opaque node"); } } - Node* n = phi->in(1); Node* sample_opaque = nullptr; Node *sample_bool = nullptr; - if (n->Opcode() == Op_Opaque4) { + if (n->is_Opaque4() || n->is_OpaqueInitializedAssertionPredicate()) { sample_opaque = n; sample_bool = n->in(1); assert(sample_bool->is_Bool(), "wrong type"); @@ -2160,7 +2165,7 @@ void PhaseIdealLoop::clone_loop_handle_data_uses(Node* old, Node_List &old_new, // make sure the Bool/Cmp input is cloned down to avoid a Phi between // the AllocateArray node and its ValidLengthTest input that could cause // split if to break. - if (use->is_If() || use->is_CMove() || use->Opcode() == Op_Opaque4 || + if (use->is_If() || use->is_CMove() || use->is_Opaque4() || (use->Opcode() == Op_AllocateArray && use->in(AllocateNode::ValidLengthTest) == old)) { // Since this code is highly unlikely, we lazily build the worklist // of such Nodes to go split. diff --git a/src/hotspot/share/opto/macro.cpp b/src/hotspot/share/opto/macro.cpp index 2a26a4ebed3..ce1a8a61134 100644 --- a/src/hotspot/share/opto/macro.cpp +++ b/src/hotspot/share/opto/macro.cpp @@ -2429,7 +2429,8 @@ void PhaseMacroExpand::eliminate_macro_nodes() { default: assert(n->Opcode() == Op_LoopLimit || n->Opcode() == Op_Opaque3 || - n->Opcode() == Op_Opaque4 || + n->is_Opaque4() || + n->is_OpaqueInitializedAssertionPredicate() || n->Opcode() == Op_MaxL || n->Opcode() == Op_MinL || BarrierSet::barrier_set()->barrier_set_c2()->is_gc_barrier_node(n), @@ -2504,7 +2505,7 @@ bool PhaseMacroExpand::expand_macro_nodes() { _igvn.replace_node(n, repl); success = true; #endif - } else if (n->Opcode() == Op_Opaque4) { + } else if (n->is_Opaque4()) { // With Opaque4 nodes, the expectation is that the test of input 1 // is always equal to the constant value of input 2. So we can // remove the Opaque4 and replace it by input 2. In debug builds, @@ -2517,6 +2518,17 @@ bool PhaseMacroExpand::expand_macro_nodes() { _igvn.replace_node(n, n->in(2)); #endif success = true; + } else if (n->is_OpaqueInitializedAssertionPredicate()) { + // Initialized Assertion Predicates must always evaluate to true. Therefore, we get rid of them in product + // builds as they are useless. In debug builds we keep them as additional verification code. Even though + // loop opts are already over, we want to keep Initialized Assertion Predicates alive as long as possible to + // enable folding of dead control paths within which cast nodes become top after due to impossible types - + // even after loop opts are over. Therefore, we delay the removal of these opaque nodes until now. +#ifdef ASSERT + _igvn.replace_node(n, n->in(1)); +#else + _igvn.replace_node(n, _igvn.intcon(1)); +#endif // ASSERT } else if (n->Opcode() == Op_OuterStripMinedLoop) { n->as_OuterStripMinedLoop()->adjust_strip_mined_loop(&_igvn); C->remove_macro_node(n); diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp index fa85344d0da..d290541dffa 100644 --- a/src/hotspot/share/opto/node.cpp +++ b/src/hotspot/share/opto/node.cpp @@ -610,7 +610,7 @@ void Node::destruct(PhaseValues* phase) { if (is_expensive()) { compile->remove_expensive_node(this); } - if (Opcode() == Op_Opaque4) { + if (is_Opaque4()) { compile->remove_template_assertion_predicate_opaq(this); } if (is_ParsePredicate()) { diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp index 314f165ac8b..dc9dc6654b5 100644 --- a/src/hotspot/share/opto/node.hpp +++ b/src/hotspot/share/opto/node.hpp @@ -138,6 +138,7 @@ class Opaque1Node; class OpaqueLoopInitNode; class OpaqueLoopStrideNode; class Opaque4Node; +class OpaqueInitializedAssertionPredicateNode; class OuterStripMinedLoopNode; class OuterStripMinedLoopEndNode; class Node; @@ -797,9 +798,10 @@ public: DEFINE_CLASS_ID(OpaqueLoopInit, Opaque1, 0) DEFINE_CLASS_ID(OpaqueLoopStride, Opaque1, 1) DEFINE_CLASS_ID(Opaque4, Node, 17) - DEFINE_CLASS_ID(Move, Node, 18) - DEFINE_CLASS_ID(LShift, Node, 19) - DEFINE_CLASS_ID(Neg, Node, 20) + DEFINE_CLASS_ID(OpaqueInitializedAssertionPredicate, Node, 18) + DEFINE_CLASS_ID(Move, Node, 19) + DEFINE_CLASS_ID(LShift, Node, 20) + DEFINE_CLASS_ID(Neg, Node, 21) _max_classes = ClassMask_Neg }; @@ -968,6 +970,7 @@ public: DEFINE_CLASS_QUERY(NeverBranch) DEFINE_CLASS_QUERY(Opaque1) DEFINE_CLASS_QUERY(Opaque4) + DEFINE_CLASS_QUERY(OpaqueInitializedAssertionPredicate) DEFINE_CLASS_QUERY(OpaqueLoopInit) DEFINE_CLASS_QUERY(OpaqueLoopStride) DEFINE_CLASS_QUERY(OuterStripMinedLoop) diff --git a/src/hotspot/share/opto/opaquenode.cpp b/src/hotspot/share/opto/opaquenode.cpp index 5075ccd4664..0abc6f86ed0 100644 --- a/src/hotspot/share/opto/opaquenode.cpp +++ b/src/hotspot/share/opto/opaquenode.cpp @@ -102,6 +102,10 @@ const Type* Opaque4Node::Value(PhaseGVN* phase) const { return phase->type(in(1)); } +const Type* OpaqueInitializedAssertionPredicateNode::Value(PhaseGVN* phase) const { + return phase->type(in(1)); +} + //============================================================================= uint ProfileBooleanNode::hash() const { return NO_HASH; } diff --git a/src/hotspot/share/opto/opaquenode.hpp b/src/hotspot/share/opto/opaquenode.hpp index 12a95ce2edb..2337989e974 100644 --- a/src/hotspot/share/opto/opaquenode.hpp +++ b/src/hotspot/share/opto/opaquenode.hpp @@ -121,17 +121,32 @@ class Opaque3Node : public Node { // GraphKit::must_be_not_null(). class Opaque4Node : public Node { public: - Opaque4Node(Compile* C, Node *tst, Node* final_tst) : Node(nullptr, tst, final_tst) { + Opaque4Node(Compile* C, Node* tst, Node* final_tst) : Node(nullptr, tst, final_tst) { init_class_id(Class_Opaque4); init_flags(Flag_is_macro); C->add_macro_node(this); } virtual int Opcode() const; - virtual const Type *bottom_type() const { return TypeInt::BOOL; } virtual const Type* Value(PhaseGVN* phase) const; + virtual const Type* bottom_type() const { return TypeInt::BOOL; } }; +// This node is used for Initialized Assertion Predicate BoolNodes. Initialized Assertion Predicates must always evaluate +// to true. Therefore, we get rid of them in product builds during macro expansion as they are useless. In debug builds +// we keep them as additional verification code (i.e. removing this node and use the BoolNode input instead). +class OpaqueInitializedAssertionPredicateNode : public Node { + public: + OpaqueInitializedAssertionPredicateNode(BoolNode* bol, Compile* C) : Node(nullptr, bol) { + init_class_id(Class_OpaqueInitializedAssertionPredicate); + init_flags(Flag_is_macro); + C->add_macro_node(this); + } + + virtual int Opcode() const; + virtual const Type* Value(PhaseGVN* phase) const; + virtual const Type* bottom_type() const { return TypeInt::BOOL; } +}; //------------------------------ProfileBooleanNode------------------------------- // A node represents value profile for a boolean during parsing. diff --git a/src/hotspot/share/opto/predicates.cpp b/src/hotspot/share/opto/predicates.cpp index 7f873e1fb6f..9f782f34bdb 100644 --- a/src/hotspot/share/opto/predicates.cpp +++ b/src/hotspot/share/opto/predicates.cpp @@ -42,13 +42,15 @@ bool AssertionPredicatesWithHalt::is_assertion_predicate_success_proj(const Node if (predicate_proj == nullptr || !predicate_proj->is_IfProj() || !predicate_proj->in(0)->is_If()) { return false; } - return has_opaque4(predicate_proj) && has_halt(predicate_proj); + return has_assertion_predicate_opaque(predicate_proj) && has_halt(predicate_proj); } -// Check if the If node of `predicate_proj` has an Opaque4 node as input. -bool AssertionPredicatesWithHalt::has_opaque4(const Node* predicate_proj) { +// Check if the If node of `predicate_proj` has an Opaque4 (Template Assertion Predicate) or an +// OpaqueInitializedAssertionPredicate (Initialized Assertion Predicate) node as input. +bool AssertionPredicatesWithHalt::has_assertion_predicate_opaque(const Node* predicate_proj) { IfNode* iff = predicate_proj->in(0)->as_If(); - return iff->in(1)->Opcode() == Op_Opaque4; + Node* bol = iff->in(1); + return bol->is_Opaque4() || bol->is_OpaqueInitializedAssertionPredicate(); } // Check if the other projection (UCT projection) of `success_proj` has a Halt node as output. diff --git a/src/hotspot/share/opto/predicates.hpp b/src/hotspot/share/opto/predicates.hpp index 670ca58cc47..bf12aeb6a51 100644 --- a/src/hotspot/share/opto/predicates.hpp +++ b/src/hotspot/share/opto/predicates.hpp @@ -199,7 +199,7 @@ class AssertionPredicatesWithHalt : public StackObj { Node* _entry; static Node* find_entry(Node* start_proj); - static bool has_opaque4(const Node* predicate_proj); + static bool has_assertion_predicate_opaque(const Node* predicate_proj); static bool has_halt(const Node* success_proj); static bool is_assertion_predicate_success_proj(const Node* predicate_proj); diff --git a/src/hotspot/share/opto/split_if.cpp b/src/hotspot/share/opto/split_if.cpp index 70369a2c9c4..ad4053e2c98 100644 --- a/src/hotspot/share/opto/split_if.cpp +++ b/src/hotspot/share/opto/split_if.cpp @@ -322,7 +322,7 @@ bool PhaseIdealLoop::clone_cmp_down(Node* n, const Node* blk1, const Node* blk2) assert( bol->is_Bool(), "" ); if (bol->outcnt() == 1) { Node* use = bol->unique_out(); - if (use->Opcode() == Op_Opaque4) { + if (use->is_Opaque4() || use->is_OpaqueInitializedAssertionPredicate()) { if (use->outcnt() == 1) { Node* iff = use->unique_out(); assert(iff->is_If(), "unexpected node type"); @@ -351,8 +351,8 @@ bool PhaseIdealLoop::clone_cmp_down(Node* n, const Node* blk1, const Node* blk2) #endif for (DUIterator j = bol->outs(); bol->has_out(j); j++) { Node* u = bol->out(j); - // Uses are either IfNodes, CMoves or Opaque4 - if (u->Opcode() == Op_Opaque4) { + // Uses are either IfNodes, CMoves, Opaque4, or OpaqueInitializedAssertionPredicates + if (u->is_Opaque4() || u->is_OpaqueInitializedAssertionPredicate()) { assert(u->in(1) == bol, "bad input"); for (DUIterator_Last kmin, k = u->last_outs(kmin); k >= kmin; --k) { Node* iff = u->last_out(k); diff --git a/test/hotspot/jtreg/compiler/predicates/assertion/TestOpaqueInitializedAssertionPredicateNode.java b/test/hotspot/jtreg/compiler/predicates/assertion/TestOpaqueInitializedAssertionPredicateNode.java new file mode 100644 index 00000000000..8fb9e693eb6 --- /dev/null +++ b/test/hotspot/jtreg/compiler/predicates/assertion/TestOpaqueInitializedAssertionPredicateNode.java @@ -0,0 +1,407 @@ +/* + * Copyright (c) 2024, 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 8330386 + * @summary Test that replacing Opaque4 nodes with OpaqueInitializedAssertionPredicate for Initialized Assertion Predicates + * works. We test following cases explicitly: + * 1) Cloning down CmpUNode in Split If with involved OpaqueInitializedAssertionPredicateNodes + * 2) Special casing OpaqueInitializedAssertionPredicate in IdealLoopTree::policy_range_check() + * 3) Special casing Opaque4 node from non-null check for intrinsics and unsafe accesses inside + * PhaseIdealLoop::update_main_loop_assertion_predicates(). + * @requires vm.compiler2.enabled + * @modules java.base/jdk.internal.misc:+open + * @run main/othervm -Xbatch -XX:LoopMaxUnroll=0 + * -XX:CompileCommand=compileonly,*TestOpaqueInitializedAssertionPredicateNode::test* + * -XX:CompileCommand=dontinline,*TestOpaqueInitializedAssertionPredicateNode::dontInline + * compiler.predicates.assertion.TestOpaqueInitializedAssertionPredicateNode + * @run main/othervm -Xcomp -XX:LoopMaxUnroll=0 -XX:-LoopUnswitching + * -XX:CompileCommand=compileonly,*TestOpaqueInitializedAssertionPredicateNode::test* + * -XX:CompileCommand=dontinline,*TestOpaqueInitializedAssertionPredicateNode::dontInline + * compiler.predicates.assertion.TestOpaqueInitializedAssertionPredicateNode + * @run main/othervm -Xbatch -XX:LoopMaxUnroll=0 -XX:PerMethodTrapLimit=0 + * -XX:CompileCommand=compileonly,*TestOpaqueInitializedAssertionPredicateNode::test* + * -XX:CompileCommand=dontinline,*TestOpaqueInitializedAssertionPredicateNode::dontInline + * compiler.predicates.assertion.TestOpaqueInitializedAssertionPredicateNode + */ + +/* + * @test id=noflags + * @bug 8330386 + * @modules java.base/jdk.internal.misc:+open + * @run main compiler.predicates.assertion.TestOpaqueInitializedAssertionPredicateNode + */ + +package compiler.predicates.assertion; + +import jdk.internal.misc.Unsafe; +import java.lang.reflect.Field; + +public class TestOpaqueInitializedAssertionPredicateNode { + + static boolean flag, flag2; + static int iFld; + static int x; + static int y = 51; + static int iArrLength; + static int[] iArr = new int[100]; + + static final Unsafe UNSAFE = Unsafe.getUnsafe(); + static final long OFFSET; + + static { + try { + Field fieldIFld = A.class.getDeclaredField("iFld"); + OFFSET = UNSAFE.objectFieldOffset(fieldIFld); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + public static void main(String[] args) { + Integer.compareUnsigned(23, 34); // Make sure loaded with -Xcomp. + A a = new A(34); + for (int i = 0; i < 10000; i++) { + iArrLength = i % 15 == 0 ? 30 : 100; + flag = i % 3 == 0; + flag2 = i % 5 == 0; + x = (i % 15 == 0 ? 100 : 0); + testCloneDown(); + testOnlyCloneDownCmp(); + testCloneDownInsideLoop(); + maybeNull(null); // Make sure return value is sometimes null. + testPolicyRangeCheck(a); + testUnsafeAccess(a); + testOpaqueOutsideLoop(); + testOpaqueInsideIfOutsideLoop(); + } + } + + // Profiling will tell us that the return value is sometimes null and sometimes not. + static A maybeNull(Object o) { + return (A)o; + } + + static void testCloneDown() { + int a; + int b; + int[] iArr = new int[iArrLength]; + + for (int i = 2; i < 4; i *= 2) ; // Make sure to run with loop opts. + + if (flag) { + a = 34; + } else { + a = 3; + } + // Region to split through + + // --- BLOCK start --- + + // CMoveI(Bool(CmpU(y, iArr.length))), 34, 23) (**) + if (Integer.compareUnsigned(y, iArr.length) < 0) { + b = 34; + } else { + b = 23; + } + iFld = b; // iFld = CMoveI -> make sure CMoveI is inside BLOCK + + // --- BLOCK end --- + + if (a > 23) { // If to split -> need to empty BLOCK + iFld = 34; + } + + if (flag2) { + // Avoid out-of-bounds access in loop below + return; + } + + // When peeling the loop, we create an Initialized Assertion Predicate with the same CmpU as (**) above: + // IAP(CmpU(y, iArr.length)) + // + // At Split If: Need to clone CmpU down because it has two uses: + // - Bool of Cmove used in "iFld = b" + // - Bool for IAP + // + // => IAP uses OpaqueInitializedAssertionPredicate -> clone_cmp_down() therefore needs to handle that. + for (int i = y - 1; i < 100; i++) { + iArr[i] = 34; // Hoisted with Loop Predicate + if (flag) { // Reason to peel. + return; + } + } + } + + // Same as test() but we only clone down the CmpU and not the Bool with the OpaqueInitializedAssertionPredicate + static void testOnlyCloneDownCmp() { + int a; + int b; + int[] iArr = new int[iArrLength]; + + for (int i = 2; i < 4; i *= 2) ; // Make sure to run with loop opts. + + if (flag) { + a = 34; + } else { + a = 3; + } + // Region to split through + + // --- BLOCK start --- + + // CMoveI(Bool(CmpU(51, iArr.length))), 34, 23) (**) + // Using constant 51 -> cannot common up with Bool from Initialized Assertion Predicate + if (Integer.compareUnsigned(51, iArr.length) < 0) { + b = 34; + } else { + b = 23; + } + iFld = b; // iFld = CMoveI -> make sure CMoveI is inside BLOCK + + // --- BLOCK end --- + + if (a > 23) { // If to split -> need to empty BLOCK + iFld = 34; + } + + if (flag2) { + // Avoid out-of-bounds access in loop below + return; + } + + // When peeling the loop, we create an Initialized Assertion Predicate with the same CmpU as (**) above: + // IAP(CmpU(y, iArr.length)) + // + // At Split If: Need to clone CmpU down because it has two uses: + // - Bool of Cmove used in "iFld = b" + // - Bool for IAP + // + // => IAP uses OpaqueInitializedAssertionPredicate -> clone_cmp_down() therefore needs to handle that. + for (int i = 50; i < 100; i++) { + iArr[i] = 34; // Hoisted with Loop Predicate + if (flag) { // Reason to peel. + return; + } + } + } + + // Same as test() but everything inside another loop. + static void testCloneDownInsideLoop() { + int a; + int b; + int[] iArr = new int[iArrLength]; + + for (int i = 3; i < 30; i *= 2) { // Non-counted loop + if (i < 10) { + a = 34; + } else { + a = 3; + } + // Region to split through + + // --- BLOCK start --- + + // CMoveI(Bool(CmpU(a + i, iArr.length))), 34, 23) (**) + if (Integer.compareUnsigned(a + i, iArr.length) < 0) { + b = 34; + } else { + b = 23; + } + iFld = b; // iFld = CMoveI -> make sure CMoveI is inside BLOCK + + // --- BLOCK end --- + + if (a > 23) { // If to split -> need to empty BLOCK + iFld = 34; + } + + if (i < x) { + // Avoid out-of-bounds access in loop below + return; + } + + // When peeling the loop, we create an Initialized Assertion Predicate with the same CmpU as (**) above: + // IAP(CmpU(a + i, iArr.length)) + // + // At Split If: Need to clone CmpU down because it has two uses: + // - Bool of Cmove used in "iFld = b" + // - Bool for IAP + // + // => IAP uses OpaqueInitializedAssertionPredicate -> clone_cmp_down() therefore needs to handle that. + for (int j = a + i - 1; j < 100; j++) { + iArr[j] = 34; // Hoisted with Loop Predicate + if (flag) { // Reason to peel. + return; + } + } + } + } + + static void testPolicyRangeCheck(Object o) { + int two = 100; + int limit = 2; + for (; limit < 4; limit *= 2); + for (int i = 2; i < limit; i++) { + two = 2; + } + + // 4) We call IdealLoopTree::policy_range_check() for this loop: + // - Initialized Assertion Predicate is now part of loop body. + // - Opaque4 node for null-check is also part of loop body. + // We also check the If nodes for these Opaque nodes could be eliminated with + // Range Check Elimination. We thus need to exclude Ifs with + // Opaque4 and OpaqueInitializedAssertionPredicate nodes in policy_range_check(). + for (int i = 0; i < 100; i++) { + A a = maybeNull(o); // Profiling tells us that return value *might* be null. + iFld = UNSAFE.getInt(a, OFFSET); // Emits If with Opaque4Node for null check. + + // 1) Apply Loop Predication: Loop Predicate + Template Assertion Predicate + // 2) Apply Loop Peeling: Create Initialized Assertion Predicate with + // OpaqueInitializedAssertionPredicate + // 3) After CCP: C2 knows that two == 2. CountedLoopEnd found to be true + // (only execute loop once) -> CountedLoop removed + for (int j = 0; j < two; j++) { + iArr[j] = 34; // Hoisted in Loop Predication + if (flag) { + return; + } + } + } + } + + static void testUnsafeAccess(Object o) { + A a = maybeNull(o); // Profiling tells us that return value *might* be null. + iFld = UNSAFE.getInt(a, OFFSET); // Emits If with Opaque4Node for null check. + + // We don't have any Parse Predicates with -XX:PerMethodTrapLimit=0. And therefore, If with Opaque4 will + // directly be above CountedLoop. When maximally unrolling the counted loop, we try to update any Assertion + // Predicate. We will find the If with the Opaque4 node for the non-null check which is not an Assertion + // Predicate. This needs to be handled separately in PhaseIdealLoop::update_main_loop_assertion_predicates(). + for (int i = 0; i < 10; i++) { + iFld *= 34; + } + } + + // [If->OpaqueInitializedAssertionPredicate]->Bool->Cmp, []: Inside Loop, other nodes outside. + static void testOpaqueOutsideLoop() { + int two = 100; + int limit = 2; + for (; limit < 4; limit *= 2); + for (int i = 2; i < limit; i++) { + two = 2; + } + + // 4) After CCP, we can apply Loop Peeling since we removed enough nodes to bring the body size down below 255. + // When cloning the Bool for the IAP, we have a use inside the loop (initializedAssertionPredicateBool) and one + // outside for the IAP (input to the OpaqueInitializedAssertionPredicate being outside the loop) + // As a result, we add the OpaqueInitializedAssertionPredicate to the split if set in clone_loop_handle_data_uses(). + for (short i = 3; i < 30; i*=2) { // Use short such that we do not need overflow protection for Loop Predicates + if (two == 100) { + // Before CCP: Uninlined method call prevents peeling. + // After CCP: C2 knows that two == 2 and we remove this call which enables Loop Peeling for i-loop. + dontInline(); + } + + // Same condition as used for IAP in j-loop below. + boolean initializedAssertionPredicateBool = Integer.compareUnsigned(1 + i, iArr.length) < 0; + + if (flag) { + // 1) Loop Predicate + Template Assertion Predicate + // 2) Loop Peeling: Create IAP with same condition as initializedAssertionPredicateBool -> can be shared. + // The IAP is on a loop-exit and therefore outside the loop. + // 3) After CCP: C2 knows that two == 2 and loop is removed. + for (short j = 0; j < two; j++) { + iArr[i + j] = 34; // Hoisted in Loop Predication + if (flag2) { + break; + } + } + break; + } + + // Use Bool inside i-loop such that when applying Loop Peeling for i-loop, ctrl of Bool is inside loop and + // OpaqueInitializedAssertionPredicate of IAP is outside of i-loop. + if (initializedAssertionPredicateBool) { + iFld = 3; + } + } + } + + // Similar to testOpaqueOutside loop but Opaque is now also inside loop. + // [If]->OpaqueInitializedAssertionPredicate->Bool->Cmp, []: Inside Loop, other nodes outside. + static void testOpaqueInsideIfOutsideLoop() { + int two = 100; + int limit = 2; + for (; limit < 4; limit *= 2); + for (int i = 2; i < limit; i++) { + two = 2; + } + + for (short i = 3; i < 30; i*=2) { + if (two == 100) { + // Before CCP: Uninlined method call prevents peeling. + // After CCP: C2 knows that two == 2 and we remove this call which enables Loop Peeling for i-loop. + dontInline(); + } + + // 1) Loop Predicate + Template Assertion Predicate + // 2) Loop Peeling: Create IAP with same condition as initializedAssertionPredicateBool -> can be shared. + // The IAP is on a loop-exit and therefore outside the loop. + // 3) After CCP: C2 knows that two == 2 and loop is removed. + for (short j = 0; j < two; j++) { + iArr[i + j] = 34; // Hoisted in Loop Predication + if (flag2) { + break; + } + } + + if (flag) { + // Same loop as above. We create the same IAP which can share the same OpaqueInitializedAssertionPredicate. + // Therefore, the OpaqueInitializedAssertionPredicate is inside the loop while this If is outside the loop. + // At Loop Peeling, we clone the Opaque node and create a Phi to merge both loop versions into the IAP If. In + // clone_loop_handle_data_uses(), we add the If for the IAP to the split if set in (). Later, we + // process its input phi with their OpaqueInitializedAssertionPredicate inputs. + for (short j = 0; j < two; j++) { + iArr[i + j] = 34; // Hoisted in Loop Predication + if (flag2) { + break; + } + } + break; + } + } + } + + // Not inlined + static void dontInline() {} + + static class A { + int iFld; + A(int i) { + this.iFld = i; + } + } +}