[vector] RFR (M): Fix ByteVector/ShortVector.shiftR
Yang Zhang (Arm Technology China)
Yang.Zhang at arm.com
Thu Feb 21 11:01:41 UTC 2019
Hi Vladimir
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.
Regards,
Yang
-----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