RFR: 8303278: Imprecise bottom type of ExtractB/UB [v2]

[Tobias Hartmann]

On Wed, 29 Mar 2023 01:22:10 GMT, Eric Liu <eliu at openjdk.org> wrote:

>> This is a trivial patch, which fixes the bottom type of ExtractB/UB nodes.
>> 
>> ExtractNode can be generated by Vector API Vector.lane(int), which gets the lane element at the given index. A more precise type of range can help to optimize out unnecessary type conversion in some cases.
>> 
>> Below shows a typical case used ExtractBNode
>> 
>> 
>>   public static byte byteLt16() {
>>    ByteVector vecb = ByteVector.broadcast(ByteVector.SPECIES_128, 1);
>>    return vecb.lane(1);
>>   }
>> 
>> 
>> In this case, c2 constructs IR graph like:
>> 
>>     ExtractB  ConI(24)
>>        |     __|
>>        |    /  |
>>     LShiftI  __|
>>        |    /
>>     RShiftI
>> 
>> which generates AArch64 code:
>> 
>>   movi    v16.16b, #0x1
>>   smov    x11, v16.b[1]
>>   sxtb    w0, w11
>> 
>> with this patch, this shift pair can be optimized out by RShiftI's identity [1]. The code is optimized to:
>> 
>>   movi    v16.16b, #0x1
>>   smov    x0, v16.b[1]
>> 
>> [TEST]
>> 
>> Full jtreg passed except 4 files on x86:
>> 
>> jdk/incubator/vector/Byte128VectorTests.java
>> jdk/incubator/vector/Byte256VectorTests.java
>> jdk/incubator/vector/Byte512VectorTests.java
>> jdk/incubator/vector/Byte64VectorTests.java
>> 
>> They are caused by a known issue on x86 [2].
>> 
>> [1] https://github.com/openjdk/jdk/blob/742bc041eaba1ff9beb7f5b6d896e4f382b030ea/src/hotspot/share/opto/mulnode.cpp#L1052
>> [2] https://bugs.openjdk.org/browse/JDK-8303508
>
> Eric Liu 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 two additional commits since the last revision:
> 
>  - Merge jdk:master
>    
>    Change-Id: I40cce803da09bae31cd74b86bf93607a08219545
>  - 8303278: Imprecise bottom type of ExtractB/UB
>    
>    This is a trivial patch, which fixes the bottom type of ExtractB/UB
>    nodes.
>    
>    ExtractNode can be generated by Vector API Vector.lane(int), which gets
>    the lane element at the given index. A more precise type of range can
>    help to optimize out unnecessary type conversion in some cases.
>    
>    Below shows a typical case used ExtractBNode
>    
>    ```
>      public static byte byteLt16() {
>       ByteVector vecb = ByteVector.broadcast(ByteVector.SPECIES_128, 1);
>       return vecb.lane(1);
>      }
>    
>    ```
>    In this case, c2 constructs IR graph like:
>    
>        ExtractB  ConI(24)
>           |     __|
>           |    /  |
>        LShiftI  __|
>           |    /
>        RShiftI
>    
>    which generates AArch64 code:
>    
>      movi    v16.16b, #0x1
>      smov    x11, v16.b[1]
>      sxtb    w0, w11
>    
>    with this patch, this shift pair can be optimized out by RShiftI's
>    identity [1]. The code is optimized to:
>    
>      movi    v16.16b, #0x1
>      smov    x0, v16.b[1]
>    
>    [TEST]
>    
>    Full jtreg passed except 4 files on x86:
>    
>    jdk/incubator/vector/Byte128VectorTests.java
>    jdk/incubator/vector/Byte256VectorTests.java
>    jdk/incubator/vector/Byte512VectorTests.java
>    jdk/incubator/vector/Byte64VectorTests.java
>    
>    They are caused by a known issue on x86 [2].
>    
>    [1] https://github.com/openjdk/jdk/blob/742bc041eaba1ff9beb7f5b6d896e4f382b030ea/src/hotspot/share/opto/mulnode.cpp#L1052
>    [2] https://bugs.openjdk.org/browse/JDK-8303508
>    
>    Change-Id: Ibea9aeacb41b4d1c5b2621c7a97494429394b599

Looks good. All tests passed.

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13070#pullrequestreview-1368306807