RFR: 8321308: AArch64: Fix matching predication for cbz/cbnz [v2]

[Andrew Dinn]

On Thu, 6 Jun 2024 14:35:29 GMT, Fei Gao <fgao at openjdk.org> wrote:

>> For array length check like:
>> 
>>   if (a.length > 0) {
>>     [Block 1]
>>   } else {
>>     [Block 2]
>>   }
>> 
>> 
>> Since `a.length` is unsigned, it's semantically equivalent to:
>> 
>>   if (a.length != 0) {
>>     [Block 1]
>>   } else {
>>     [Block 2]
>>   }
>> 
>> 
>> On aarch64 port, we can do the conversion like above, during c2 compiler instruction matching, for certain unsigned integral comparisons.
>> 
>> For example,
>> 
>> cmpw  w11, #0 # unsigned
>> bls   label   # unsigned
>> [Block 1]
>> 
>> label:
>> [Block 2]
>> 
>> 
>> can be converted to:
>> 
>> cbz  w11, label
>> [Block 1]
>> 
>> label:
>> [Block 2]
>> 
>> 
>> Currently, we have some matching rules to do the conversion [[1]](https://github.com/openjdk/jdk/blob/4f1a10f84bcfadef263a0890b6834ccd3d5bb52f/src/hotspot/cpu/aarch64/aarch64.ad#L15688). But the predicate here [[2]](https://github.com/openjdk/jdk/blob/4f1a10f84bcfadef263a0890b6834ccd3d5bb52f/src/hotspot/cpu/aarch64/aarch64.ad#L5631) matches wrong `BoolTest` masks, so these rules fail to convert. I guess it's a typo introduced in [JDK-8160006](https://bugs.openjdk.org/browse/JDK-8160006). The patch fixes it.
>
> Fei Gao has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Redefine the interface for cmpOpUEqNeLeGt
>  - Merge branch 'master' into fg8321308
>  - 8321308: AArch64: Fix matching predication for cbz/cbnz
>    
>    For array length check like:
>    ```
>      if (a.length > 0) {
>        [Block 1]
>      } else {
>        [Block 2]
>      }
>    ```
>    
>    Since `a.length` is unsigned, it's semantically equivalent to:
>    ```
>      if (a.length != 0) {
>        [Block 1]
>      } else {
>        [Block 2]
>      }
>    ```
>    
>    On aarch64 port, we can do the conversion like above, during c2
>    compiler instruction matching, for certain unsigned integral
>    comparisons.
>    
>    For example,
>    ```
>    cmpw  w11, #0 # unsigned
>    bls   label   # unsigned
>    [Block 1]
>    
>    label:
>    [Block 2]
>    ```
>    
>    can be converted to:
>    ```
>    cbz  w11, label
>    [Block 1]
>    
>    label:
>    [Block 2]
>    ```
>    
>    Currently, we have some matching rules to do the conversion[1].
>    But the predicate here[2] matches wrong `BoolTest` masks,
>    so these rules fail to convert. I guess it's a typo introduced
>    in JDK-8160006. The patch fixes it.
>    
>    [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/aarch64.ad#L16179
>    [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/aarch64.ad#L6140

Thanks, Fei. Looks good.

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

Marked as reviewed by adinn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16989#pullrequestreview-2107413052