Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -859,8 +859,8 @@ static void hide_strip_mined_loop(OuterStripMinedLoopNode* outer, CountedLoopNod
phase->register_control(new_outer, phase->get_loop(outer), outer->in(LoopNode::EntryControl));
Node* new_le = new IfNode(le->in(0), le->in(1), le->_prob, le->_fcnt);
phase->register_control(new_le, phase->get_loop(le), le->in(0));
phase->lazy_replace(outer, new_outer);
phase->lazy_replace(le, new_le);
phase->replace_node_and_forward_ctrl(outer, new_outer);
phase->replace_node_and_forward_ctrl(le, new_le);
inner->clear_strip_mined();
}

Expand Down Expand Up @@ -1324,7 +1324,7 @@ void ShenandoahBarrierC2Support::pin_and_expand(PhaseIdealLoop* phase) {
Node* backedge = head->in(LoopNode::LoopBackControl);
Node* new_head = new LoopNode(entry, backedge);
phase->register_control(new_head, phase->get_loop(entry), entry);
phase->lazy_replace(head, new_head);
phase->replace_node_and_forward_ctrl(head, new_head);
}
}

Expand Down Expand Up @@ -1777,7 +1777,7 @@ void MemoryGraphFixer::collect_memory_nodes() {
if (u->adr_type() == TypePtr::BOTTOM) {
fix_memory_uses(u, n, n, c);
} else if (_phase->C->get_alias_index(u->adr_type()) == _alias) {
_phase->lazy_replace(u, n);
_phase->igvn().replace_node(u, n);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see, the lazy_replace only did igvn.replace_node for non-ctrl nodes anyway. Since we are dealing with PhiNodes here, we might as well only use igvn.replace_node.

I discovered this, because it hit my !old_node->is_CFG() check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwestrel Do you have an opinion on this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That change looks good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwestrel Thank, good to know :)

--i; --imax;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/loopPredicate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ IfTrueNode* PhaseIdealLoop::create_new_if_for_predicate(const ParsePredicateSucc
}
// Move nodes pinned on the projection or whose control is set to
// the projection to the region.
lazy_replace(uncommon_proj_orig, uncommon_trap);
replace_node_and_forward_ctrl(uncommon_proj_orig, uncommon_trap);
} else {
// Find region's edge corresponding to uncommon_proj
for (; proj_index < uncommon_trap->req(); proj_index++)
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/opto/loopTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4071,7 +4071,7 @@ bool PhaseIdealLoop::intrinsify_fill(IdealLoopTree* lpt) {
Node* outer_sfpt = head->outer_safepoint();
Node* in = outer_sfpt->in(0);
Node* outer_out = head->outer_loop_exit();
lazy_replace(outer_out, in);
replace_node_and_forward_ctrl(outer_out, in);
_igvn.replace_input_of(outer_sfpt, 0, C->top());
}

Expand All @@ -4080,7 +4080,7 @@ bool PhaseIdealLoop::intrinsify_fill(IdealLoopTree* lpt) {
// state of the loop. It's safe in this case to replace it with the
// result_mem.
_igvn.replace_node(store->in(MemNode::Memory), result_mem);
lazy_replace(exit, result_ctrl);
replace_node_and_forward_ctrl(exit, result_ctrl);
_igvn.replace_node(store, result_mem);
// Any uses the increment outside of the loop become the loop limit.
_igvn.replace_node(head->incr(), head->limit());
Expand Down
9 changes: 5 additions & 4 deletions src/hotspot/share/opto/loopUnswitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -520,11 +520,12 @@ IfTrueNode* PhaseIdealLoop::create_new_if_for_multiversion(IfTrueNode* multivers
region->add_req(new_if_false);
register_control(region, lp, new_multiversion_slow_proj);

// Hook region into slow_path, in stead of the multiversion_slow_proj.
// Hook region into slow_path, instead of the multiversion_slow_proj.
// This also moves all other dependencies of the multiversion_slow_proj to the region.
// The lazy_replace ensures that any get_ctrl that used to have multiversion_slow_proj
// as their control are forwarded to the new region node as their control.
lazy_replace(multiversion_slow_proj, region);
// The replace_node_and_forward_ctrl ensures that any get_ctrl that used to have
// multiversion_slow_proj as their control are forwarded to the new region node as
// their control.
replace_node_and_forward_ctrl(multiversion_slow_proj, region);

return new_if_true;
}
Expand Down
22 changes: 11 additions & 11 deletions src/hotspot/share/opto/loopnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1709,8 +1709,8 @@ LoopNode* PhaseIdealLoop::create_inner_head(IdealLoopTree* loop, BaseCountedLoop
set_loop(new_inner_exit, loop);
set_idom(new_inner_head, idom(head), dom_depth(head));
set_idom(new_inner_exit, idom(exit_test), dom_depth(exit_test));
lazy_replace(head, new_inner_head);
lazy_replace(exit_test, new_inner_exit);
replace_node_and_forward_ctrl(head, new_inner_head);
replace_node_and_forward_ctrl(exit_test, new_inner_exit);
loop->_head = new_inner_head;
return new_inner_head;
}
Expand Down Expand Up @@ -2382,7 +2382,7 @@ bool PhaseIdealLoop::is_counted_loop(Node* x, IdealLoopTree*&loop, BasicType iv_
return false;
}
if (is_deleteable_safept(backedge_sfpt)) {
lazy_replace(backedge_sfpt, iftrue);
replace_node_and_forward_ctrl(backedge_sfpt, iftrue);
if (loop->_safepts != nullptr) {
loop->_safepts->yank(backedge_sfpt);
}
Expand Down Expand Up @@ -2483,8 +2483,8 @@ bool PhaseIdealLoop::is_counted_loop(Node* x, IdealLoopTree*&loop, BasicType iv_
set_loop(iff2, get_loop(iffalse));

// Lazy update of 'get_ctrl' mechanism.
lazy_replace(iffalse, iff2);
lazy_replace(iftrue, ift2);
replace_node_and_forward_ctrl(iffalse, iff2);
replace_node_and_forward_ctrl(iftrue, ift2);

// Swap names
iffalse = iff2;
Expand All @@ -2499,7 +2499,7 @@ bool PhaseIdealLoop::is_counted_loop(Node* x, IdealLoopTree*&loop, BasicType iv_
set_idom(iftrue, le, dd+1);
set_idom(iffalse, le, dd+1);
assert(iff->outcnt() == 0, "should be dead now");
lazy_replace( iff, le ); // fix 'get_ctrl'
replace_node_and_forward_ctrl(iff, le); // fix 'get_ctrl'

Node* entry_control = init_control;
bool strip_mine_loop = iv_bt == T_INT &&
Expand All @@ -2525,7 +2525,7 @@ bool PhaseIdealLoop::is_counted_loop(Node* x, IdealLoopTree*&loop, BasicType iv_
loop->_head = l;
// Fix all data nodes placed at the old loop head.
// Uses the lazy-update mechanism of 'get_ctrl'.
lazy_replace( x, l );
replace_node_and_forward_ctrl(x, l);
set_idom(l, entry_control, dom_depth(entry_control) + 1);

if (iv_bt == T_INT && (LoopStripMiningIter == 0 || strip_mine_loop)) {
Expand All @@ -2551,7 +2551,7 @@ bool PhaseIdealLoop::is_counted_loop(Node* x, IdealLoopTree*&loop, BasicType iv_
register_control(sfpt_clone, outer_ilt, iffalse, body_populated);
set_idom(outer_le, sfpt_clone, dom_depth(sfpt_clone));
}
lazy_replace(sfpt, sfpt->in(TypeFunc::Control));
replace_node_and_forward_ctrl(sfpt, sfpt->in(TypeFunc::Control));
if (loop->_safepts != nullptr) {
loop->_safepts->yank(sfpt);
}
Expand Down Expand Up @@ -3516,7 +3516,7 @@ void OuterStripMinedLoopNode::transform_to_counted_loop(PhaseIterGVN* igvn, Phas
if (iloop == nullptr) {
igvn->replace_node(outer_le, new_end);
} else {
iloop->lazy_replace(outer_le, new_end);
iloop->replace_node_and_forward_ctrl(outer_le, new_end);
}
// the backedge of the inner loop must be rewired to the new loop end
Node* backedge = cle->proj_out(true);
Expand Down Expand Up @@ -4487,7 +4487,7 @@ void IdealLoopTree::remove_safepoints(PhaseIdealLoop* phase, bool keep_one) {
Node* n = sfpts->at(i);
assert(phase->get_loop(n) == this, "");
if (n != keep && phase->is_deleteable_safept(n)) {
phase->lazy_replace(n, n->in(TypeFunc::Control));
phase->replace_node_and_forward_ctrl(n, n->in(TypeFunc::Control));
}
}
}
Expand Down Expand Up @@ -6145,7 +6145,7 @@ void PhaseIdealLoop::build_loop_early( VectorSet &visited, Node_List &worklist,
if( !_verify_only && !_verify_me && ilt->_has_sfpt && n->Opcode() == Op_SafePoint &&
is_deleteable_safept(n)) {
Node *in = n->in(TypeFunc::Control);
lazy_replace(n,in); // Pull safepoint now
replace_node_and_forward_ctrl(n, in); // Pull safepoint now
if (ilt->_safepts != nullptr) {
ilt->_safepts->yank(n);
}
Expand Down
120 changes: 88 additions & 32 deletions src/hotspot/share/opto/loopnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,9 @@ class PhaseIdealLoop : public PhaseTransform {
friend class AutoNodeBudget;

// Map loop membership for CFG nodes, and ctrl for non-CFG nodes.
//
// Exception: dead CFG nodes may instead have a ctrl/idom forwarding
// installed. See: forward_ctrl
Node_List _loop_or_ctrl;

// Pre-computed def-use info
Expand Down Expand Up @@ -956,7 +959,13 @@ class PhaseIdealLoop : public PhaseTransform {

public:
// Set/get control node out. Set lower bit to distinguish from IdealLoopTree
// Returns true if "n" is a data node, false if it's a control node.
// Returns true if "n" is a data node, false if it's a CFG node.
//
// Exception:
// control nodes that are dead because of "replace_node_and_forward_ctrl"
// or have otherwise modified their ctrl state by "forward_ctrl".
// They return "true", because they have a ctrl "forwarding" to the other ctrl node they
// were replaced with.
bool has_ctrl(const Node* n) const { return ((intptr_t)_loop_or_ctrl[n->_idx]) & 1; }

private:
Expand Down Expand Up @@ -1070,17 +1079,18 @@ class PhaseIdealLoop : public PhaseTransform {
}
set_ctrl(n, ctrl);
}
// Control nodes can be replaced or subsumed. During this pass they
// get their replacement Node in slot 1. Instead of updating the block
// location of all Nodes in the subsumed block, we lazily do it. As we
// pull such a subsumed block out of the array, we write back the final
// correct block.

// Retrieves the ctrl for a data node i.
Node* get_ctrl(const Node* i) {
assert(has_node(i), "");
Node *n = get_ctrl_no_update(i);
assert(has_node(i) && has_ctrl(i), "must be data node with ctrl");
Node* n = get_ctrl_no_update(i);
// We store the found ctrl in the side-table again. In most cases,
// this is a no-op, since we just read from _loop_or_ctrl. But in cases
// where there was a ctrl forwarding via dead ctrl nodes, this shortens the path.
// See: forward_ctrl
_loop_or_ctrl.map(i->_idx, (Node*)((intptr_t)n + 1));
assert(has_node(i) && has_ctrl(i), "");
assert(n == find_non_split_ctrl(n), "must return legal ctrl" );
assert(has_node(i) && has_ctrl(i), "must still be data node with ctrl");
assert(n == find_non_split_ctrl(n), "must return legal ctrl");
return n;
}

Expand All @@ -1097,24 +1107,34 @@ class PhaseIdealLoop : public PhaseTransform {
}
}

private:
Node* get_ctrl_no_update_helper(const Node* i) const {
assert(has_ctrl(i), "should be control, not loop");
// We expect only data nodes (which must have a ctrl set), or
// dead ctrl nodes that have a ctrl "forwarding".
// See: forward_ctrl.
assert(has_ctrl(i), "only data nodes or ctrl nodes with ctrl forwarding expected");
return (Node*)(((intptr_t)_loop_or_ctrl[i->_idx]) & ~1);
}

// Compute the ctrl of node i, jumping over ctrl forwardings.
Node* get_ctrl_no_update(const Node* i) const {
assert( has_ctrl(i), "" );
Node *n = get_ctrl_no_update_helper(i);
if (!n->in(0)) {
// Skip dead CFG nodes
assert(has_ctrl(i), "only data nodes expected");
Node* n = get_ctrl_no_update_helper(i);
if (n->in(0) == nullptr) {
// We encountered a dead CFG node.
// If everything went right, this dead CFG node should have had a ctrl
// forwarding installed, using "forward_ctrl". We now have to jump from
// the old (dead) ctrl node to the new (live) ctrl node, in possibly
// multiple ctrl forwarding steps.
do {
n = get_ctrl_no_update_helper(n);
} while (!n->in(0));
} while (n->in(0) == nullptr);
n = find_non_split_ctrl(n);
}
return n;
}

public:
// Check for loop being set
// "n" must be a control node. Returns true if "n" is known to be in a loop.
bool has_loop( Node *n ) const {
Expand All @@ -1125,19 +1145,43 @@ class PhaseIdealLoop : public PhaseTransform {
void set_loop( Node *n, IdealLoopTree *loop ) {
_loop_or_ctrl.map(n->_idx, (Node*)loop);
}
// Lazy-dazy update of 'get_ctrl' and 'idom_at' mechanisms. Replace
// the 'old_node' with 'new_node'. Kill old-node. Add a reference
// from old_node to new_node to support the lazy update. Reference
// replaces loop reference, since that is not needed for dead node.
void lazy_update(Node *old_node, Node *new_node) {

// Install a ctrl "forwarding" from an old (dead) control node.
// This is a "lazy" update of the "get_ctrl" and "idom" mechanism:
// - Install a forwarding from old_node (dead ctrl) to new_node.
// - When querying "get_ctrl": jump from data node over possibly
// multiple dead ctrl nodes with ctrl forwarding to eventually
// reach a live ctrl node. Shorten the path to avoid chasing the
// forwarding in the future.
// - When querying "idom": from some node get its old idom, which
// may be dead but has an idom forwarding to the new and live
// idom. Shorten the path to avoid chasing the forwarding in the
// future.
// Note: while the "idom" information is stored in the "_idom"
// side-table, the idom forwarding piggybacks on the ctrl
// forwarding on "_loop_or_ctrl".
// Using "forward_ctrl" allows us to only edit the entry for the old
// dead node now, and we do not have to update all the nodes that had
// the old_node as their "get_ctrl" or "idom". We clean up the forwarding
// links when we query "get_ctrl" or "idom" for these nodes the next time.
void forward_ctrl(Node* old_node, Node* new_node) {
assert(!has_ctrl(old_node) && old_node->is_CFG() && old_node->in(0) == nullptr,
"must be dead ctrl (CFG) node");
assert(!has_ctrl(new_node) && new_node->is_CFG() && new_node->in(0) != nullptr,
"must be live ctrl (CFG) node");
assert(old_node != new_node, "no cycles please");
// Re-use the side array slot for this node to provide the
// forwarding pointer.
_loop_or_ctrl.map(old_node->_idx, (Node*)((intptr_t)new_node + 1));
assert(has_ctrl(old_node), "must have installed ctrl forwarding");
}
void lazy_replace(Node *old_node, Node *new_node) {

// Replace the old ctrl node with a new ctrl node.
// - Update the node inputs of all uses.
// - Lazily update the ctrl and idom info of all uses, via a ctrl/idom forwarding.
void replace_node_and_forward_ctrl(Node* old_node, Node* new_node) {
_igvn.replace_node(old_node, new_node);
lazy_update(old_node, new_node);
forward_ctrl(old_node, new_node);
}

private:
Expand Down Expand Up @@ -1208,29 +1252,41 @@ class PhaseIdealLoop : public PhaseTransform {

Node* insert_convert_node_if_needed(BasicType target, Node* input);

public:
Node* idom_no_update(Node* d) const {
return idom_no_update(d->_idx);
}

Node* idom_no_update(uint didx) const {
assert(didx < _idom_size, "oob");
Node* n = _idom[didx];
Node* idom_no_update(uint node_idx) const {
assert(node_idx < _idom_size, "oob");
Node* n = _idom[node_idx];
assert(n != nullptr,"Bad immediate dominator info.");
while (n->in(0) == nullptr) { // Skip dead CFG nodes
// We encountered a dead CFG node.
// If everything went right, this dead CFG node should have had an idom
// forwarding installed, using "forward_ctrl". We now have to jump from
// the old (dead) idom node to the new (live) idom node, in possibly
// multiple idom forwarding steps.
// Note that we piggyback on "_loop_or_ctrl" to do the forwarding,
// since we forward both "get_ctrl" and "idom" from the dead to the
// new live ctrl/idom nodes.
n = (Node*)(((intptr_t)_loop_or_ctrl[n->_idx]) & ~1);
assert(n != nullptr,"Bad immediate dominator info.");
}
return n;
}

Node *idom(Node* d) const {
return idom(d->_idx);
public:
Node* idom(Node* n) const {
return idom(n->_idx);
}

Node *idom(uint didx) const {
Node *n = idom_no_update(didx);
_idom[didx] = n; // Lazily remove dead CFG nodes from table.
Node* idom(uint node_idx) const {
Node* n = idom_no_update(node_idx);
// We store the found idom in the side-table again. In most cases,
// this is a no-op, since we just read from _idom. But in cases where
// there was an idom forwarding via dead idom nodes, this shortens the path.
// See: forward_ctrl
_idom[node_idx] = n;
return n;
}

Expand Down
11 changes: 6 additions & 5 deletions src/hotspot/share/opto/loopopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2742,12 +2742,13 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l
}

assert(use->is_Proj(), "loop exit should be projection");
// lazy_replace() below moves all nodes that are:
// replace_node_and_forward_ctrl() below moves all nodes that are:
// - control dependent on the loop exit or
// - have control set to the loop exit
// below the post-loop merge point. lazy_replace() takes a dead control as first input. To make it
// possible to use it, the loop exit projection is cloned and becomes the new exit projection. The initial one
// becomes dead and is "replaced" by the region.
// below the post-loop merge point.
// replace_node_and_forward_ctrl() takes a dead control as first input.
// To make it possible to use it, the loop exit projection is cloned and becomes the
// new exit projection. The initial one becomes dead and is "replaced" by the region.
Node* use_clone = use->clone();
register_control(use_clone, use_loop, idom(use), dom_depth(use));
// Now finish up 'r'
Expand All @@ -2756,7 +2757,7 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l
_igvn.register_new_node_with_optimizer(r);
set_loop(r, use_loop);
set_idom(r, (side_by_side_idom == nullptr) ? newuse->in(0) : side_by_side_idom, dd_r);
lazy_replace(use, r);
replace_node_and_forward_ctrl(use, r);
// Map the (cloned) old use to the new merge point
old_new.map(use_clone->_idx, r);
} // End of if a loop-exit test
Expand Down
Loading