RFR: 8369646: Detection of redundant conversion patterns in add_users_of_use_to_worklist is too restrictive [v5]

[Benoît Maillard]

On Fri, 7 Nov 2025 07:48:34 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Benoît Maillard has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add comments to clarify how add_users_to_worklist and add_users_of_use_to_worklist
>
> src/hotspot/share/opto/phaseX.hpp line 536:
> 
>> 534:   // optimizations have dependencies that extend beyond a node's direct
>> 535:   // inputs, so it is necessary to ensure the appropriate notifications
>> 536:   // are made here.
> 
> Maybe also add what 'n' is. Could it be named 'parent'?

I would keep the name `n` for consistency, `parent` sounds a bit confusing from the point of view of the caller imho (as it is the node we are modifying in the first place). But I described more explicitly what `n` is.

> src/hotspot/share/opto/phaseX.hpp line 542:
> 
>> 540:   // affected by changes to 'n', to the worklist.
>> 541:   // This function may be called with a node that is about to be
>> 542:   // replaced as argument 'n'. In this case, 'n' should not be considered
> 
> By another node?
> Suggestion:
> 
>   // replaced by another node. In this case, 'n' should not be considered

That's not what I meant, but yes it was confusing. I have changed the comment a bit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27900#discussion_r2504404220
PR Review Comment: https://git.openjdk.org/jdk/pull/27900#discussion_r2504408495