[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