RFR: 8370220: C2: rename methods and improve documentation around get_ctrl and idom lazy updating/forwarding of ctrl and idom via dead ctrl nodes [v7]

Christian Hagedorn chagedorn at openjdk.org
Tue Oct 21 09:45:17 UTC 2025


On Tue, 21 Oct 2025 08:20:26 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> When working on https://github.com/openjdk/jdk/pull/27889, I was irritated by the lack of documentation and suboptimal naming.
>> 
>> Here, I'm doing the following:
>> - Add more documentation, and improve it in other cases.
>> - Rename "lazy" methods: "lazy" could indicate that we delay it somehow until later, but it is unclear what is delayed.
>>   - `lazy_replace` -> `replace_ctrl_node_and_forward_ctrl_and_idom`
>>   - `lazy_update` -> `install_lazy_ctrl_and_idom_forwarding`
>> - Made some methods private, and added some additional asserts.
>> 
>> I'd be more than happy for even better names, and suggestions how to improve the documentation further :)
>> 
>> Related issues:
>> https://github.com/openjdk/jdk/pull/27889
>> https://github.com/openjdk/jdk/pull/15720
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   renaming for Tobias

Good naming update! The update looks good to me apart from some last nits.

src/hotspot/share/opto/loopnode.hpp line 1161:

> 1159:   //   future.
> 1160:   //   Note: while the "idom" information is stored in the "_idom"
> 1161:   //   side-table, the idom forwarding piggy-packs on the ctrl

Suggestion:

  //   side-table, the idom forwarding piggybacks on the ctrl

src/hotspot/share/opto/loopnode.hpp line 1182:

> 1180:   // - Update the node inputs of all uses.
> 1181:   // - Lazily update the ctrl and idom info of all uses, via a ctrl/idom forwarding.
> 1182:   void replace_node_and_forward_ctrl(Node *old_node, Node *new_node) {

Suggestion:

  void replace_node_and_forward_ctrl(Node* old_node, Node* new_node) {

src/hotspot/share/opto/loopnode.hpp line 1269:

> 1267:       // the old (dead) idom node to the new (live) idom node, in possibly
> 1268:       // multiple idom forwarding steps.
> 1269:       // Note that we piggy-back on "_loop_or_ctrl" to do the forwarding,

Suggestion:

      // Note that we piggyback on "_loop_or_ctrl" to do the forwarding,

-------------

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27892#pullrequestreview-3359723589
PR Review Comment: https://git.openjdk.org/jdk/pull/27892#discussion_r2447507606
PR Review Comment: https://git.openjdk.org/jdk/pull/27892#discussion_r2447512959
PR Review Comment: https://git.openjdk.org/jdk/pull/27892#discussion_r2447509451


More information about the shenandoah-dev mailing list