On Tue, 16 Jan 2024 15:09:27 GMT, Emanuel Peter <[email protected]> wrote:
>> @eme64 I have tried your patch, it seems that there are some limitations:
>>
>> - The stores are not merged if the order is not right (e.g `a[2] = 2; a[1] =
>> 1;`)
>> - The stores are not merged if they are floating point constants.
>> - The stores are not merged if they are consecutive fields in an object. E.g:
>>
>>
>> class Point {
>> int x; int y;
>> }
>>
>> p.x = 1;
>> p.y = 2; // Cannot merge into mov [p.x], 0x200000001
>>
>>
>> Regarding the final point, fields may be of different types with different
>> sizes and there may be padding between them. This means that for load-store
>> sequence merges, I think SLP cannot handle these cases.
>>
>> Thanks.
>
> @merykitty @cl4es @RogerRiggs @vnkozlov I wonder if you think that the
> approach of this PR is good, and if you have any suggestions about it?
>
> - Is a separate phase ok?
> - Is this PR in a sweet-spot that reaches the goals of the library-folks, but
> is not too complex?
> - Would you prefer a more general solution, like a straight-line SLP
> algorithm, that can merge (even vectorize) any load / store sequences, even
> merge accesses with different element sizes and with gaps/padding?
@eme64 I would suggest to change the subject of RFE and this PR to something
like:
"C2: optimize stores into primitive arrays by combining values into larger
store"
It will correctly describes the scope of changes. In a future we may have
separate RFE for object fields - I don't think we should do it in this RFE.
For performance result it would be nice to have only one table with additional
column with % difference. It is hard to see now the difference.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16245#issuecomment-1894659376
