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

[Tobias Hartmann]

On Fri, 17 Dec 2021 11:29:56 GMT, Fei Gao <fgao at openjdk.org> wrote:

>> 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
>>   ...
>
> Fei Gao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use uabs() to calculate the absolute value of constant
>   
>   Change-Id: Ie6f37ab159fb7092e1443b9af8d620562a45ae47

Looks good to me otherwise (expect the points about the test that @DamonFool already raised).

src/hotspot/share/opto/subnode.cpp line 1897:

> 1895:     PhaseIterGVN* igvn = phase->is_IterGVN();
> 1896:     if (igvn) {
> 1897:       igvn->_worklist.push(in1);

Why is that needed? Because `in1` could become dead? You should use `set_req_X` above.

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

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