RFR: 8318446: C2: implement StoreNode::Ideal_merge_stores

Vladimir Kozlov kvn at openjdk.org
Tue Jan 16 23:04:51 UTC 2024


On Tue, 16 Jan 2024 15:09:27 GMT, Emanuel Peter <epeter at openjdk.org> 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


More information about the hotspot-compiler-dev mailing list