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

[Emanuel Peter]

On Thu, 8 Feb 2024 11:54:20 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/callGenerator.cpp line 935:
>> 
>>> 933:   // an uncommon trap, it could reach either the first or the second cache probe if first. Figure out which is the first
>>> 934:   // here.
>>> 935:   void find_first_probe_if(Unique_Node_List &wq) {
>> 
>> Suggestion:
>> 
>>   void find_first_probe_if(const Unique_Node_List &wq) {
>> 
>> I don't think you are modifying `wq`?
>> What are the assumptions about `wq`? A better name would help here.
>
> This method does more than "find". Adjust the name to explain the side-effect.
> Because technically, we already found it, and actually need the reference to the first probe if.
> But we adjust the order of the first and second probe if.
> `adjust_order_or_first_and_second_probe_if`?

Do you want to assert that at least `_first_index_in_cache != nullptr` ?

>> src/hotspot/share/opto/callGenerator.cpp line 1224:
>> 
>>> 1222:   }
>>> 1223: 
>>> 1224:   void remove_thread_scoped_cache_call(const CallProjections& scoped_value_cache_projs) const {
>> 
>> What is the "thread" for?
>
> And really you are only removing it on some paths: io and merged_memory. What about everything else?

I think you are doing it elsewhere at the call-site. But the name is not quite accurate.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1482849577
PR Review Comment: https://git.openjdk.org/jdk/pull/16966#discussion_r1482976289