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

[Emanuel Peter]

On Mon, 9 Jun 2025 07:15:46 GMT, Kuai Wei <kwei at openjdk.org> wrote:

>> @kuaiwei Thanks for your reply!
>> 
>>> I think it's more easy to mark the combine operator checked.
>> 
>> It may seem easier now. But over time, if multiple operations had such flags, things would become very messy. And now every node that can be such a `combine operator` has to have an additional flag, and consumes more memory.
>> 
>>> I tried to use match pattern for MergePrimitiveLoads::has_no_merge_load_combine_below() . But I think it has some difficulty. For mergeable operators, they can be linked in different way, like:
>>> (((item1 Or item2) Or item3) Or item4)
>>> ((item1 Or item2) Or (item3 Or item4))
>>> ...
>> 
>> Yes, we may have to deal with inputs being permuted. But I think we should be able to deal with the permutations, we do that in other places too.
>> 
>>> To check the next Or operator is a valid last one of combine operator chain. We may check its all input recursively. I didn't find a good way to revolve it. If you have better idea, I will check it.
>> 
>> I'm not sure I understood what you said here.
>> 
>>> We may check its all input recursively
>> 
>> You probably mean we could check all outputs?
>> 
>> So if you are looking at the `OrINode`, and the pattern above it is already a `MergeLoad` pattern, then we should also look down, and see if we find other `OrINode`. For each of these output nodes, we should check if their other input could also be merged with what we already have. Do you not think this is possible? What exactly makes it difficult or impossible?
>
> @eme64 From your description, I may change like below. Could you check if I understand correct? Thanks.
> When IGVN check the input combine operator, called it as `_checked`. We can go down the combine operators chain to find the _last one.
> 
> for op from _last to _checked:  // _checked is not include
>   collect merge_info_list by op
>   if it can be merged and _checked is in the list:
>       return     // it will be merged when IGVN optimize this op
>   if can not merge or _checked is not in list:
>       continue;
> // all successors of combine operators are checked, we can start to merge with _checked
> ...
> 
> I think it can work but there are some redundant `collect and check` work. And we can add a cache in IGVN to reduce it. Do you have other suggestion about it ?

@kuaiwei I'm struggling to follow your pseudocode, can you please expand a little or try to describe in other words?

It is good to reduce redundant work, but it has to be worth the complexity. But I'm skeptical of caching things between IGVN optimizations, it is just not something we do, as far as I know. Caching could also be tricky when the cached things are not accurate any more. What exactly would be your approach with caching?

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

PR Comment: https://git.openjdk.org/jdk/pull/24023#issuecomment-2962451732