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

[Xiaohong Gong]

On Fri, 14 Mar 2025 09:43:15 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

> The following assertion fails on AArch64:
> 
> 
>   Internal Error (jdk/src/hotspot/cpu/aarch64/assembler_aarch64.hpp:2991), pid=3822987, tid=3823007
>   assert((1 << ((T>>1)+3)) > shift) failed: Invalid Shift value
> 
> 
> with a simple Vector API case:
> 
> public static IntVector test() {
>     IntVector iv = IntVector.zero(IntVector.SPECIES_128);
>     return iv.lanewise(VectorOperators.ROR, iv);
> }
> 
> 
> On AArch64, vector `ROR/ROL` (rotate right/left) operations are implemented with a combination of shifts. Please see the pattern for `ROR`:
> 
> 
>   lsr dst1, src, cnt            // unsigned right shift
>   lsl dst2, src, bitSize - cnt  // left shift
>   orr dst, dst1, dst2            // logical or
> 
> where `bitSize` is the element type width (e.g. `32` for `int`). In above case, `cnt` is a zero constant, resulting in a left shift of 32 (`bitSize - 0`), which exceeds the instruction's valid shift count range and triggers the assertion. To fix this, we need to mask the shift count to ensure it stays within valid range when calculating shift counts for rotate operations: `shiftCnt = shiftCnt & (bitSize - 1)`.
> 
> Note that the mask is only necessary for constant shift counts. This not only fixes the assertion failure, but also allows `ROR/ROL src, 0` to be optimized to `src` directly.
> 
> For vector variables as shift counts, the masking can be safely omitted because:
> 1. Vector shift instructions that take a vector register as the shift count may not automatically apply modulo arithmetic based on register size. When the shift count is `32` for int type, the result may be either `zeros` or `src`. However, this doesn't affect correctness for rotate since the final result is combined with `src` using a logical `OR` operation.
> 2. It saves a vector logical `AND` for masking, which is friendly to the performance.

This pull request has now been integrated.

Changeset: f9bcef4d
Author:    Xiaohong Gong <xgong at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/f9bcef4dba569701ebed7762fc8730d552325382
Stats:     304 lines in 2 files changed: 303 ins; 0 del; 1 mod

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

Reviewed-by: chagedorn, epeter, jbhateja, adinn

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

PR: https://git.openjdk.org/jdk/pull/24051