RFR: 8254872: Optimize Rotate on AArch64

[Eric Liu]

On Fri, 13 Nov 2020 17:54:34 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> This patch is a supplement for
>> https://bugs.openjdk.java.net/browse/JDK-8248830.
>> 
>> With the implementation of rotate node in IR, this patch:
>> 
>> 1. canonicalizes RotateLeft into RotateRight when shift is a constant,
>>    so that GVN could identify the pre-existing node better.
>> 2. implements scalar rotate match rules and removes the original
>>    combinations of Or and Shifts on AArch64.
>> 
>> This patch doesn't implement vector rotate due to the lack of
>> corresponding vector instructions on AArch64.
>> 
>> Test case below is an explanation for this patch.
>> 
>>         public static int test(int i) {
>>             int a =  (i >>> 29) | (i << -29);
>>             int b = i << 3;
>>             int c = i >>> -3;
>>             int d = b | c;
>>             return a ^ d;
>>         }
>> 
>> Before:
>> 
>>         lsl     w12, w1, #3
>>         lsr     w10, w1, #29
>>         add     w11, w10, w12
>>         orr     w12, w12, w10
>>         eor     w0, w11, w12
>> 
>> After:
>> 
>>         ror     w10, w1, #29
>>         eor     w0, w10, w10
>> 
>> Tested jtreg TestRotate.java, hotspot::hotspot_all_no_apps,
>> jdk::jdk_core, langtools::tier1.
>
> Looks very nice in general, but please add one more case for the EOR (shifted register) instructions with rotation. At present we only do LSR, ASL, and ASR. You could add ROR to
> 
> define(`ALL_SHIFT_KINDS',
> `BOTH_SHIFT_INSNS($1, $2, URShift, LSR)
> BOTH_SHIFT_INSNS($1, $2, RShift, ASR)
> BOTH_SHIFT_INSNS($1, $2, LShift, LSL)')dnl
> 
> This is used in, for example, Java code for SHA 3.

Hi Andrew,

I just thought about this a bit more.

> Looks very nice in general, but please add one more case for the EOR (shifted register) instructions with rotation. At present we only do LSR, ASL, and ASR. You could add ROR to
> 
> define(`ALL_SHIFT_KINDS', `BOTH_SHIFT_INSNS($1, $2, URShift, LSR)
> BOTH_SHIFT_INSNS($1, $2, RShift, ASR)
> BOTH_SHIFT_INSNS($1, $2, LShift, LSL)')dnl
> 
> This is used in, for example, Java code for SHA 3.

I prefer to integrate this patch first.
-  I added those cases for EOR instructions in my local, they work fine in general but I suppose it still needs more strict regressions. 
-  For another rule `dst (AddI (LShiftI src1 lshift) (URShiftI src2 rshift))`, I presume it can be transformed to Rotate in middle-end if `lshift + rshift = 0` but I didn't implement it in this patch.
- @vnkozlov (Vladimir Kozlov) left over an issue(https://bugs.openjdk.java.net/browse/JDK-8252776), which asks for refactoring the test cases in TestRotate.java, It's also a good chance to add other new test cases.

This patch is basically pure without regressions, and for above tasks I prefer to finish them in the next patch once for all.

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

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