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

Emanuel Peter epeter at openjdk.org
Tue Oct 21 08:11:10 UTC 2025


On Tue, 21 Oct 2025 07:51:04 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Apply suggestions from code review
>>   
>>   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>
> src/hotspot/share/opto/loopnode.hpp line 1176:
> 
>> 1174:   // - Update the node inputs of all uses.
>> 1175:   // - Lazily update the ctrl and idom info of all uses, via a ctrl/idom forwarding.
>> 1176:   void replace_ctrl_node_and_forward_ctrl_and_idom(Node *old_node, Node *new_node) {
> 
> Drive-by comment (sorry): I think this is way too heavy. Descriptive names are good but for complex methods, not all the semantics can be put into the name but should rather be in a comment.
> 
> Couldn't we just name these `replace_and_update_ctrl` and `update_ctrl`? I think this entails updating idom which is dependent on ctrl. And the comments explain the details well enough.

@TobiHartmann 

`update_ctrl`: Sounds too much like `set_ctrl`, but it is not at all similar conceptually.

What about:
- `install_ctrl_and_idom_forwarding` -> `forward_ctrl`
- `replace_ctrl_node_and_forward_ctrl_and_idom` -> `replace_node_and_forward_ctrl`
  - Because it is really a specialization of `_igvn.replace_node`, so it should have some name similarity.

What do you think?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27892#discussion_r2447148523


More information about the shenandoah-dev mailing list