RFR: 8347405: MergeStores with reverse bytes order value [v2]

[kuaiwei]

On Fri, 10 Jan 2025 11:08:53 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> kuaiwei has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix as review comments
>
> src/hotspot/share/opto/memnode.cpp line 2794:
> 
>> 2792:   StoreNode* const _store;
>> 2793:   enum DataOrder { Unknown, Forward, Reverse};
>> 2794:   DataOrder  _value_order;
> 
> Suggestion:
> 
>   // State machine with initial state Unknown
>   // Allowed transitions:
>   //   Unknown  -> Forward
>   //   Unknown  -> Backward
>   //   Forward  -> Forward
>   //   Backward -> Backward
>   enum DataOrder { Unknown, Forward, Reverse };
>   DataOrder  _value_order;
> 
> 
> Also: call it either consistently data-order or value-order ;)

Thanks for comment. I rename the enum to ValueOrder, and rename Reverse to Backward.

> src/hotspot/share/opto/memnode.cpp line 2993:
> 
>> 2991:   }
>> 2992: 
>> 2993:   // initialize value_order once
> 
> Suggestion:
> 
>   // Initial state "Unknown": check for transition to Forward or Reverse.

Fixed

> src/hotspot/share/opto/memnode.cpp line 2996:
> 
>> 2994:   if (_value_order == DataOrder::Unknown) {
>> 2995:     if (shift_n1 < shift_n2) {
>> 2996:       _value_order = DataOrder::Forward;
> 
> Suggestion:
> 
>       _value_order = DataOrder::Forward; // First pair has Forward order.

Fixed

> src/hotspot/share/opto/memnode.cpp line 3010:
> 
>> 3008:   if ((_value_order == DataOrder::Forward && shift_n1 > shift_n2) ||
>> 3009:       (_value_order == DataOrder::Reverse && shift_n1 < shift_n2)) {
>> 3010:     // wrong order
> 
> Suggestion:
> 
>     // Wrong order: mixed Forward and Reverse not allowed.

Fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23030#discussion_r1910374211
PR Review Comment: https://git.openjdk.org/jdk/pull/23030#discussion_r1910374912
PR Review Comment: https://git.openjdk.org/jdk/pull/23030#discussion_r1910375233
PR Review Comment: https://git.openjdk.org/jdk/pull/23030#discussion_r1910374680