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

[Emanuel Peter]

On Tue, 18 Mar 2025 09:27:20 GMT, kuaiwei <duke at openjdk.org> wrote:

>> src/hotspot/share/opto/memnode.cpp line 1902:
>> 
>>> 1900:   PhaseGVN* const      _phase;
>>> 1901:   LoadNode* const       _load;
>>> 1902:   int          _last_op_index;    // Index of the last item in merged_list
>> 
>> What is the `merged_list`? I could not find it.
>
> It's defined in MergePrimitiveLoads::run . It looks unnecessary since I have _last_op in MergeLoadInfo.

Ah, I think it is a typo, it is `merge_list`.

>> src/hotspot/share/opto/memnode.cpp line 2522:
>> 
>>> 2520:       MergePrimitiveLoads merge(phase, this);
>>> 2521:       Node* merged = merge.run();
>>> 2522:       if (merged != nullptr) { return merged; }
>> 
>> I'm a little confused here... So imagine we have a `LoadB` here. How can we now return a `LoadI` instead, and replace all uses of the `LoadB` with the `LoadI`? Should we not be replacing the `OrI` instead?
>
> I'm not clear about it. I think we need replace OrI node here, but return the origin LoadB?

Can you please elaborate? I'm not understanding what you are saying.

> Now there's limit to merge 2 LoadI as LoadL.

Exactly, and that is fine. But you need a test where you merge two ints ;)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24023#discussion_r2000580122
PR Review Comment: https://git.openjdk.org/jdk/pull/24023#discussion_r2000572888
PR Review Comment: https://git.openjdk.org/jdk/pull/24023#discussion_r2000582314