RFR: 8352585: Add special case handling for Float16.max/min x86 backend [v5]

[Emanuel Peter]

On Wed, 26 Mar 2025 19:24:51 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> This bugfix patch adds the special handling as per x86 AVX512-FP16 ISA specification[1][2] to compute max/min operations with +/-0.0 or NaN operands.
>> 
>> Special handling leverage the instruction semantic, central idea is to shuffle the operands such that smaller input gets assigned to second operand for min operation or a larger input gets assigned to second operand for max operation, in addition result equals NaN if an unordered comparison detects first input as a NaN value else we return the result of min/max operation. 
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>> 
>> [1] https://www.felixcloutier.com/x86/vminsh
>> [2] https://www.felixcloutier.com/x86/vmaxsh
>
> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments resolution

I have not looked at the x64 instructions, but only the tests again.

I have noticed that you only cover specific values. You could improve tests with this:
- Add non-canonical NaN values.
- Just iterate over all possible Float16 input pairs. It's onls `2^32`, that should be feasible! Then compare compiled vs interpreted results.

It seems that bugs like these happen because somehow we do not systematically cover all inputs. Maybe we should do the same for all Float16 operations?

test/hotspot/jtreg/compiler/intrinsics/float16/TestFloat16MaxMinSpecialValues.java line 57:

> 55:     @Run(test = "testMaxNaNOperands")
> 56:     public void launchMaxNaNOperands() {
> 57:         RES = testMaxNaNOperands(SRC, Float16.NaN);

You are not only using the "canonical" `NaN` in the tests. Are there not other encodings?

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24169#pullrequestreview-2721398291
PR Review Comment: https://git.openjdk.org/jdk/pull/24169#discussion_r2016545317