RFR: 8350835: C2 SuperWord: assert/wrong result when using Float.float16ToFloat with byte instead of short input

[Jatin Bhateja]

On Fri, 7 Mar 2025 01:56:49 GMT, Sandhya Viswanathan <sviswanathan at openjdk.org> wrote:

> Float.float16ToFloat generates wrong vectorized code in product build and asserts in fastdebug/debug when argument is of type byte, int, or long array. The short term solution is to not auto vectorize in these cases.
> 
> Review comments are welcome.
> 
> Best Regards,
> Sandhya

Hi @sviswa7, Fix looks resonable to me. Kindly consider including some suggestions.

Best Regards

test/hotspot/jtreg/compiler/vectorization/TestFloat16ToFloatConv.java line 62:

> 60:     }
> 61: 
> 62:     @Test

Suggestion:

    @Test
    @IR(failOn = { IRNode.VECTOR_CAST_HF2F }, applyIfCPUFeatureOr = { "avx512vl", "true", "f16c", "true" })

test/hotspot/jtreg/compiler/vectorization/TestFloat16ToFloatConv.java line 71:

> 69:     }
> 70: 
> 71:     @Test

Suggestion:

    @Test
    @IR(counts = { IRNode.VECTOR_CAST_HF2F, " >0 " }, applyIfCPUFeatureOr = { "avx512vl", "true", "f16c", "true" })

test/hotspot/jtreg/compiler/vectorization/TestFloat16ToFloatConv.java line 80:

> 78:     }
> 79: 
> 80:     @Test

Suggestion:

    /*
     *  C2 handles i2s conversion by constraining the value range of the integral argument; thus
     *  argument fed to ConvHF2F is of type T_INT. Fix for JDK-8350835 skips over vectorizing such a case
     *  for now.
     */
     @Test
     @IR(failOn = { IRNode.VECTOR_CAST_HF2F }, applyIfCPUFeatureOr = { "avx512vl", "true", "f16c", "true" })

test/hotspot/jtreg/compiler/vectorization/TestFloat16ToFloatConv.java line 89:

> 87:     }
> 88: 
> 89:     @Test

``` suggestion
    /*
     * C2 handles this in two steps: l2i handling creates ConvL2I IR ,followed by i2s conversion which onstrains the
     * value range of the integral argument; thus, the argument fed to ConvHF2F is of type T_INT. Fix for
     * JDK-8350835 skip over vectorizing such a case for now.
     */
    @Test
    @IR(failOn = { IRNode.VECTOR_CAST_HF2F }, applyIfCPUFeatureOr = { "avx512vl", "true", "f16c", "true" })

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

PR Review: https://git.openjdk.org/jdk/pull/23939#pullrequestreview-2666472302
PR Review Comment: https://git.openjdk.org/jdk/pull/23939#discussion_r1984601608
PR Review Comment: https://git.openjdk.org/jdk/pull/23939#discussion_r1984602177
PR Review Comment: https://git.openjdk.org/jdk/pull/23939#discussion_r1984603876
PR Review Comment: https://git.openjdk.org/jdk/pull/23939#discussion_r1984605156