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]

Tobias Hartmann thartmann at openjdk.org
Tue Oct 21 08:11:11 UTC 2025


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

>> 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?

Yes, that's much better, good with me!

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

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


More information about the shenandoah-dev mailing list