[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