[vectorIntrinsics] [PATCH] Elemental shifts and rotates speedup

[Kharbas, Kishor]

Hi Jatin,
As John pointed, there is discrepancy in Javadoc and the definition of shiftLanesLeft, shiftLanesRight.
Otherwise the patch looks good.

Thanks,
Kishor

> -----Original Message-----
> From: panama-dev [mailto:panama-dev-bounces at openjdk.java.net] On
> Behalf Of Bhateja, Jatin
> Sent: Friday, May 31, 2019 3:27 AM
> To: john.r.rose at oracle.com
> Cc: panama-dev at openjdk.java.net
> Subject: Re: [vectorIntrinsics] [PATCH] Elemental shifts and rotates speedup
> 
> 
> Hi John,
> 
> Thanks for your comments.
> 
> Please find my response embedded below.
> 
> Regards,
> Jatin
> 
> -------- Original message --------
> From: John Rose <john.r.rose at oracle.com>
> Date: 30/05/2019 22:12 (GMT+05:30)
> To: "Bhateja, Jatin" <jatin.bhateja at intel.com>
> Cc: panama-dev at openjdk.java.net
> Subject: Re: [vectorIntrinsics] [PATCH] Elemental shifts and rotates speedup
> 
> On May 30, 2019, at 9:35 AM, John Rose <john.r.rose at oracle.com> wrote:
> >
> > I looked at the Java changes.  They look good.
> 
> JATIN >> Thanks
> 
> One more comment at the API level.  I have been thinking about  the Java
> rules for <<n and >>n, versus our vector-level shifting operations, and I think
> the Java rules are not for the best here. So in Java x<<n is reduced modulo
> bitsize(x) (by and-ing with n-1).  But this doesn't seem to make sense (to me)
> when working with lanes.
> If you are shifting lanes by a large number, I think you should just shift in
> zeroes or (even better) throw an exception.
> 
> Reason:  Lane bundles *sometimes* wrap modulo the lane-count but
> *usually* do not, because a bundle of lanes always represents an oriented
> segment in a much larger data workload.  In this setting, an overflowing lane
> shift n in shiftLanesRight(n) is probably unintentional and will reorder the
> workload unpredictably.
> The user can say v.shiftLanesRight(n%LC) if the n parameter really needs to
> be wrapped.
> 
> Comments?
> 
> JATIN>> I agree, shift count at lane level is not modulo wrapped, for count
> greater than lane count and -ve shift count special handling has already been
> done in API definition as you mentioned.
> 
> — John