RFR: 8345485: C2 MergeLoads: merge adjacent array/native memory loads into larger load [v12]

[kuaiwei]

On Thu, 17 Apr 2025 10:12:25 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/addnode.cpp line 950:
>> 
>>> 948:   const Node* check = by_pass_i2l(_combine);
>>> 949:   if (check->outcnt() == 1 && check->unique_out()->Opcode() == _combine->Opcode()) {
>>> 950:     // It's in the middle of combine operators
>> 
>> Ah, I see. This is about checking that there is nothing further down to merge with....
>> 
>> But are you sure this is a good idea to check that there is no `OR` below? I mean there could be valid uses of `OrI` below that do some OR-ing with something else... Now you are forbidding these cases.
>> 
>> Look at this:
>> 
>> int x = (... merge load pattern with OR ...);
>> int y = ... some other value ...
>> int z = x | y;
>> 
>> 
>> I would expect that we could merge the loads here too, but your pattern matching seems to forbid it, right?
>
> Also: I would change the name of the method if we keep it. It is really about checking down, i.e. that there is not other candidate below. Maybe `has_no_merge_load_combine_below`?
> 
> Ah, another example: We could have two merged loads that we OR:
> 
> int x = (... merge load pattern with OR ...);
> int y = (... merge load pattern with OR ...);
> int z = x | y;
> 
> It would be nice if this could be optimized too, and we should have an IR test for it :)

Yes, it's the limit of this implementation. I need to find the last `combine` node which can be replaced with merged load. But if it's used by other `Or` operator. So far I can not find a good way to distinguish these two cases.
I may add a new `checked` flag for combine operator. For case like:

int x = (... merge load pattern with OR ...);
int y = (... merge load pattern with OR ...);
int z = x | y;

When IGVN check the `Or` in `x | y`, it's the last one of combine nodes. But it will fail to merge because `collect_merge_list` can not find a related `Load` for it. And I can mark it as `checked`. So when IGVN check the `Or` nodes in line 1 and line2. it will find the next `Or` is checked and get the right one.

Do you think if it is doable? Other suggestion is appreciated. Thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24023#discussion_r2053662360