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

[Emanuel Peter]

On Tue, 20 Feb 2024 20:49:01 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/callGenerator.cpp line 956:
> 
>> 954:     // an uncommon trap, it could reach either the first or the second cache probe if first. Figure out which is the first
>> 955:     // here.
>> 956:     void adjust_order_or_first_and_second_probe_if(const Unique_Node_List &scoped_value_get_subgraph) {
> 
> Suggestion:
> 
>     void adjust_order_of_first_and_second_probe_if(const Unique_Node_List& scoped_value_get_subgraph) {
> 
> typo

Typo is still here: `or` vs `of`.
Or is this intentional?

> src/hotspot/share/opto/callGenerator.cpp line 961:
> 
>> 959:       }
>> 960:       assert(_first_cache_probe_iff != nullptr, "can't have a second iff if there's no first one");
>> 961:       Node_Stack stack(0);
> 
> Suggestion:
> 
>       ResourceMark rm;
>       Node_Stack stack(0);
> 
> Would that work?

Plus: it still makes me a bit nervous that there is not visited set... But I guess if we needed one, then we would have already walked outside of the relevant subgraph, and things would be broken... hmm

> src/hotspot/share/opto/callGenerator.cpp line 1091:
> 
>> 1089:     {
>> 1090:       pattern_match();
>> 1091:     }
> 
> Suggestion:
> 
>     }
>     NONCOPYABLE(ScopedValueGetPatternMatcher);
> 
> Must for good measure ;)

Would some other classes be ok with adding `NONCOPYABLE` as well?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1502675906
PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1502681953
PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1502727891