Skip to content

Commit

Permalink
Keep Template Predicates when last value is a ConI, otherwise we coul…
Browse files Browse the repository at this point in the history 
…d get an assert, wrong data updates and possibly miss initialized asseriton predicates
  • Loading branch information
@chhagedorn
chhagedorn committed Sep 27, 2023
1 parent 86a6e89 commit 4a0cca1
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 32 deletions.
12 changes: 5 additions & 7 deletions src/hotspot/share/opto/cfgnode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2720,20 +2720,20 @@ const RegMask &GotoNode::out_RegMask() const {

TemplateAssertionPredicateNode::TemplateAssertionPredicateNode(Node* control, BoolNode* bool_init_value,
BoolNode* bool_last_value,
int initialized_init_value_opcode,
int initialized_last_value_opcode)
const int initialized_init_value_opcode,
const int initialized_last_value_opcode)
: Node(control, bool_init_value, bool_last_value),
_initialized_init_value_opcode(initialized_init_value_opcode),
_initialized_last_value_opcode(initialized_last_value_opcode),
_useless(false) {
assert(initialized_init_value_opcode == Op_If || initialized_init_value_opcode == Op_RangeCheck, "invalid opcode");
assert(initialized_last_value_opcode == Op_If || initialized_last_value_opcode == Op_RangeCheck, "invalid opcode");
init_class_id(Class_TemplateAssertionPredicate);
init_flags(Flag_is_macro);
}

IfNode* TemplateAssertionPredicateNode::create_initialized_assertion_predicate(
Node* control, OpaqueAssertionPredicateNode* opaque_bool, AssertionPredicateType initialized_assertion_predicate_type) {
Node* control, OpaqueAssertionPredicateNode* opaque_bool,
AssertionPredicateType initialized_assertion_predicate_type) const {
bool create_if_node = true;
switch (initialized_assertion_predicate_type) {
case AssertionPredicateType::Init_value:
Expand All @@ -2751,9 +2751,7 @@ IfNode* TemplateAssertionPredicateNode::create_initialized_assertion_predicate(
}

Node* TemplateAssertionPredicateNode::Identity(PhaseGVN* phase) {
if (phase->C->post_loop_opts_phase() || _useless || in(InitValue)->is_ConI() || in(LastValue)->is_ConI()) {
// Useless or a BoolNode folded to a constant. In the latter case, the Hoisted Check Predicate is either always true
// or always false. In both cases, we can remove this node because we do not need to create Assertion Predicates from.
if (phase->C->post_loop_opts_phase() || _useless) {
return in(0);
} else {
phase->C->record_for_post_loop_opts_igvn(this);
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/cfgnode.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ class TemplateAssertionPredicateNode : public Node {
int initialized_init_value_opcode, int initialized_last_value_opcode);

IfNode* create_initialized_assertion_predicate(Node* control, OpaqueAssertionPredicateNode* opaque_bool,
AssertionPredicateType initialized_assertion_predicate_type);
AssertionPredicateType initialized_assertion_predicate_type) const;

void mark_useless() {
_useless = true;
Expand Down
20 changes: 10 additions & 10 deletions src/hotspot/share/opto/predicates.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ Node* AssertionPredicates::create_stride(const int stride_con) {
TemplateAssertionPredicateBool::TemplateAssertionPredicateBool(Node* source_bool) : _source_bool(source_bool->isa_Bool()) {
#ifdef ASSERT
// We could have already folded the BoolNode to a constant.
if (is_valid()) {
if (is_not_dead()) {
// During IGVN, we could have multiple outputs of the _source_bool, for example, when the backedge of the loop is
// this Template Assertion Predicate is about to die and the CastII on the last value bool already folded to a
// constant (i.e. no OpaqueLoop* nodes anymore). Then IGVN could already have commoned up the bool with the bool of
Expand Down Expand Up @@ -471,7 +471,7 @@ class CloneAssertionPredicateBool : public StackObj {
}
}

bool is_cloned_node(Node* node) {
bool is_cloned_node(Node* node) const {
return node->_idx >= _idx_before_cloning;
}

Expand Down Expand Up @@ -555,7 +555,7 @@ class CloneOpaqueLoopNodes : public TransformOpaqueLoopNodes {
// Clones this Template Assertion Predicate bool. This includes all nodes from the BoolNode to the OpaqueLoop* nodes.
// The cloned nodes are not updated.
BoolNode* TemplateAssertionPredicateBool::clone(Node* new_ctrl, PhaseIdealLoop* phase) {
assert(is_valid(), "must be valid");
assert(is_not_dead(), "must not be dead");
CloneOpaqueLoopNodes clone_opaque_loop_nodes(phase, new_ctrl);
CloneAssertionPredicateBool clone_assertion_predicate_bool(_source_bool, new_ctrl, phase);
return clone_assertion_predicate_bool.clone(&clone_opaque_loop_nodes);
Expand Down Expand Up @@ -590,7 +590,7 @@ class CloneWithNewOpaqueInitInput : public TransformOpaqueLoopNodes {
// not updated.
BoolNode* TemplateAssertionPredicateBool::clone_update_opaque_init(Node* new_ctrl, Node* new_opaque_init_input,
PhaseIdealLoop* phase) {
assert(is_valid(), "must be valid");
assert(is_not_dead(), "must not be dead");
CloneWithNewOpaqueInitInput clone_with_new_opaque_init_input(phase, new_ctrl, new_opaque_init_input);
CloneAssertionPredicateBool clone_assertion_predicate_bool(_source_bool, new_ctrl, phase);
return clone_assertion_predicate_bool.clone(&clone_with_new_opaque_init_input);
Expand All @@ -613,7 +613,7 @@ class RemoveOpaqueLoopNodes : public TransformOpaqueLoopNodes {
// Clones this Template Assertion Predicate bool. This includes all nodes from the BoolNode to the OpaqueLoop* nodes.
// The OpaqueLoop* nodes are not cloned but replaced by their input nodes (i.e. folding the OpaqueLoop* nodes away).
BoolNode* TemplateAssertionPredicateBool::clone_remove_opaque_loop_nodes(Node* new_ctrl, PhaseIdealLoop* phase) {
assert(is_valid(), "must be valid");
assert(is_not_dead(), "must not be dead");
RemoveOpaqueLoopNodes remove_opaque_loop_nodes;
CloneAssertionPredicateBool clone_assertion_predicate_bool(_source_bool, new_ctrl, phase);
return clone_assertion_predicate_bool.clone(&remove_opaque_loop_nodes);
Expand Down Expand Up @@ -663,7 +663,7 @@ class OpaqueLoopStrideNodes : public StackObj {

// Sets 'new_opaque_stride_input' as new input of the OpaqueLoopStride node of this Template Assertion Predicate bool.
void TemplateAssertionPredicateBool::update_opaque_stride(Node* new_opaque_stride_input, PhaseIterGVN* igvn) {
assert(is_valid(), "must be valid");
assert(is_not_dead(), "must not be dead");
UpdateOpaqueStrideInput update_opaque_stride_input(igvn, new_opaque_stride_input);
OpaqueLoopStrideNodes opaque_loop_stride_nodes(_source_bool);
opaque_loop_stride_nodes.findAndVisit(&update_opaque_stride_input);
Expand All @@ -688,19 +688,19 @@ void TemplateAssertionPredicateBool::verify_no_opaque_stride() {
#endif // ASSERT

TemplateAssertionPredicate
TemplateAssertionPredicate::create_and_init(Node* new_ctrl, BoolNode* new_init_bool, BoolNode* new_last_bool,
TemplateAssertionPredicate::create_and_init(Node* new_ctrl, BoolNode* new_init_bool, Node* new_last_value,
TemplateAssertionPredicateDataOutput* node_in_target_loop, PhaseIdealLoop* phase) {
TemplateAssertionPredicateNode* cloned_template = _template_assertion_predicate->clone()->as_TemplateAssertionPredicate();
update_data_dependencies_to_clone(cloned_template, node_in_target_loop, phase);
init_new_template(cloned_template, new_ctrl, new_init_bool, new_last_bool, phase);
init_new_template(cloned_template, new_ctrl, new_init_bool, new_last_value, phase);
return { cloned_template->as_TemplateAssertionPredicate() };
}

void TemplateAssertionPredicate::init_new_template(TemplateAssertionPredicateNode* cloned_template, Node* new_ctrl,
BoolNode* new_init_bool, BoolNode* new_last_bool,
BoolNode* new_init_bool, Node* new_last_value,
PhaseIdealLoop* phase) {
phase->igvn().replace_input_of(cloned_template, TemplateAssertionPredicateNode::InitValue, new_init_bool);
phase->igvn().replace_input_of(cloned_template, TemplateAssertionPredicateNode::LastValue, new_last_bool);
phase->igvn().replace_input_of(cloned_template, TemplateAssertionPredicateNode::LastValue, new_last_value);
phase->igvn().replace_input_of(cloned_template, 0, new_ctrl);
phase->register_control(cloned_template, phase->get_loop(new_ctrl), new_ctrl);
}
Expand Down
41 changes: 27 additions & 14 deletions src/hotspot/share/opto/predicates.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,15 +417,13 @@ class AssertionPredicateBoolOpcodes : public StackObj {
class TemplateAssertionPredicateBool : public StackObj {
BoolNode* _source_bool;

#ifdef ASSERT
bool is_valid() const {
return _source_bool != nullptr;
}
#endif // ASSERT

public:
explicit TemplateAssertionPredicateBool(Node* source_bool);

bool is_not_dead() const {
return _source_bool != nullptr;
}

BoolNode* clone(Node* new_ctrl, PhaseIdealLoop* phase);
BoolNode* clone_update_opaque_init(Node* new_ctrl, Node* new_opaque_init_input, PhaseIdealLoop* phase);
BoolNode* clone_remove_opaque_loop_nodes(Node* new_ctrl, PhaseIdealLoop* phase);
Expand All @@ -439,10 +437,10 @@ class TemplateAssertionPredicate : public Predicate {
TemplateAssertionPredicateBool _init_value_bool;
TemplateAssertionPredicateBool _last_value_bool;

TemplateAssertionPredicate create_and_init(Node* new_ctrl, BoolNode* new_init_bool, BoolNode* new_last_bool,
TemplateAssertionPredicate create_and_init(Node* new_ctrl, BoolNode* new_init_bool, Node* new_last_value,
TemplateAssertionPredicateDataOutput* node_in_target_loop, PhaseIdealLoop* phase);
static void init_new_template(TemplateAssertionPredicateNode* cloned_template, Node* new_ctrl, BoolNode* new_init_bool,
BoolNode* new_last_bool, PhaseIdealLoop* phase);
Node* new_last_value, PhaseIdealLoop* phase);
void update_data_dependencies_to_clone(TemplateAssertionPredicateNode* cloned_template_assertion_predicate,
TemplateAssertionPredicateDataOutput* node_in_target_loop, PhaseIdealLoop* phase);
void create_initialized_predicate(Node* new_ctrl, PhaseIdealLoop* phase, TemplateAssertionPredicateBool &template_bool,
Expand Down Expand Up @@ -470,33 +468,48 @@ class TemplateAssertionPredicate : public Predicate {
// Clones this Template Assertion Predicate which will also clone its bools. The cloned nodes are not updated in any way.
TemplateAssertionPredicate clone(Node* new_ctrl, TemplateAssertionPredicateDataOutput* node_in_target_loop, PhaseIdealLoop* phase) {
BoolNode* new_init_bool = _init_value_bool.clone(new_ctrl, phase);
BoolNode* new_last_bool = _last_value_bool.clone(new_ctrl, phase);
return create_and_init(new_ctrl, new_init_bool, new_last_bool, node_in_target_loop, phase);
Node* new_last_value;
if (_last_value_bool.is_not_dead()) {
new_last_value = _last_value_bool.clone(new_ctrl, phase);
} else {
new_last_value = phase->igvn().intcon(1);
}
return create_and_init(new_ctrl, new_init_bool, new_last_value, node_in_target_loop, phase);
}

// Same as clone() but the cloned OpaqueLoopInitNode nodes will get 'new_opaque_cloned nodes' as new input.
TemplateAssertionPredicate clone_update_opaque_init(Node* new_ctrl, Node* new_opaque_init_input,
TemplateAssertionPredicateDataOutput* node_in_target_loop,
PhaseIdealLoop* phase) {
BoolNode* new_init_bool = _init_value_bool.clone_update_opaque_init(new_ctrl, new_opaque_init_input, phase);
BoolNode* new_last_bool = _last_value_bool.clone_update_opaque_init(new_ctrl, new_opaque_init_input, phase);
return create_and_init(new_ctrl, new_init_bool, new_last_bool, node_in_target_loop, phase);
Node* new_last_value;
if (_last_value_bool.is_not_dead()) {
new_last_value = _last_value_bool.clone_update_opaque_init(new_ctrl, new_opaque_init_input, phase);
} else {
new_last_value = phase->igvn().intcon(1);
}
return create_and_init(new_ctrl, new_init_bool, new_last_value, node_in_target_loop, phase);
}

// Update the input of the OpaqueLoopStrideNode of the last value bool of this Template Assertion Predicate to
// the provided 'new_opaque_stride_input'.
void update_opaque_stride(Node* new_opaque_stride_input, PhaseIterGVN* igvn) {
// Only last value bool has OpaqueLoopStrideNode.
DEBUG_ONLY(_init_value_bool.verify_no_opaque_stride());
_last_value_bool.update_opaque_stride(new_opaque_stride_input, igvn);
if (_last_value_bool.is_not_dead()) {
_last_value_bool.update_opaque_stride(new_opaque_stride_input, igvn);
}
}

// Create an Initialized Assertion Predicate from this Template Assertion Predicate for the init and the last value.
// This is done by cloning the Template Assertion Predicate bools and removing the OpaqueLoop* nodes (i.e. folding
// them away and using their inputs instead).
void initialize(PhaseIdealLoop* phase, PredicateChain& predicate_chain) {
Node* new_ctrl = entry();
create_initialized_predicate(new_ctrl, phase, _last_value_bool, AssertionPredicateType::Last_value, predicate_chain);
if (_last_value_bool.is_not_dead()) {
create_initialized_predicate(new_ctrl, phase, _last_value_bool, AssertionPredicateType::Last_value,
predicate_chain);
}
create_initialized_predicate(new_ctrl, phase, _init_value_bool, AssertionPredicateType::Init_value, predicate_chain);
}

Expand Down

0 comments on commit 4a0cca1

Please sign in to comment.