[vector] RFR (M): Fix ByteVector/ShortVector.shiftR

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Fri Feb 22 06:36:02 UTC 2019


Thanks, Yang.

> It looks good to me.
> I verified the patch in our machines. Both AArch64 and x86 vector api tests are passed.
> But I found a weird thing. I wrote a simple test http://cr.openjdk.java.net/~yzhang/vectorapi.shift/TestShortVector128.java
> When I checked the assembly code in my test case, ShortVector.shiftR() isn't intrinsified with or without the patch. I will continue to check ByteVector/IntVector.shiftR.

Good catch!

It turned out that logical shift is disabled for short & byte since the 
implementation is used for auto-vectorization and it's incorrect to use 
URShiftBV/URShiftSV there.

How about the following?

   http://cr.openjdk.java.net/~vlivanov/panama/vector/shiftr/webrev.01/

(Fixed a but in vsrl32B_avx on x86 along the way.)

Best regards,
Vladimir Ivanov

> -----Original Message-----
> From: panama-dev <panama-dev-bounces at openjdk.java.net> On Behalf Of Vladimir Ivanov
> Sent: Thursday, February 21, 2019 6:44 AM
> To: panama-dev at openjdk.java.net
> Subject: [vector] RFR (M): Fix ByteVector/ShortVector.shiftR
> 
> http://cr.openjdk.java.net/~vlivanov/panama/vector/shiftr/webrev.00/
> 
> ByteVector/ShortVector.shiftR erroneously behaves as signed (and not
> unsigned) shift. (There's aShiftR for the former.)
> 
> While fixing that, I spotted a bug in existing implementation - missing AD instructions for some shapes with constants.
> 
> So, the patch contains the following:
> 
>     * adjusts default implementation to do zero extension first: (a &
> 0xFF) >>> (b & 7) and (a & 0xFFFF) >>> (b & 15)
> 
>     * adjusts C2 to do zero extension;
> 
>     * unifies IR shape for constant and variable shifts making LShiftCntV/RShiftCntV nodes mandatory; then extend AD instructions for constant shifts to take them into account.
> 
> The last point requires changes both on x86 & aarch64 side and I try to cover both.
> 
> Best regards,
> Vladimir Ivanov
> 


More information about the panama-dev mailing list