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

[Emanuel Peter]

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

>> kuaiwei has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
>> 
>>  - Merge remote-tracking branch 'origin/master' into dev/merge_loads
>>  - Remove unused code
>>  - Move code to addnode.cpp and add more tests
>>  - Merge remote-tracking branch 'origin/master' into dev/merge_loads
>>  - Fix test
>>  - Add more tests
>>  - Enable StressIGVN and riscv platform
>>  - Change tests as review comments
>>  - Fix test failure and change for review comments
>>  - Revert extract value and add more tests
>>  - ... and 4 more: https://git.openjdk.org/jdk/compare/660b17a6...f6518b26
>
> 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 :)

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

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