RFR: 8351627: C2 AArch64 ROR/ROL: assert((1 << ((T>>1)+3)) > shift) failed: Invalid Shift value [v2]

Tue Mar 25 07:32:14 UTC 2025

On Tue, 25 Mar 2025 07:22:47 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

> I don't have access to Grace :-) but I have verified your fix over Graviton3.
> 
> LGTM.

Thanks for your testing and review!

> FTR, Java-side implementation ensures rounding off the shift count https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/LongVector.java#L969

Thanks for your review @jatin-bhateja ! Yes, the Java-side would make the masking before it goes to hotspot. This issue happens because the initial shift count in java-size is `0`, and `32` is generated by `shift_mask + 1 - 0`.  So an additional mask is needed to the new generated shift count.

> This is the edge case which does not impact x86 as instruction semantics are clear about setting the register to zero if [shift count is greater than respective lane count](https://www.felixcloutier.com/x86/psllw:pslld:psllq#:~:text=If%20the%20value%20specified%20by%20the%20count%20operand%20is%20greater%20than%2015%20(for%20words)%2C%2031%20(for%20doublewords)%2C%20or%2063%20(for%20a%20quadword)%2C%20then%20the%20destination%20operand%20is%20set%20to%20all%200s.%20Figure%204%2D17%20gives%20an%20example%20of%20shifting%20words%20in%20a%2064%2Dbit%20operand.)
>
> Thus right shifted vector by 0 shift count when ORed with all zero left-shited vector by 32 shift count still gives the correct result.

That would be good. Thanks!

-------------

PR Comment: https://git.openjdk.org/jdk/pull/24051#issuecomment-2750337201
PR Review Comment: https://git.openjdk.org/jdk/pull/24051#discussion_r2011478714