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

[Emanuel Peter]

On Fri, 2 May 2025 10:18:23 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> kuaiwei has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix build error on mac and windows
>
> src/hotspot/share/opto/addnode.cpp line 981:
> 
>> 979:   // Check the Load node has the pattern "(Or (LShift (Load .. ) ConI) ..)" or "(Or (Load ..) ..)"
>> 980:   for (DUIterator_Fast imax, iter = check->fast_outs(imax); iter < imax; iter++) {
>> 981:     Node *out = check->fast_out(iter);
> 
> Do you need to loop here?
> Do all cases we expect to optimize only have a single use of `check`?
> Or what exactly would happen here if we had multiple uses?
> 
> Could be good if you had a regression test that triggers such a case with multiple uses here.

Yeah, it seems you actually check when looping if you find a second one, and then return `invalid`. And the default case also gets `invalid`. Hence, I really see no point in looping here, you should just use `unique_out`, right?

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

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