diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 516036d839b..24861cf2725 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -1430,40 +1430,6 @@ void PhaseIdealLoop::copy_assertion_predicates_to_main_loop_helper(const Predica } } -// Is 'n' a node that can be found on the input chain of a Template Assertion Predicate bool (i.e. between a Template -// Assertion Predicate If node and the OpaqueLoop* nodes)? -static bool is_part_of_template_assertion_predicate_bool(Node* n) { - int op = n->Opcode(); - return (n->is_Bool() || - n->is_Cmp() || - op == Op_AndL || - op == Op_OrL || - op == Op_RShiftL || - op == Op_LShiftL || - op == Op_LShiftI || - op == Op_AddL || - op == Op_AddI || - op == Op_MulL || - op == Op_MulI || - op == Op_SubL || - op == Op_SubI || - op == Op_ConvI2L || - op == Op_CastII); -} - -bool PhaseIdealLoop::subgraph_has_opaque(Node* n) { - if (n->Opcode() == Op_OpaqueLoopInit || n->Opcode() == Op_OpaqueLoopStride) { - return true; - } - if (!is_part_of_template_assertion_predicate_bool(n)) { - return false; - } - uint init; - uint stride; - count_opaque_loop_nodes(n, init, stride); - return init != 0 || stride != 0; -} - bool PhaseIdealLoop::assertion_predicate_has_loop_opaque_node(IfNode* iff) { uint init; uint stride; @@ -1507,19 +1473,19 @@ void PhaseIdealLoop::count_opaque_loop_nodes(Node* n, uint& init, uint& stride) wq.push(n); for (uint i = 0; i < wq.size(); i++) { Node* n = wq.at(i); - if (is_part_of_template_assertion_predicate_bool(n)) { - for (uint j = 1; j < n->req(); j++) { - Node* m = n->in(j); - if (m != nullptr) { - wq.push(m); + if (TemplateAssertionPredicateExpressionNode::is_maybe_in_expression(n)) { + if (n->is_OpaqueLoopInit()) { + init++; + } else if (n->is_OpaqueLoopStride()) { + stride++; + } else { + for (uint j = 1; j < n->req(); j++) { + Node* m = n->in(j); + if (m != nullptr) { + wq.push(m); + } } } - continue; - } - if (n->Opcode() == Op_OpaqueLoopInit) { - init++; - } else if (n->Opcode() == Op_OpaqueLoopStride) { - stride++; } } } diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 658da04f209..75276aae36d 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -951,7 +951,6 @@ private: Node* uncommon_proj, Node* control, IdealLoopTree* outer_loop, Node* input_proj); static void count_opaque_loop_nodes(Node* n, uint& init, uint& stride); - static bool subgraph_has_opaque(Node* n); static bool assertion_predicate_has_loop_opaque_node(IfNode* iff); static void get_assertion_predicates(Node* predicate, Unique_Node_List& list, bool get_opaque = false); void update_main_loop_assertion_predicates(Node* ctrl, CountedLoopNode* loop_head, Node* init, int stride_con); @@ -1759,14 +1758,12 @@ public: void finish_clone_loop(Node_List* split_if_set, Node_List* split_bool_set, Node_List* split_cex_set); - bool clone_cmp_down(Node* n, const Node* blk1, const Node* blk2); - - void clone_loadklass_nodes_at_cmp_index(const Node* n, Node* cmp, int i); - - bool clone_cmp_loadklass_down(Node* n, const Node* blk1, const Node* blk2); - bool at_relevant_ctrl(Node* n, const Node* blk1, const Node* blk2); + bool clone_cmp_loadklass_down(Node* n, const Node* blk1, const Node* blk2); + void clone_loadklass_nodes_at_cmp_index(const Node* n, Node* cmp, int i); + bool clone_cmp_down(Node* n, const Node* blk1, const Node* blk2); + void clone_template_assertion_predicate_expression_down(Node* node); Node* similar_subtype_check(const Node* x, Node* r_in); diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp index 19b9a11d9ce..bb58c2ff8ce 100644 --- a/src/hotspot/share/opto/node.hpp +++ b/src/hotspot/share/opto/node.hpp @@ -1708,6 +1708,22 @@ public: if( !_in_worklist.test_set(b->_idx) ) Node_List::push(b); } + void push_non_cfg_inputs_of(const Node* node) { + for (uint i = 1; i < node->req(); i++) { + Node* input = node->in(i); + if (input != nullptr && !input->is_CFG()) { + push(input); + } + } + } + + void push_outputs_of(const Node* node) { + for (DUIterator_Fast imax, i = node->fast_outs(imax); i < imax; i++) { + Node* output = node->fast_out(i); + push(output); + } + } + Node *pop() { if( _clock_index >= size() ) _clock_index = 0; Node *b = at(_clock_index); diff --git a/src/hotspot/share/opto/predicates.cpp b/src/hotspot/share/opto/predicates.cpp index 997a05dc1f2..7f873e1fb6f 100644 --- a/src/hotspot/share/opto/predicates.cpp +++ b/src/hotspot/share/opto/predicates.cpp @@ -323,7 +323,7 @@ Opaque4Node* TemplateAssertionPredicateExpression::clone(const TransformStrategy auto is_opaque_loop_node = [](const Node* node) { return node->is_Opaque1(); }; - DataNodesOnPathsToTargets data_nodes_on_path_to_targets(TemplateAssertionPredicateExpression::maybe_contains, + DataNodesOnPathsToTargets data_nodes_on_path_to_targets(TemplateAssertionPredicateExpressionNode::is_maybe_in_expression, is_opaque_loop_node); const Unique_Node_List& collected_nodes = data_nodes_on_path_to_targets.collect(_opaque4_node); DataNodeGraph data_node_graph(collected_nodes, phase); @@ -332,3 +332,25 @@ Opaque4Node* TemplateAssertionPredicateExpression::clone(const TransformStrategy Node* opaque4_clone = *orig_to_new.get(_opaque4_node); return opaque4_clone->as_Opaque4(); } + +// Check if this node belongs a Template Assertion Predicate Expression (including OpaqueLoop* nodes). +bool TemplateAssertionPredicateExpressionNode::is_in_expression(Node* node) { + if (is_maybe_in_expression(node)) { + ResourceMark rm; + Unique_Node_List list; + list.push(node); + for (uint i = 0; i < list.size(); i++) { + Node* next = list.at(i); + if (next->is_OpaqueLoopInit() || next->is_OpaqueLoopStride()) { + return true; + } else if (is_maybe_in_expression(next)) { + list.push_non_cfg_inputs_of(next); + } + } + } + return false; +} + +bool TemplateAssertionPredicateExpressionNode::is_template_assertion_predicate(Node* node) { + return node->is_If() && node->in(1)->is_Opaque4(); +} diff --git a/src/hotspot/share/opto/predicates.hpp b/src/hotspot/share/opto/predicates.hpp index a794e31ac3b..670ca58cc47 100644 --- a/src/hotspot/share/opto/predicates.hpp +++ b/src/hotspot/share/opto/predicates.hpp @@ -283,14 +283,41 @@ class TemplateAssertionPredicateExpression : public StackObj { Opaque4Node* clone(const TransformStrategyForOpaqueLoopNodes& transform_strategy, Node* new_ctrl, PhaseIdealLoop* phase); public: - // Is 'n' a node that could be part of a Template Assertion Predicate Expression (i.e. could be found on the input - // chain of a Template Assertion Predicate Opaque4Node up to and including the OpaqueLoop* nodes)? - static bool maybe_contains(const Node* n) { - const int opcode = n->Opcode(); - return (opcode == Op_OpaqueLoopInit || - opcode == Op_OpaqueLoopStride || - n->is_Bool() || - n->is_Cmp() || + Opaque4Node* clone(Node* new_ctrl, PhaseIdealLoop* phase); + Opaque4Node* clone_and_replace_init(Node* new_init, Node* new_ctrl,PhaseIdealLoop* phase); + Opaque4Node* clone_and_replace_init_and_stride(Node* new_init, Node* new_stride, Node* new_ctrl, PhaseIdealLoop* phase); +}; + +// Class to represent a node being part of a Template Assertion Predicate Expression. +// +// The expression itself can belong to no, one, or two Template Assertion Predicates: +// - None: This node is already dead (i.e. we replaced the Bool condition of the Template Assertion Predicate). +// - Two: A OpaqueLoopInitNode could be part of two Template Assertion Predicates. +// - One: In all other cases. +class TemplateAssertionPredicateExpressionNode : public StackObj { + Node* const _node; + + public: + explicit TemplateAssertionPredicateExpressionNode(Node* node) : _node(node) { + assert(is_in_expression(node), "must be valid"); + } + NONCOPYABLE(TemplateAssertionPredicateExpressionNode); + + private: + static bool is_template_assertion_predicate(Node* node); + + public: + // Check whether the provided node is part of a Template Assertion Predicate Expression or not. + static bool is_in_expression(Node* node); + + // Check if the opcode of node could be found in a Template Assertion Predicate Expression. + // This also provides a fast check whether a node is unrelated. + static bool is_maybe_in_expression(const Node* node) { + const int opcode = node->Opcode(); + return (node->is_OpaqueLoopInit() || + node->is_OpaqueLoopStride() || + node->is_Bool() || + node->is_Cmp() || opcode == Op_AndL || opcode == Op_OrL || opcode == Op_RShiftL || @@ -306,9 +333,33 @@ class TemplateAssertionPredicateExpression : public StackObj { opcode == Op_CastII); } - Opaque4Node* clone(Node* new_ctrl, PhaseIdealLoop* phase); - Opaque4Node* clone_and_replace_init(Node* new_init, Node* new_ctrl,PhaseIdealLoop* phase); - Opaque4Node* clone_and_replace_init_and_stride(Node* new_init, Node* new_stride, Node* new_ctrl, PhaseIdealLoop* phase); + // Apply the given function to all Template Assertion Predicates (if any) to which this Template Assertion Predicate + // Expression Node belongs to. + template + void for_each_template_assertion_predicate(Callback callback) { + ResourceMark rm; + Unique_Node_List list; + list.push(_node); + DEBUG_ONLY(int template_counter = 0;) + for (uint i = 0; i < list.size(); i++) { + Node* next = list.at(i); + if (is_template_assertion_predicate(next)) { + callback(next->as_If()); + DEBUG_ONLY(template_counter++;) + } else { + assert(!next->is_CFG(), "no CFG expected in Template Assertion Predicate Expression"); + list.push_outputs_of(next); + } + } + + // Each node inside a Template Assertion Predicate Expression is in between a Template Assertion Predicate and + // its OpaqueLoop* nodes (or an OpaqueLoop* node itself). The OpaqueLoop* nodes do not common up. Therefore, each + // Template Assertion Predicate Expression node belongs to a single expression - except for OpaqueLoopInitNodes. + // An OpaqueLoopInitNode is shared between the init and last value Template Assertion Predicate at creation. + // Later, when cloning the expressions, they are no longer shared. + assert(template_counter <= 2, "a node cannot be part of more than two templates"); + assert(template_counter <= 1 || _node->is_OpaqueLoopInit(), "only OpaqueLoopInit nodes can be part of two templates"); + } }; // This class represents a Predicate Block (i.e. either a Loop Predicate Block, a Profiled Loop Predicate Block, diff --git a/src/hotspot/share/opto/split_if.cpp b/src/hotspot/share/opto/split_if.cpp index 0b7faffda00..70369a2c9c4 100644 --- a/src/hotspot/share/opto/split_if.cpp +++ b/src/hotspot/share/opto/split_if.cpp @@ -95,25 +95,7 @@ bool PhaseIdealLoop::split_up( Node *n, Node *blk1, Node *blk2 ) { return true; } - if (subgraph_has_opaque(n)) { - Unique_Node_List wq; - wq.push(n); - for (uint i = 0; i < wq.size(); i++) { - Node* m = wq.at(i); - if (m->is_If()) { - assert(assertion_predicate_has_loop_opaque_node(m->as_If()), "opaque node not reachable from if?"); - TemplateAssertionPredicateExpression template_assertion_predicate_expression(m->in(1)->as_Opaque4()); - Opaque4Node* cloned_opaque4_node = template_assertion_predicate_expression.clone(m->in(0), this); - _igvn.replace_input_of(m, 1, cloned_opaque4_node); - } else { - assert(!m->is_CFG(), "not CFG expected"); - for (DUIterator_Fast jmax, j = m->fast_outs(jmax); j < jmax; j++) { - Node* u = m->fast_out(j); - wq.push(u); - } - } - } - } + clone_template_assertion_predicate_expression_down(n); if (n->Opcode() == Op_OpaqueZeroTripGuard) { // If this Opaque1 is part of the zero trip guard for a loop: @@ -427,6 +409,27 @@ bool PhaseIdealLoop::clone_cmp_down(Node* n, const Node* blk1, const Node* blk2) return false; } +// 'n' could be a node belonging to a Template Assertion Predicate Expression (i.e. any node between a Template +// Assertion Predicate and its OpaqueLoop* nodes (included)). We cannot simply split this node up since this would +// create a phi node inside the Template Assertion Predicate Expression - making it unrecognizable as such. Therefore, +// we completely clone the entire Template Assertion Predicate Expression "down". This ensures that we have an +// untouched copy that is still recognized by the Template Assertion Predicate matching code. +void PhaseIdealLoop::clone_template_assertion_predicate_expression_down(Node* node) { + if (!TemplateAssertionPredicateExpressionNode::is_in_expression(node)) { + return; + } + + TemplateAssertionPredicateExpressionNode template_assertion_predicate_expression_node(node); + auto clone_expression = [&](IfNode* template_assertion_predicate) { + Opaque4Node* opaque4_node = template_assertion_predicate->in(1)->as_Opaque4(); + TemplateAssertionPredicateExpression template_assertion_predicate_expression(opaque4_node); + Node* new_ctrl = template_assertion_predicate->in(0); + Opaque4Node* cloned_opaque4_node = template_assertion_predicate_expression.clone(new_ctrl, this); + igvn().replace_input_of(template_assertion_predicate, 1, cloned_opaque4_node); + }; + template_assertion_predicate_expression_node.for_each_template_assertion_predicate(clone_expression); +} + //------------------------------register_new_node------------------------------ void PhaseIdealLoop::register_new_node( Node *n, Node *blk ) { assert(!n->is_CFG(), "must be data node"); diff --git a/test/hotspot/jtreg/compiler/predicates/assertion/TestSplitIfCloningDown.java b/test/hotspot/jtreg/compiler/predicates/assertion/TestSplitIfCloningDown.java new file mode 100644 index 00000000000..2e7a887a7ad --- /dev/null +++ b/test/hotspot/jtreg/compiler/predicates/assertion/TestSplitIfCloningDown.java @@ -0,0 +1,117 @@ +/* + * 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 8330004 + * @summary Sanity test to exercise code to clone a Template Assertion Predicate down in Split If. + * @requires vm.compiler2.enabled + * @run main/othervm -Xcomp -XX:LoopMaxUnroll=0 + * -XX:CompileCommand=compileonly,compiler.predicates.assertion.TestSplitIfCloningDown::* + * compiler.predicates.assertion.TestSplitIfCloningDown + */ + +/* + * @test id=no-flags + * @bug 8330004 + * @summary Sanity test to exercise code to clone a Template Assertion Predicate down in Split If. + * @run main compiler.predicates.assertion.TestSplitIfCloningDown + */ + +package compiler.predicates.assertion; + +public class TestSplitIfCloningDown { + static int[] iArr = new int[100]; + static boolean flag; + static int iFld; + + public static void main(String[] args) { + for (int i = 0; i < 10000; i++) { + testPhiIntoNonOpaqueLoopExpressionNode(); + testPhiIntoOpaqueLoopExpressionNode(); + } + } + + + static void testPhiIntoNonOpaqueLoopExpressionNode() { + int zero = 34; + int limit = 2; + for (; limit < 4; limit *= 2); + for (int i = 2; i < limit; i++) { + zero = 0; + } + + + for (int t = 0; t < 100; t++) { // Use outer loop such that OpaqueLoop* will get an earlier ctrl. + iArr = new int[1000]; + + // Replaced by CMove which is an input into Template Assertion Predicate Expression which + // is not an OpaqueLoop* node. Split If will create a phi and tries to split a Template + // Assertion Predicate Expression node -> Need to clone template down. + int a; + if (flag) { + a = 4; + } else { + a = 3; + } + + for (int i = 0; i < 100; i++) { + iArr[i+a] = 34; // Hoisted with Hoisted Check Predicate and Template Assertion Predicate + if (i * zero < iFld) { // Unswitched after Split If to check further template cloning. + return; + } + } + } + } + + // Same as test above but this time the phi inputs into an OpaqueLoop* node and not a node in between. + static void testPhiIntoOpaqueLoopExpressionNode() { + int zero = 34; + int limit = 2; + for (; limit < 4; limit *= 2); + for (int i = 2; i < limit; i++) { + zero = 0; + } + + iArr = new int[1000]; + + // Replaced by CMove which is an input into Template Assertion Predicate Expression which + // is not an OpaqueLoop* node. Split If will create a phi and tries to split a Template + // Assertion Predicate Expression node -> Need to clone template down. + int a; + if (flag) { + a = 4; + } else { + a = 3; + } + + for (int i = 0; i < 100; i++) { + iArr[i+a] = 34; // Hoisted with Hoisted Check Predicate and Template Assertion Predicate + if (i * zero < iFld) { // Unswitched after Split If to check further template cloning. + return; + } + } + } +} +