RFR: 8333334: C2: Make result of `Node::dominates` more precise to enhance scalar replacement [v5]

[Qizheng Xing]

On Fri, 5 Jul 2024 08:17:54 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Qizheng Xing has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add `@requires` for G1 GC.
>
> src/hotspot/share/opto/memnode.cpp line 510:
> 
>> 508:           only_dominating_controls = true;
>> 509:         } else {
>> 510:           if (dead_code) dead_code_flag = true;
> 
> A few comments about this line:
> - You need curly braces for the body
> - We do not do implicit null-checks: you should always do `dead_code != null`. But why are you even checking this here? In all other cases you would just set the `dead_code_flag`, and if `dead_code` then this would go to the `dummy_flag`, right?

This line is even sillier than I thought once I understood your comment, please allow me to explain:

In this scope, `dead_code` is not the pointer argument of the method, it's just a local variable with the same name. I declared it here to accept the return value of `Node::dominates`, and if it's true, we should set the pointer argument to true to indicate that we have encountered a dead control flow.

This is a bad practice as the name of the variable can cause confusion. Sorry for writing this🤦, I've removed the relevant code in the latest commit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19496#discussion_r1668110416