RFR: 8332119: Incorrect IllegalArgumentException for C2 compiled permute kernel

[Sandhya Viswanathan]

On Fri, 31 May 2024 21:01:35 GMT, Sandhya Viswanathan <sviswanathan at openjdk.org> wrote:

>> Currently inline expansion of vector to shuffle conversion simply type casts the vector holding indexes to byte vector[1] where as fallback implementation[2] also wraps the indexes to a valid index range [0, VEC_LEN-1) or generates a -ve index for exceptional / OOB indices.
>> 
>> This patch extends the conversion inline expander to match the fall back implementation. This imposes around 20% performance tax on Vector.toShuffle() intrinsic but fixes this functional bug.
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>> 
>> PS: Patch also fixes an incorrectness issue reported with [JDK-8332118](https://bugs.openjdk.org/browse/JDK-8332118)
>> 
>> [1] https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/FloatVector.java#L2352
>> [2] https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractShuffle.java#L58
>
> src/hotspot/share/opto/vectorIntrinsics.cpp line 2411:
> 
>> 2409:      op = wrap_indexes(op, num_elem_to, elem_bt_to);
>> 2410:   }
>> 2411: 
> 
> The wrap_indexes is needed only for two vector rearrange. It looks to me that doing a wrap_indexes here at convert would force it for single vector rearrange (or selectFrom) and thereby reduce the performance for that case as well. Please note that the single vector rearrange throws "IndexOutOfBoundsException" and doesn't need to do a wrap.

Please ignore the above comment. I verified that each index is partially wrapped as part of toShuffle().  We should name the wrap_indexes() to partially_wrap_indexes() for clarity.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19442#discussion_r1622996294