RFR: 8276673: Optimize abs operations in C2 compiler [v5]

[Fei Gao]

On Thu, 16 Dec 2021 10:01:41 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Fei Gao has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:
>> 
>>  - Add a jmh benchmark case
>>    
>>    Change-Id: I64938d543126c2e3f9fad8ffc4a50e25e4473d8f
>>  - Merge branch 'master' of github.com:fg1417/jdk into fg8276673
>>    
>>    Change-Id: I71987594e9288a489a04de696e69a62f4ad19357
>>  - Merge branch 'master' into fg8276673
>>    
>>    Change-Id: I5e3898054b75f49653b8c3b37e4f5007675fa963
>>  - 8276673: Optimize abs operations in C2 compiler
>>    
>>    The patch aims to help optimize Math.abs() mainly from these three parts:
>>    1) Remove redundant instructions for abs with constant values
>>    2) Remove redundant instructions for abs with char type
>>    3) Convert some common abs operations to ideal forms
>>    
>>    1. Remove redundant instructions for abs with constant values
>>    
>>    If we can decide the value of the input node for function Math.abs()
>>    at compile-time, we can substitute the Abs node with the absolute
>>    value of the constant and don't have to calculate it at runtime.
>>    
>>    For example,
>>      int[] a
>>      for (int i = 0; i < SIZE; i++) {
>>        a[i] = Math.abs(-38);
>>      }
>>    
>>    Before the patch, the generated code for the testcase above is:
>>    ...
>>      mov   w10, #0xffffffda
>>      cmp   w10, wzr
>>      cneg  w17, w10, lt
>>      dup   v16.8h, w17
>>    ...
>>    After the patch, the generated code for the testcase above is :
>>    ...
>>      movi  v16.4s, #0x26
>>    ...
>>    
>>    2. Remove redundant instructions for abs with char type
>>    
>>    In Java semantics, as the char type is always non-negative, we
>>    could actually remove the absI node in the C2 middle end.
>>    
>>    As for vectorization part, in current SLP, the vectorization of
>>    Math.abs() with char type is intentionally disabled after
>>    JDK-8261022 because it generates incorrect result before. After
>>    removing the AbsI node in the middle end, Math.abs(char) can be
>>    vectorized naturally.
>>    
>>    For example,
>>    
>>      char[] a;
>>      char[] b;
>>      for (int i = 0; i < SIZE; i++) {
>>        b[i] = (char) Math.abs(a[i]);
>>      }
>>    
>>    Before the patch, the generated assembly code for the testcase
>>    above is:
>>    
>>    B15:
>>      add   x13, x21, w20, sxtw #1
>>      ldrh  w11, [x13, #16]
>>      cmp   w11, wzr
>>      cneg  w10, w11, lt
>>      strh  w10, [x13, #16]
>>      ldrh  w10, [x13, #18]
>>      cmp   w10, wzr
>>      cneg  w10, w10, lt
>>      strh  w10, [x13, #18]
>>      ...
>>      add   w20, w20, #0x1
>>      cmp   w20, w17
>>      b.lt  B15
>>    
>>    After the patch, the generated assembly code is:
>>    B15:
>>      sbfiz x18, x19, #1, #32
>>      add   x0, x14, x18
>>      ldr   q16, [x0, #16]
>>      add   x18, x21, x18
>>      str   q16, [x18, #16]
>>      ldr   q16, [x0, #32]
>>      str   q16, [x18, #32]
>>      ...
>>      add   w19, w19, #0x40
>>      cmp   w19, w17
>>      b.lt  B15
>>    
>>    3. Convert some common abs operations to ideal forms
>>    
>>    The patch overrides some virtual support functions for AbsNode
>>    so that optimization of gvn can work on it. Here are the optimizable
>>    forms:
>>    
>>    a) abs(0 - x) => abs(x)
>>    
>>    Before the patch:
>>      ...
>>      ldr   w13, [x13, #16]
>>      neg   w13, w13
>>      cmp   w13, wzr
>>      cneg  w14, w13, lt
>>      ...
>>    After the patch:
>>      ...
>>      ldr   w13, [x13, #16]
>>      cmp   w13, wzr
>>      cneg  w13, w13, lt
>>      ...
>>    
>>    b) abs(abs(x))  => abs(x)
>>    
>>    Before the patch:
>>      ...
>>      ldr   w12, [x12, #16]
>>      cmp   w12, wzr
>>      cneg  w12, w12, lt
>>      cmp   w12, wzr
>>      cneg  w12, w12, lt
>>      ...
>>    After the patch:
>>      ...
>>      ldr   w13, [x13, #16]
>>      cmp   w13, wzr
>>      cneg  w13, w13, lt
>>      ...
>>    
>>    Change-Id: I5434c01a225796caaf07ffbb19983f4fe2e206bd
>
> src/hotspot/share/opto/subnode.cpp line 1854:
> 
>> 1852:       // Special case for min_jint: Math.abs(min_jint) = min_jint.
>> 1853:       // Do not use C++ abs() for min_jint to avoid undefined behavior.
>> 1854:       return (ti->is_con(min_jint)) ? TypeInt::MIN : TypeInt::make(abs(ti->get_con()));
> 
> Suggestion:
> 
>       return TypeInt::make(uabs(ti->get_con());
> 
> We have uabs() for julong and unsigned int.

Thanks for your review. Fixed.

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

PR: https://git.openjdk.java.net/jdk/pull/6755