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

[Jie Fu]

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

test/hotspot/jtreg/compiler/c2/TestAbs.java line 39:

> 37: 
> 38: public class TestAbs {
> 39:     private static int SIZE = 500;

Not used?

test/hotspot/jtreg/compiler/c2/TestAbs.java line 114:

> 112: 
> 113:         // Test abs(constant) optimization for float
> 114:         Asserts.assertEquals(Float.NaN, Math.abs(Float.NaN));

I would suggest something like:

assertTrue(Float.isNaN(Math.abs(Float.NaN)))

test/hotspot/jtreg/compiler/c2/TestAbs.java line 136:

> 134:     }
> 135: 
> 136:     private static void testAbsTransformInt(int[] a) {

If you want to verify C2's transformation, probably we should use C2's IR test framework.

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

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