RFR: 8283307: Vectorize unsigned shift right on signed subword types

[Jie Fu]

On Mon, 28 Mar 2022 02:11:55 GMT, Fei Gao <fgao at openjdk.org> wrote:

> public short[] vectorUnsignedShiftRight(short[] shorts) {
>     short[] res = new short[SIZE];
>     for (int i = 0; i < SIZE; i++) {
>         res[i] = (short) (shorts[i] >>> 3);
>     }
>     return res;
> }
> 
> In C2's SLP, vectorization of unsigned shift right on signed subword types (byte/short) like the case above is intentionally disabled[1]. Because the vector unsigned shift on signed subword types behaves differently from the Java spec. It's worthy to vectorize more cases in quite low cost. Also, unsigned shift right on signed subword is not uncommon and we may find similar cases in Lucene benchmark[2].
> 
> Taking unsigned right shift on short type as an example,
> ![image](https://user-images.githubusercontent.com/39403138/160313924-6bded802-c135-48db-98b8-7c5f43d8ff54.png)
> 
> when the shift amount is a constant not greater than the number of sign extended bits, 16 higher bits for short type shown like
> above, the unsigned shift on signed subword types can be transformed into a signed shift and hence becomes vectorizable. Here is the transformation:
> ![image](https://user-images.githubusercontent.com/39403138/160314151-30249bfc-bdfc-4700-b4fb-97617b45184b.png)
> 
> This patch does the transformation in `SuperWord::implemented()` and `SuperWord::output()`. It helps vectorize the short cases above. We can handle unsigned right shift on byte type in a similar way. The generated assembly code for one iteration on aarch64 is like:
> 
> ...
> sbfiz   x13, x10, #1, #32
> add     x15, x11, x13
> ldr     q16, [x15, #16]
> sshr    v16.8h, v16.8h, #3
> add     x13, x17, x13
> str     q16, [x13, #16]
> ...
> 
> 
> Here is the performance data for micro-benchmark before and after this patch on both AArch64 and x64 machines. We can observe about ~80% improvement with this patch.
> 
> The perf data on AArch64:
> Before the patch:
> Benchmark        (SIZE)  (shiftCount)  Mode  Cnt    Score   Error  Units
> urShiftImmByte    1024         3       avgt    5  295.711 ± 0.117  ns/op
> urShiftImmShort   1024         3       avgt    5  284.559 ± 0.148  ns/op
> 
> after the patch:
> Benchmark         (SIZE) (shiftCount)  Mode  Cnt    Score   Error  Units
> urShiftImmByte     1024        3       avgt    5   45.111 ± 0.047  ns/op
> urShiftImmShort    1024        3       avgt    5   55.294 ± 0.072  ns/op
> 
> The perf data on X86:
> Before the patch:
> Benchmark        (SIZE) (shiftCount)  Mode  Cnt    Score    Error  Units
> urShiftImmByte    1024        3       avgt    5  361.374 ±  4.621  ns/op
> urShiftImmShort   1024        3       avgt    5  365.390 ±  3.595  ns/op
> 
> After the patch:
> Benchmark        (SIZE) (shiftCount)  Mode  Cnt    Score    Error  Units
> urShiftImmByte    1024        3       avgt    5  105.489 ±  0.488  ns/op
> urShiftImmShort   1024        3       avgt    5   43.400 ±  0.394  ns/op
> 
> [1] https://github.com/openjdk/jdk/blob/002e3667443d94e2303c875daf72cf1ccbbb0099/src/hotspot/share/opto/vectornode.cpp#L190
> [2] https://github.com/jpountz/decode-128-ints-benchmark/

> I suppose this idea #8224 works and would be still correct even though the SLP analysis fails.
> 
> But we don't have to do the replacement when vectorization fails, right? And, if we want to understand the new implementation in your code, we still need to know the background and the relationship between `rshift` and `urshift` on subword types. The idea in this pr is intended to put off the replacement to the necessary stage. Also, changing the vector opcode is a simple and low-cost way to do the replacement. WDYT?

I agree with you.
After hours of work to implement https://github.com/openjdk/jdk/pull/8224 last night, I think your patch is excellent and smart enough.

test/hotspot/jtreg/compiler/c2/irTests/TestVectorizeURShiftSubword.java line 36:

> 34:  * @key randomness
> 35:  * @summary Auto-vectorization enhancement for unsigned shift right on signed subword types
> 36:  * @requires vm.cpu.features ~= ".*simd.*"

This `requires` would disable the test on some x86 machines.
So we'd better fix it.

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

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