From 3c03c0717e171208ef377331b5781c3c9dd82eed Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Mon, 20 Oct 2025 10:57:26 +0200 Subject: [PATCH 01/16] JDK-8370220 --- src/hotspot/share/opto/loopnode.hpp | 34 ++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 2645df86d96fc..77c8528edefad 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -957,6 +957,11 @@ 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. + // + // Exception: control nodes that are dead because of "lazy_replace" or have + // otherwise modified their ctrl state by "lazy_update". 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: @@ -1098,18 +1103,28 @@ class PhaseIdealLoop : public PhaseTransform { } 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 lazy_replace. + assert(has_ctrl(i), "only data nodes or ctrl nodes with ctrl forwarding expected"); return (Node*)(((intptr_t)_loop_or_ctrl[i->_idx]) & ~1); } + // TODO: desc 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)) { + assert(has_ctrl(i), "only data nodes expected"); + Node* n = get_ctrl_no_update_helper(i); + if (n->in(0) == nullptr) { // Skip dead CFG nodes + // With "lazy_update" or "lazy_replace", we may have created a + // ctrl "forwarding" from the dead node to some other node. This + // "forwarding" ensures that get_ctrl responds with the other node + // instead of with the dead node, lazily moving all those "controllees" + // from the dead ctrl node to the other ctrl node. + // There may be repeated "forwardings", so we must loop until we find + // a live ctrl node. 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; @@ -1129,12 +1144,19 @@ class PhaseIdealLoop : public PhaseTransform { // 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) { + // TODO: desc + // + // Install a ctrl "forwarding" from an old control node + void lazy_update(Node* old_node, Node* new_node) { + assert(!has_ctrl(old_node), "only expect live ctrl nodes"); 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"); } + + // TODO: desc void lazy_replace(Node *old_node, Node *new_node) { _igvn.replace_node(old_node, new_node); lazy_update(old_node, new_node); From d44a9cd00ea7dc5a616dba8834f690430f27ad1c Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Mon, 20 Oct 2025 14:05:09 +0200 Subject: [PATCH 02/16] more documentation --- src/hotspot/share/opto/loopnode.hpp | 74 ++++++++++++++++++----------- src/hotspot/share/opto/split_if.cpp | 2 +- 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 77c8528edefad..40b166716ba35 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1075,17 +1075,26 @@ 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. + + // Retreives the ctrl for a data node i. + // The ctrl info is stored in the _loop_or_ctrl side-table. However, it + // could be that the ctrl of the node i has been replaced or subsumed by + // another ctrl node (see lazy_replace). Instead of updating all affected + // data nodes that used to have the old ctrl, we simply install a ctrl + // forwarding at the old ctrl. When computing get_ctrl, we then have to + // trace over those forwardings, and eventually find the new live ctrl. + // In the end, we can update the ctrl of i in _loop_or_ctrl, dropping the + // extra steps required with the forwarding, so that a next query returns + // immediately (this shortens the path). 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. _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; } @@ -1109,19 +1118,16 @@ class PhaseIdealLoop : public PhaseTransform { return (Node*)(((intptr_t)_loop_or_ctrl[i->_idx]) & ~1); } - // TODO: desc + // Compute the ctrl of node i, jumping over ctrl forwardings. Node* get_ctrl_no_update(const Node* i) const { assert(has_ctrl(i), "only data nodes expected"); Node* n = get_ctrl_no_update_helper(i); if (n->in(0) == nullptr) { - // Skip dead CFG nodes - // With "lazy_update" or "lazy_replace", we may have created a - // ctrl "forwarding" from the dead node to some other node. This - // "forwarding" ensures that get_ctrl responds with the other node - // instead of with the dead node, lazily moving all those "controllees" - // from the dead ctrl node to the other ctrl node. - // There may be repeated "forwardings", so we must loop until we find - // a live ctrl node. + // We encountered a dead CFG node. + // If everything went right, this dead CFG node should have had a ctrl + // forwarding installed, using "lazy_replace" or "lazy_update". 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) == nullptr); @@ -1140,15 +1146,27 @@ 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. - // TODO: desc - // - // Install a ctrl "forwarding" from an old control 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 a ctrl forwarding to the new and live + // idom. Shorten the path to avoid chasing the forwarding in the + // future. + // Using "lazy_update" 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". void lazy_update(Node* old_node, Node* new_node) { - assert(!has_ctrl(old_node), "only expect live ctrl nodes"); + assert(!has_ctrl(old_node), "must be ctrl node"); + assert(!has_ctrl(new_node), "must be ctrl node"); + assert(old_node->in(0) == nullptr, "old node must be dead"); + assert(new_node->in(0) != nullptr, "new node must be live"); assert(old_node != new_node, "no cycles please"); // Re-use the side array slot for this node to provide the // forwarding pointer. @@ -1156,7 +1174,9 @@ class PhaseIdealLoop : public PhaseTransform { assert(has_ctrl(old_node), "must have installed ctrl forwarding"); } - // TODO: desc + // 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 forwarding. void lazy_replace(Node *old_node, Node *new_node) { _igvn.replace_node(old_node, new_node); lazy_update(old_node, new_node); diff --git a/src/hotspot/share/opto/split_if.cpp b/src/hotspot/share/opto/split_if.cpp index bede04c6b2c54..0411abf43794a 100644 --- a/src/hotspot/share/opto/split_if.cpp +++ b/src/hotspot/share/opto/split_if.cpp @@ -670,8 +670,8 @@ void PhaseIdealLoop::do_split_if(Node* iff, RegionNode** new_false_region, Regio _igvn.remove_dead_node(new_iff); // Lazy replace IDOM info with the region's dominator lazy_replace(iff, region_dom); - lazy_update(region, region_dom); // idom must be update before handle_uses region->set_req(0, nullptr); // Break the self-cycle. Required for lazy_update to work on region + lazy_update(region, region_dom); // idom must be update before handle_uses // Now make the original merge point go dead, by handling all its uses. small_cache region_cache; From 6e86c123d651e9322daa6afb3e537e7a0290cd8b Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Mon, 20 Oct 2025 14:16:53 +0200 Subject: [PATCH 03/16] wip documentation and renaming --- src/hotspot/share/opto/loopnode.hpp | 28 +++++++++++++++++----------- src/hotspot/share/opto/split_if.cpp | 5 +++-- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 40b166716ba35..1832e3c1d3159 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -959,9 +959,9 @@ class PhaseIdealLoop : public PhaseTransform { // Returns true if "n" is a data node, false if it's a control node. // // Exception: control nodes that are dead because of "lazy_replace" or have - // otherwise modified their ctrl state by "lazy_update". They return "true", - // because they have a ctrl "forwarding" to the other ctrl node they were - // replaced with. + // otherwise modified their ctrl state by "install_lazy_ctrl_and_idom_forwarding". + // 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: @@ -1125,8 +1125,8 @@ class PhaseIdealLoop : public PhaseTransform { 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 "lazy_replace" or "lazy_update". We now - // have to jump from the old (dead) ctrl node to the new (live) ctrl node, + // forwarding installed, using "lazy_replace" or "install_lazy_ctrl_and_idom_forwarding". + // 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); @@ -1158,11 +1158,11 @@ class PhaseIdealLoop : public PhaseTransform { // may be dead but has a ctrl forwarding to the new and live // idom. Shorten the path to avoid chasing the forwarding in the // future. - // Using "lazy_update" 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". - void lazy_update(Node* old_node, Node* new_node) { + // Using "install_lazy_ctrl_and_idom_forwarding" 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". + void install_lazy_ctrl_and_idom_forwarding(Node* old_node, Node* new_node) { assert(!has_ctrl(old_node), "must be ctrl node"); assert(!has_ctrl(new_node), "must be ctrl node"); assert(old_node->in(0) == nullptr, "old node must be dead"); @@ -1179,7 +1179,7 @@ class PhaseIdealLoop : public PhaseTransform { // - Lazily update the ctrl and idom info of all uses, via a ctrl forwarding. void lazy_replace(Node *old_node, Node *new_node) { _igvn.replace_node(old_node, new_node); - lazy_update(old_node, new_node); + install_lazy_ctrl_and_idom_forwarding(old_node, new_node); } private: @@ -1260,6 +1260,11 @@ class PhaseIdealLoop : public PhaseTransform { Node* n = _idom[didx]; 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 a idom/ctrl + // forwarding installed, using "lazy_replace" or "install_lazy_ctrl_and_idom_forwarding". + // We now have to jump from the old (dead) ctrl node to the new (live) ctrl node, + // in possibly multiple ctrl/idom forwarding steps. n = (Node*)(((intptr_t)_loop_or_ctrl[n->_idx]) & ~1); assert(n != nullptr,"Bad immediate dominator info."); } @@ -1272,6 +1277,7 @@ class PhaseIdealLoop : public PhaseTransform { Node *idom(uint didx) const { Node *n = idom_no_update(didx); + _idom[didx] = n; // Lazily remove dead CFG nodes from table. return n; } diff --git a/src/hotspot/share/opto/split_if.cpp b/src/hotspot/share/opto/split_if.cpp index 0411abf43794a..e3b0683fbd034 100644 --- a/src/hotspot/share/opto/split_if.cpp +++ b/src/hotspot/share/opto/split_if.cpp @@ -670,8 +670,9 @@ void PhaseIdealLoop::do_split_if(Node* iff, RegionNode** new_false_region, Regio _igvn.remove_dead_node(new_iff); // Lazy replace IDOM info with the region's dominator lazy_replace(iff, region_dom); - region->set_req(0, nullptr); // Break the self-cycle. Required for lazy_update to work on region - lazy_update(region, region_dom); // idom must be update before handle_uses + // Break the self-cycle. Required for install_lazy_ctrl_and_idom_forwarding to work on region. + region->set_req(0, nullptr); + install_lazy_ctrl_and_idom_forwarding(region, region_dom); // idom must be updated before handle_use // Now make the original merge point go dead, by handling all its uses. small_cache region_cache; From 3bd0435c1e030dd485ac1b760c7c09018d4fa904 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Mon, 20 Oct 2025 14:26:43 +0200 Subject: [PATCH 04/16] make helper method private --- src/hotspot/share/opto/loopnode.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 1832e3c1d3159..25e974554b722 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1111,6 +1111,7 @@ class PhaseIdealLoop : public PhaseTransform { } } +private: Node* get_ctrl_no_update_helper(const Node* i) const { // We expect only data nodes (which must have a ctrl set), or // dead ctrl nodes that have a ctrl "forwarding", see lazy_replace. @@ -1118,6 +1119,7 @@ class PhaseIdealLoop : public PhaseTransform { return (Node*)(((intptr_t)_loop_or_ctrl[i->_idx]) & ~1); } +public: // Compute the ctrl of node i, jumping over ctrl forwardings. Node* get_ctrl_no_update(const Node* i) const { assert(has_ctrl(i), "only data nodes expected"); From 5605adbc99448993bcaf6518cae5a0b8fcf03e24 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Mon, 20 Oct 2025 14:40:57 +0200 Subject: [PATCH 05/16] rename lazy methods --- src/hotspot/share/opto/loopPredicate.cpp | 2 +- src/hotspot/share/opto/loopTransform.cpp | 4 +-- src/hotspot/share/opto/loopnode.cpp | 22 ++++++------- src/hotspot/share/opto/loopnode.hpp | 42 ++++++++++-------------- src/hotspot/share/opto/loopopts.cpp | 11 ++++--- src/hotspot/share/opto/split_if.cpp | 4 +-- 6 files changed, 40 insertions(+), 45 deletions(-) diff --git a/src/hotspot/share/opto/loopPredicate.cpp b/src/hotspot/share/opto/loopPredicate.cpp index 561f3ce75cb3a..94b24765590ee 100644 --- a/src/hotspot/share/opto/loopPredicate.cpp +++ b/src/hotspot/share/opto/loopPredicate.cpp @@ -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_ctrl_node_and_forward_ctrl_and_idom(uncommon_proj_orig, uncommon_trap); } else { // Find region's edge corresponding to uncommon_proj for (; proj_index < uncommon_trap->req(); proj_index++) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index f92833e9e1c0a..101e2738558bf 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -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_ctrl_node_and_forward_ctrl_and_idom(outer_out, in); _igvn.replace_input_of(outer_sfpt, 0, C->top()); } @@ -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_ctrl_node_and_forward_ctrl_and_idom(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()); diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index a3e3be66583b7..4fba5915ba989 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -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_ctrl_node_and_forward_ctrl_and_idom(head, new_inner_head); + replace_ctrl_node_and_forward_ctrl_and_idom(exit_test, new_inner_exit); loop->_head = new_inner_head; return new_inner_head; } @@ -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_ctrl_node_and_forward_ctrl_and_idom(backedge_sfpt, iftrue); if (loop->_safepts != nullptr) { loop->_safepts->yank(backedge_sfpt); } @@ -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_ctrl_node_and_forward_ctrl_and_idom(iffalse, iff2); + replace_ctrl_node_and_forward_ctrl_and_idom(iftrue, ift2); // Swap names iffalse = iff2; @@ -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_ctrl_node_and_forward_ctrl_and_idom(iff, le); // fix 'get_ctrl' Node* entry_control = init_control; bool strip_mine_loop = iv_bt == T_INT && @@ -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_ctrl_node_and_forward_ctrl_and_idom(x, l); set_idom(l, entry_control, dom_depth(entry_control) + 1); if (iv_bt == T_INT && (LoopStripMiningIter == 0 || strip_mine_loop)) { @@ -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_ctrl_node_and_forward_ctrl_and_idom(sfpt, sfpt->in(TypeFunc::Control)); if (loop->_safepts != nullptr) { loop->_safepts->yank(sfpt); } @@ -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_ctrl_node_and_forward_ctrl_and_idom(outer_le, new_end); } // the backedge of the inner loop must be rewired to the new loop end Node* backedge = cle->proj_out(true); @@ -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_ctrl_node_and_forward_ctrl_and_idom(n, n->in(TypeFunc::Control)); } } } @@ -6156,7 +6156,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_ctrl_node_and_forward_ctrl_and_idom(n, in); // Pull safepoint now if (ilt->_safepts != nullptr) { ilt->_safepts->yank(n); } diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 25e974554b722..bb1961b399066 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -958,10 +958,11 @@ class PhaseIdealLoop : public PhaseTransform { // 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. // - // Exception: control nodes that are dead because of "lazy_replace" or have - // otherwise modified their ctrl state by "install_lazy_ctrl_and_idom_forwarding". - // They return "true", because they have a ctrl "forwarding" to the other ctrl node - // they were replaced with. + // Exception: + // control nodes that are dead because of "replace_ctrl_node_and_forward_ctrl_and_idom" + // or have otherwise modified their ctrl state by "install_lazy_ctrl_and_idom_forwarding". + // 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: @@ -1077,21 +1078,13 @@ class PhaseIdealLoop : public PhaseTransform { } // Retreives the ctrl for a data node i. - // The ctrl info is stored in the _loop_or_ctrl side-table. However, it - // could be that the ctrl of the node i has been replaced or subsumed by - // another ctrl node (see lazy_replace). Instead of updating all affected - // data nodes that used to have the old ctrl, we simply install a ctrl - // forwarding at the old ctrl. When computing get_ctrl, we then have to - // trace over those forwardings, and eventually find the new live ctrl. - // In the end, we can update the ctrl of i in _loop_or_ctrl, dropping the - // extra steps required with the forwarding, so that a next query returns - // immediately (this shortens the path). Node* get_ctrl(const Node* 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: install_lazy_ctrl_and_idom_forwarding _loop_or_ctrl.map(i->_idx, (Node*)((intptr_t)n + 1)); 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"); @@ -1114,12 +1107,12 @@ class PhaseIdealLoop : public PhaseTransform { private: Node* get_ctrl_no_update_helper(const Node* i) const { // We expect only data nodes (which must have a ctrl set), or - // dead ctrl nodes that have a ctrl "forwarding", see lazy_replace. + // dead ctrl nodes that have a ctrl "forwarding". + // See: install_lazy_ctrl_and_idom_forwarding. assert(has_ctrl(i), "only data nodes or ctrl nodes with ctrl forwarding expected"); return (Node*)(((intptr_t)_loop_or_ctrl[i->_idx]) & ~1); } -public: // Compute the ctrl of node i, jumping over ctrl forwardings. Node* get_ctrl_no_update(const Node* i) const { assert(has_ctrl(i), "only data nodes expected"); @@ -1127,9 +1120,9 @@ class PhaseIdealLoop : public PhaseTransform { 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 "lazy_replace" or "install_lazy_ctrl_and_idom_forwarding". - // We now have to jump from the old (dead) ctrl node to the new (live) ctrl node, - // in possibly multiple ctrl forwarding steps. + // forwarding installed, using "install_lazy_ctrl_and_idom_forwarding". + // We now have to jump from the old (dead) ctrl node to the new (live) + // ctrl node, in possibly multiple ctrl/idom forwarding steps. do { n = get_ctrl_no_update_helper(n); } while (n->in(0) == nullptr); @@ -1138,6 +1131,7 @@ class PhaseIdealLoop : public PhaseTransform { 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 { @@ -1178,8 +1172,8 @@ class PhaseIdealLoop : public PhaseTransform { // 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 forwarding. - void lazy_replace(Node *old_node, Node *new_node) { + // - Lazily update the ctrl and idom info of all uses, via a ctrl/idom forwarding. + void replace_ctrl_node_and_forward_ctrl_and_idom(Node *old_node, Node *new_node) { _igvn.replace_node(old_node, new_node); install_lazy_ctrl_and_idom_forwarding(old_node, new_node); } @@ -1252,7 +1246,6 @@ 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); } @@ -1264,15 +1257,16 @@ class PhaseIdealLoop : public PhaseTransform { 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 a idom/ctrl - // forwarding installed, using "lazy_replace" or "install_lazy_ctrl_and_idom_forwarding". - // We now have to jump from the old (dead) ctrl node to the new (live) ctrl node, - // in possibly multiple ctrl/idom forwarding steps. + // forwarding installed, using "install_lazy_ctrl_and_idom_forwarding". + // We now have to jump from the old (dead) ctrl node to the new (live) + // ctrl/idom node, in possibly multiple ctrl/idom forwarding steps. n = (Node*)(((intptr_t)_loop_or_ctrl[n->_idx]) & ~1); assert(n != nullptr,"Bad immediate dominator info."); } return n; } +public: Node *idom(Node* d) const { return idom(d->_idx); } diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index a9baac394a2a8..85d042f69a902 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -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_ctrl_node_and_forward_ctrl_and_idom() 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_ctrl_node_and_forward_ctrl_and_idom() 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' @@ -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_ctrl_node_and_forward_ctrl_and_idom(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 diff --git a/src/hotspot/share/opto/split_if.cpp b/src/hotspot/share/opto/split_if.cpp index e3b0683fbd034..f08446ec50ad2 100644 --- a/src/hotspot/share/opto/split_if.cpp +++ b/src/hotspot/share/opto/split_if.cpp @@ -655,7 +655,7 @@ void PhaseIdealLoop::do_split_if(Node* iff, RegionNode** new_false_region, Regio // Replace in the graph with lazy-update mechanism new_iff->set_req(0, new_iff); // hook self so it does not go dead - lazy_replace(ifp, ifpx); + replace_ctrl_node_and_forward_ctrl_and_idom(ifp, ifpx); new_iff->set_req(0, region); // Record bits for later xforms @@ -669,7 +669,7 @@ void PhaseIdealLoop::do_split_if(Node* iff, RegionNode** new_false_region, Regio } _igvn.remove_dead_node(new_iff); // Lazy replace IDOM info with the region's dominator - lazy_replace(iff, region_dom); + replace_ctrl_node_and_forward_ctrl_and_idom(iff, region_dom); // Break the self-cycle. Required for install_lazy_ctrl_and_idom_forwarding to work on region. region->set_req(0, nullptr); install_lazy_ctrl_and_idom_forwarding(region, region_dom); // idom must be updated before handle_use From c3b87a8b0e8eb5a83cbf3ca2cd0f3eff3c1e132d Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Mon, 20 Oct 2025 14:49:27 +0200 Subject: [PATCH 06/16] missing part --- src/hotspot/share/opto/loopnode.hpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index bb1961b399066..9713bb0fde576 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1273,8 +1273,11 @@ class PhaseIdealLoop : public PhaseTransform { Node *idom(uint didx) const { Node *n = idom_no_update(didx); - - _idom[didx] = n; // Lazily remove dead CFG nodes from table. + // 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 a ctrl forwarding via dead ctrl nodes, this shortens the path. + // See: install_lazy_ctrl_and_idom_forwarding + _idom[didx] = n; return n; } From 21911dc43b378393e236e43e2f196c7ddba0d37d Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Mon, 20 Oct 2025 14:50:08 +0200 Subject: [PATCH 07/16] code style --- src/hotspot/share/opto/loopnode.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 9713bb0fde576..13487823f23ce 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1267,12 +1267,12 @@ class PhaseIdealLoop : public PhaseTransform { } public: - Node *idom(Node* d) const { + Node* idom(Node* d) const { return idom(d->_idx); } - Node *idom(uint didx) const { - Node *n = idom_no_update(didx); + Node* idom(uint didx) const { + Node* n = idom_no_update(didx); // 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 a ctrl forwarding via dead ctrl nodes, this shortens the path. From e0b6ad68f2bdbf51c9e01370d280034228d43eea Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Tue, 21 Oct 2025 09:13:17 +0200 Subject: [PATCH 08/16] fix merge --- src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp | 8 ++++---- src/hotspot/share/opto/loopUnswitch.cpp | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp b/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp index 8210718126bc2..7f1159f60ac33 100644 --- a/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp +++ b/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp @@ -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_ctrl_node_and_forward_ctrl_and_idom(outer, new_outer); + phase->replace_ctrl_node_and_forward_ctrl_and_idom(le, new_le); inner->clear_strip_mined(); } @@ -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_ctrl_node_and_forward_ctrl_and_idom(head, new_head); } } @@ -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->replace_ctrl_node_and_forward_ctrl_and_idom(u, n); --i; --imax; } } diff --git a/src/hotspot/share/opto/loopUnswitch.cpp b/src/hotspot/share/opto/loopUnswitch.cpp index f79afee31039c..d96bae62a1ac5 100644 --- a/src/hotspot/share/opto/loopUnswitch.cpp +++ b/src/hotspot/share/opto/loopUnswitch.cpp @@ -522,9 +522,10 @@ IfTrueNode* PhaseIdealLoop::create_new_if_for_multiversion(IfTrueNode* multivers // Hook region into slow_path, in stead 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_ctrl_node_and_forward_ctrl_and_idom 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_ctrl_node_and_forward_ctrl_and_idom(multiversion_slow_proj, region); return new_if_true; } From d9f7926ecb559d6b2abefb04fd34157046cc7eec Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Tue, 21 Oct 2025 09:26:22 +0200 Subject: [PATCH 09/16] for Christian part 1 --- src/hotspot/share/opto/loopnode.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index ac54b0ed7fecc..eae3e820cb94b 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1159,10 +1159,10 @@ class PhaseIdealLoop : public PhaseTransform { // 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". void install_lazy_ctrl_and_idom_forwarding(Node* old_node, Node* new_node) { - assert(!has_ctrl(old_node), "must be ctrl node"); - assert(!has_ctrl(new_node), "must be ctrl node"); - assert(old_node->in(0) == nullptr, "old node must be dead"); - assert(new_node->in(0) != nullptr, "new node must be live"); + 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. From 73bb42f53769dd4ff5aed3de36efea7531a41cd5 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Tue, 21 Oct 2025 09:30:43 +0200 Subject: [PATCH 10/16] Apply suggestions from code review Co-authored-by: Christian Hagedorn --- src/hotspot/share/opto/loopnode.hpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index eae3e820cb94b..0c3eecb2f9e63 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1077,7 +1077,7 @@ class PhaseIdealLoop : public PhaseTransform { set_ctrl(n, ctrl); } - // Retreives the ctrl for a data node i. + // Retrieves the ctrl for a data node i. Node* get_ctrl(const Node* i) { assert(has_node(i) && has_ctrl(i), "must be data node with ctrl"); Node* n = get_ctrl_no_update(i); @@ -1122,7 +1122,7 @@ class PhaseIdealLoop : public PhaseTransform { // If everything went right, this dead CFG node should have had a ctrl // forwarding installed, using "install_lazy_ctrl_and_idom_forwarding". // We now have to jump from the old (dead) ctrl node to the new (live) - // ctrl node, in possibly multiple ctrl/idom forwarding steps. + // ctrl node, in possibly multiple ctrl forwarding steps. do { n = get_ctrl_no_update_helper(n); } while (n->in(0) == nullptr); @@ -1256,9 +1256,9 @@ class PhaseIdealLoop : public PhaseTransform { 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 a idom/ctrl + // If everything went right, this dead CFG node should have had an idom // forwarding installed, using "install_lazy_ctrl_and_idom_forwarding". - // We now have to jump from the old (dead) ctrl node to the new (live) + // We now have to jump from the old (dead) idom node to the new (live) // ctrl/idom node, in possibly multiple ctrl/idom forwarding steps. n = (Node*)(((intptr_t)_loop_or_ctrl[n->_idx]) & ~1); assert(n != nullptr,"Bad immediate dominator info."); @@ -1267,8 +1267,8 @@ class PhaseIdealLoop : public PhaseTransform { } public: - Node* idom(Node* d) const { - return idom(d->_idx); + Node* idom(Node* n) const { + return idom(n->_idx); } Node* idom(uint didx) const { From c5fb615c90fcf408489a059a6051421037ba7414 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Tue, 21 Oct 2025 09:52:03 +0200 Subject: [PATCH 11/16] more for Christian part 2 --- src/hotspot/share/opto/split_if.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/opto/split_if.cpp b/src/hotspot/share/opto/split_if.cpp index f08446ec50ad2..8e68930dd6b95 100644 --- a/src/hotspot/share/opto/split_if.cpp +++ b/src/hotspot/share/opto/split_if.cpp @@ -670,9 +670,9 @@ void PhaseIdealLoop::do_split_if(Node* iff, RegionNode** new_false_region, Regio _igvn.remove_dead_node(new_iff); // Lazy replace IDOM info with the region's dominator replace_ctrl_node_and_forward_ctrl_and_idom(iff, region_dom); - // Break the self-cycle. Required for install_lazy_ctrl_and_idom_forwarding to work on region. + // Break the self-cycle. Required for install_ctrl_and_idom_forwarding to work on region. region->set_req(0, nullptr); - install_lazy_ctrl_and_idom_forwarding(region, region_dom); // idom must be updated before handle_use + install_ctrl_and_idom_forwarding(region, region_dom); // idom must be updated before handle_use // Now make the original merge point go dead, by handling all its uses. small_cache region_cache; From 94950b6e299b4e0ae30c1bb5ce110fad9288ca74 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Tue, 21 Oct 2025 09:53:36 +0200 Subject: [PATCH 12/16] more for Christian part 3 --- src/hotspot/share/opto/loopnode.hpp | 50 +++++++++++++++++------------ 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 0c3eecb2f9e63..704d54945da91 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -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: install_ctrl_and_idom_forwarding Node_List _loop_or_ctrl; // Pre-computed def-use info @@ -956,11 +959,11 @@ 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_ctrl_node_and_forward_ctrl_and_idom" - // or have otherwise modified their ctrl state by "install_lazy_ctrl_and_idom_forwarding". + // or have otherwise modified their ctrl state by "install_ctrl_and_idom_forwarding". // 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; } @@ -1084,7 +1087,7 @@ class PhaseIdealLoop : public PhaseTransform { // 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: install_lazy_ctrl_and_idom_forwarding + // See: install_ctrl_and_idom_forwarding _loop_or_ctrl.map(i->_idx, (Node*)((intptr_t)n + 1)); 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"); @@ -1108,7 +1111,7 @@ class PhaseIdealLoop : public PhaseTransform { Node* get_ctrl_no_update_helper(const Node* i) const { // We expect only data nodes (which must have a ctrl set), or // dead ctrl nodes that have a ctrl "forwarding". - // See: install_lazy_ctrl_and_idom_forwarding. + // See: install_ctrl_and_idom_forwarding. assert(has_ctrl(i), "only data nodes or ctrl nodes with ctrl forwarding expected"); return (Node*)(((intptr_t)_loop_or_ctrl[i->_idx]) & ~1); } @@ -1120,7 +1123,7 @@ class PhaseIdealLoop : public PhaseTransform { 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 "install_lazy_ctrl_and_idom_forwarding". + // forwarding installed, using "install_ctrl_and_idom_forwarding". // We now have to jump from the old (dead) ctrl node to the new (live) // ctrl node, in possibly multiple ctrl forwarding steps. do { @@ -1151,14 +1154,18 @@ class PhaseIdealLoop : public PhaseTransform { // 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 a ctrl forwarding to the new and live + // 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. - // Using "install_lazy_ctrl_and_idom_forwarding" allows us to only edit + // Note: while the "idom" information is stored in the "_idom" + // side-table, the idom forwarding piggy-packs on the ctrl + // forwarding on "_loop_or_ctrl". + // Using "install_ctrl_and_idom_forwarding" 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". - void install_lazy_ctrl_and_idom_forwarding(Node* old_node, Node* new_node) { + // clean up the forwarding links when we query "get_ctrl" or "idom" + // for these nodes the next time. + void install_ctrl_and_idom_forwarding(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, @@ -1175,7 +1182,7 @@ class PhaseIdealLoop : public PhaseTransform { // - Lazily update the ctrl and idom info of all uses, via a ctrl/idom forwarding. void replace_ctrl_node_and_forward_ctrl_and_idom(Node *old_node, Node *new_node) { _igvn.replace_node(old_node, new_node); - install_lazy_ctrl_and_idom_forwarding(old_node, new_node); + install_ctrl_and_idom_forwarding(old_node, new_node); } private: @@ -1250,16 +1257,19 @@ class PhaseIdealLoop : public PhaseTransform { 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 "install_lazy_ctrl_and_idom_forwarding". + // forwarding installed, using "install_ctrl_and_idom_forwarding". // We now have to jump from the old (dead) idom node to the new (live) - // ctrl/idom node, in possibly multiple ctrl/idom forwarding steps. + // idom node, in possibly multiple idom forwarding steps. + // Note that we piggy-back 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."); } @@ -1271,13 +1281,13 @@ class PhaseIdealLoop : public PhaseTransform { return idom(n->_idx); } - Node* idom(uint didx) const { - Node* n = idom_no_update(didx); + 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 a ctrl forwarding via dead ctrl nodes, this shortens the path. - // See: install_lazy_ctrl_and_idom_forwarding - _idom[didx] = n; + // there was an idom forwarding via dead idom nodes, this shortens the path. + // See: install_ctrl_and_idom_forwarding + _idom[node_idx] = n; return n; } From a91396becfa077cba8d7f748513c366e970e5058 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Tue, 21 Oct 2025 09:53:19 +0200 Subject: [PATCH 13/16] Apply suggestions from code review Co-authored-by: Tobias Hartmann --- src/hotspot/share/opto/loopUnswitch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/loopUnswitch.cpp b/src/hotspot/share/opto/loopUnswitch.cpp index d96bae62a1ac5..43d633fccc484 100644 --- a/src/hotspot/share/opto/loopUnswitch.cpp +++ b/src/hotspot/share/opto/loopUnswitch.cpp @@ -520,7 +520,7 @@ 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 replace_ctrl_node_and_forward_ctrl_and_idom ensures that any get_ctrl that used // to have multiversion_slow_proj as their control are forwarded to the new region node From aaa91d18272d0e5b9afd377fb58739674cedce0b Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Tue, 21 Oct 2025 10:17:23 +0200 Subject: [PATCH 14/16] renaming for Tobias --- .../gc/shenandoah/c2/shenandoahSupport.cpp | 8 ++-- src/hotspot/share/opto/loopPredicate.cpp | 2 +- src/hotspot/share/opto/loopTransform.cpp | 4 +- src/hotspot/share/opto/loopUnswitch.cpp | 8 ++-- src/hotspot/share/opto/loopnode.cpp | 22 +++++------ src/hotspot/share/opto/loopnode.hpp | 39 +++++++++---------- src/hotspot/share/opto/loopopts.cpp | 6 +-- src/hotspot/share/opto/split_if.cpp | 8 ++-- 8 files changed, 48 insertions(+), 49 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp b/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp index 7f1159f60ac33..4d02b045a21fd 100644 --- a/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp +++ b/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp @@ -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->replace_ctrl_node_and_forward_ctrl_and_idom(outer, new_outer); - phase->replace_ctrl_node_and_forward_ctrl_and_idom(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(); } @@ -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->replace_ctrl_node_and_forward_ctrl_and_idom(head, new_head); + phase->replace_node_and_forward_ctrl(head, new_head); } } @@ -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->replace_ctrl_node_and_forward_ctrl_and_idom(u, n); + _phase->replace_node_and_forward_ctrl(u, n); --i; --imax; } } diff --git a/src/hotspot/share/opto/loopPredicate.cpp b/src/hotspot/share/opto/loopPredicate.cpp index 94b24765590ee..61a7ed29c3eb9 100644 --- a/src/hotspot/share/opto/loopPredicate.cpp +++ b/src/hotspot/share/opto/loopPredicate.cpp @@ -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. - replace_ctrl_node_and_forward_ctrl_and_idom(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++) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 101e2738558bf..94c781d82c2af 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -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(); - replace_ctrl_node_and_forward_ctrl_and_idom(outer_out, in); + replace_node_and_forward_ctrl(outer_out, in); _igvn.replace_input_of(outer_sfpt, 0, C->top()); } @@ -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); - replace_ctrl_node_and_forward_ctrl_and_idom(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()); diff --git a/src/hotspot/share/opto/loopUnswitch.cpp b/src/hotspot/share/opto/loopUnswitch.cpp index 43d633fccc484..287f8354dc1b9 100644 --- a/src/hotspot/share/opto/loopUnswitch.cpp +++ b/src/hotspot/share/opto/loopUnswitch.cpp @@ -522,10 +522,10 @@ IfTrueNode* PhaseIdealLoop::create_new_if_for_multiversion(IfTrueNode* multivers // 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 replace_ctrl_node_and_forward_ctrl_and_idom 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_ctrl_node_and_forward_ctrl_and_idom(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; } diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index 08182e17684eb..b6c4d1b85749b 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -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)); - replace_ctrl_node_and_forward_ctrl_and_idom(head, new_inner_head); - replace_ctrl_node_and_forward_ctrl_and_idom(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; } @@ -2382,7 +2382,7 @@ bool PhaseIdealLoop::is_counted_loop(Node* x, IdealLoopTree*&loop, BasicType iv_ return false; } if (is_deleteable_safept(backedge_sfpt)) { - replace_ctrl_node_and_forward_ctrl_and_idom(backedge_sfpt, iftrue); + replace_node_and_forward_ctrl(backedge_sfpt, iftrue); if (loop->_safepts != nullptr) { loop->_safepts->yank(backedge_sfpt); } @@ -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. - replace_ctrl_node_and_forward_ctrl_and_idom(iffalse, iff2); - replace_ctrl_node_and_forward_ctrl_and_idom(iftrue, ift2); + replace_node_and_forward_ctrl(iffalse, iff2); + replace_node_and_forward_ctrl(iftrue, ift2); // Swap names iffalse = iff2; @@ -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"); - replace_ctrl_node_and_forward_ctrl_and_idom(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 && @@ -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'. - replace_ctrl_node_and_forward_ctrl_and_idom(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)) { @@ -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)); } - replace_ctrl_node_and_forward_ctrl_and_idom(sfpt, sfpt->in(TypeFunc::Control)); + replace_node_and_forward_ctrl(sfpt, sfpt->in(TypeFunc::Control)); if (loop->_safepts != nullptr) { loop->_safepts->yank(sfpt); } @@ -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->replace_ctrl_node_and_forward_ctrl_and_idom(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); @@ -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->replace_ctrl_node_and_forward_ctrl_and_idom(n, n->in(TypeFunc::Control)); + phase->replace_node_and_forward_ctrl(n, n->in(TypeFunc::Control)); } } } @@ -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); - replace_ctrl_node_and_forward_ctrl_and_idom(n, in); // Pull safepoint now + replace_node_and_forward_ctrl(n, in); // Pull safepoint now if (ilt->_safepts != nullptr) { ilt->_safepts->yank(n); } diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 704d54945da91..924194c68bf6d 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -892,7 +892,7 @@ class PhaseIdealLoop : public PhaseTransform { // 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: install_ctrl_and_idom_forwarding + // installed. See: forward_ctrl Node_List _loop_or_ctrl; // Pre-computed def-use info @@ -962,8 +962,8 @@ class PhaseIdealLoop : public PhaseTransform { // Returns true if "n" is a data node, false if it's a CFG node. // // Exception: - // control nodes that are dead because of "replace_ctrl_node_and_forward_ctrl_and_idom" - // or have otherwise modified their ctrl state by "install_ctrl_and_idom_forwarding". + // 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; } @@ -1087,7 +1087,7 @@ class PhaseIdealLoop : public PhaseTransform { // 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: install_ctrl_and_idom_forwarding + // See: forward_ctrl _loop_or_ctrl.map(i->_idx, (Node*)((intptr_t)n + 1)); 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"); @@ -1111,7 +1111,7 @@ class PhaseIdealLoop : public PhaseTransform { Node* get_ctrl_no_update_helper(const Node* i) const { // We expect only data nodes (which must have a ctrl set), or // dead ctrl nodes that have a ctrl "forwarding". - // See: install_ctrl_and_idom_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); } @@ -1123,9 +1123,9 @@ class PhaseIdealLoop : public PhaseTransform { 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 "install_ctrl_and_idom_forwarding". - // We now have to jump from the old (dead) ctrl node to the new (live) - // ctrl node, in possibly multiple ctrl forwarding steps. + // 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) == nullptr); @@ -1160,12 +1160,11 @@ class PhaseIdealLoop : public PhaseTransform { // Note: while the "idom" information is stored in the "_idom" // side-table, the idom forwarding piggy-packs on the ctrl // forwarding on "_loop_or_ctrl". - // Using "install_ctrl_and_idom_forwarding" 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 install_ctrl_and_idom_forwarding(Node* old_node, Node* new_node) { + // 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, @@ -1180,9 +1179,9 @@ class PhaseIdealLoop : public PhaseTransform { // 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_ctrl_node_and_forward_ctrl_and_idom(Node *old_node, Node *new_node) { + void replace_node_and_forward_ctrl(Node *old_node, Node *new_node) { _igvn.replace_node(old_node, new_node); - install_ctrl_and_idom_forwarding(old_node, new_node); + forward_ctrl(old_node, new_node); } private: @@ -1264,9 +1263,9 @@ class PhaseIdealLoop : public PhaseTransform { 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 "install_ctrl_and_idom_forwarding". - // We now have to jump from the old (dead) idom node to the new (live) - // idom node, in possibly multiple idom forwarding steps. + // 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 piggy-back 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. @@ -1286,7 +1285,7 @@ class PhaseIdealLoop : public PhaseTransform { // 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: install_ctrl_and_idom_forwarding + // See: forward_ctrl _idom[node_idx] = n; return n; } diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 330eecff30604..71e52f373f7de 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -2742,11 +2742,11 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l } assert(use->is_Proj(), "loop exit should be projection"); - // replace_ctrl_node_and_forward_ctrl_and_idom() 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. - // replace_ctrl_node_and_forward_ctrl_and_idom() takes a dead control as first input. + // 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(); @@ -2757,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); - replace_ctrl_node_and_forward_ctrl_and_idom(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 diff --git a/src/hotspot/share/opto/split_if.cpp b/src/hotspot/share/opto/split_if.cpp index 8e68930dd6b95..425c0c9e99d94 100644 --- a/src/hotspot/share/opto/split_if.cpp +++ b/src/hotspot/share/opto/split_if.cpp @@ -655,7 +655,7 @@ void PhaseIdealLoop::do_split_if(Node* iff, RegionNode** new_false_region, Regio // Replace in the graph with lazy-update mechanism new_iff->set_req(0, new_iff); // hook self so it does not go dead - replace_ctrl_node_and_forward_ctrl_and_idom(ifp, ifpx); + replace_node_and_forward_ctrl(ifp, ifpx); new_iff->set_req(0, region); // Record bits for later xforms @@ -669,10 +669,10 @@ void PhaseIdealLoop::do_split_if(Node* iff, RegionNode** new_false_region, Regio } _igvn.remove_dead_node(new_iff); // Lazy replace IDOM info with the region's dominator - replace_ctrl_node_and_forward_ctrl_and_idom(iff, region_dom); - // Break the self-cycle. Required for install_ctrl_and_idom_forwarding to work on region. + replace_node_and_forward_ctrl(iff, region_dom); + // Break the self-cycle. Required for forward_ctrl to work on region. region->set_req(0, nullptr); - install_ctrl_and_idom_forwarding(region, region_dom); // idom must be updated before handle_use + forward_ctrl(region, region_dom); // idom must be updated before handle_use // Now make the original merge point go dead, by handling all its uses. small_cache region_cache; From ac0573952c8d5b4cfce70b648553f8f19696a25f Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Tue, 21 Oct 2025 14:11:37 +0200 Subject: [PATCH 15/16] Apply suggestions from code review Co-authored-by: Christian Hagedorn --- src/hotspot/share/opto/loopnode.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 924194c68bf6d..aee934b0d118e 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1158,7 +1158,7 @@ class PhaseIdealLoop : public PhaseTransform { // 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 piggy-packs on the ctrl + // 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 @@ -1179,7 +1179,7 @@ class PhaseIdealLoop : public PhaseTransform { // 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) { + void replace_node_and_forward_ctrl(Node* old_node, Node* new_node) { _igvn.replace_node(old_node, new_node); forward_ctrl(old_node, new_node); } @@ -1266,7 +1266,7 @@ class PhaseIdealLoop : public PhaseTransform { // 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 piggy-back on "_loop_or_ctrl" to do the forwarding, + // 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); From 1488616def1b0d9173db7fba0c0e49623ca3b301 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Wed, 22 Oct 2025 08:29:13 +0200 Subject: [PATCH 16/16] fix shenandoah replace for phis --- src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp b/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp index 4d02b045a21fd..35ec9ed5c8cab 100644 --- a/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp +++ b/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp @@ -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->replace_node_and_forward_ctrl(u, n); + _phase->igvn().replace_node(u, n); --i; --imax; } }