RFR: 8320649: C2: Optimize scoped values [v9]

[Emanuel Peter]

On Mon, 26 Feb 2024 15:31:07 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   32 bit build fix
>
> src/hotspot/share/opto/graphKit.cpp line 4269:
> 
>> 4267: }
>> 4268: 
>> 4269: Node* GraphKit::scopedValueCache_helper() {
> 
> Suggestion:
> 
> Node* GraphKit::scopedValueCache_handle() {
> 
> That would be less generic, and it seems that is what you call it on the call-site, i.e. the variable.

Ah. I see that you just moved it. But I think it is still a better name, and this would be a good time to improve it.

> src/hotspot/share/opto/loopTransform.cpp line 487:
> 
>> 485: 
>> 486:   while (test != _head) {   // Scan till run off top of loop
>> 487:     if (test->is_If() && !scoped_value_only) {    // Test?
> 
> What is happening here? Would a comment be helpful?

I think the issue is with the variable name that is not descriptive enough.

> src/hotspot/share/opto/loopTransform.cpp line 503:
> 
>> 501:         return estimate;    // Found reason to peel!
>> 502:       }
>> 503:     } else if (test->Opcode() == Op_ScopedValueGetResult && !is_member(phase->get_loop(phase->get_ctrl((test->as_ScopedValueGetResult())->scoped_value())))) {
> 
> This line is a bit difficult to read 😅

Is there not any utility method for checking if a node has a ctrl that is in a loop which is inside "this" loop?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1502824502
PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1502905861
PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1502887303