RFR: 8332119: Incorrect IllegalArgumentException for C2 compiled permute kernel

[Sandhya Viswanathan]

On Wed, 29 May 2024 06:10:53 GMT, Jatin Bhateja <jbhateja 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

Other than these two minor comments, the PR looks good to me.

test/hotspot/jtreg/compiler/vectorapi/TestTwoVectorPermute.java line 48:

> 46:            float expected = Float.NaN;
> 47:            // Exceptional index.
> 48:            if (shuf[i] < 0 || shuf[i] >= FSP.length()) {

To better match the specs, this could be:
   if ( (int)shuf[i] < 0 || (int)shuf[i] >= FSP.length()) {

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

PR Review: https://git.openjdk.org/jdk/pull/19442#pullrequestreview-2091849837
PR Review Comment: https://git.openjdk.org/jdk/pull/19442#discussion_r1623038218