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

[Sandhya Viswanathan]

On Mon, 10 Mar 2025 08:56:17 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Sandhya Viswanathan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review comments
>
> test/hotspot/jtreg/compiler/vectorization/TestFloat16ToFloatConv.java line 29:
> 
>> 27: * @bug 8350835
>> 28: * @summary Test bug fix for JDK-8350835 discovered through Template Framework
>> 29: * @requires vm.compiler2.enabled
> 
> Suggestion:
> 
> 
> Is this restriction necessary? I generally prefer running tests on all platforms, and only restricting IR rules. That way we can get more test coverage with result verification.

The restriction is not necessary, removed.

> test/hotspot/jtreg/compiler/vectorization/TestFloat16ToFloatConv.java line 31:
> 
>> 29: * @requires vm.compiler2.enabled
>> 30: * @library /test/lib /
>> 31: * @run main/othervm -XX:-TieredCompilation -XX:CompileOnly=compiler.vectorization.TestFloat16ToFloatConv::test* compiler.vectorization.TestFloat16ToFloatConv
> 
> Are the additional flags really necessary for reproducing the bug? I would suspect not really. The IR framework already takes care of ensuring we run with C2 compilation.

Removed the additional flags.

> test/hotspot/jtreg/compiler/vectorization/TestFloat16ToFloatConv.java line 47:
> 
>> 45:     private static int[] aI = new int[SIZE];
>> 46:     private static long[] aL = new long[SIZE];
>> 47:     private static float[] goldB, goldS, goldI, goldL;
> 
> Are you testing for `char` as well?

Added test for char.

> test/hotspot/jtreg/compiler/vectorization/TestFloat16ToFloatConv.java line 55:
> 
>> 53:             aI[i] = RANDOM.nextInt();
>> 54:             aL[i] = RANDOM.nextLong();
>> 55:         }
> 
> I would prefer if we could start using `Generators`. There is a `fill` method for arrays. It generates more "interesting" values. It is not super relevant here, but it would be nice if we made this common practice now ;)

The Generators don't support the bytes, shorts, and chars yet so ended up not using them for this PR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23939#discussion_r1988049345
PR Review Comment: https://git.openjdk.org/jdk/pull/23939#discussion_r1988048883
PR Review Comment: https://git.openjdk.org/jdk/pull/23939#discussion_r1988048633
PR Review Comment: https://git.openjdk.org/jdk/pull/23939#discussion_r1988048278