[lworld+fp16] RFR: 8341414: Add support for FP16 conversion routines [v2]

Bhavana Kilambi bkilambi at openjdk.org
Mon Nov 18 16:48:47 UTC 2024


On Thu, 14 Nov 2024 08:49:11 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Bhavana Kilambi has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove intrinsification of conversion methods in Float16
>
> src/hotspot/share/opto/cfgnode.cpp line 1899:
> 
>> 1897: 
>> 1898: // Returns the BasicType of a given convert node and a type, with special handling to ensure that conversions to
>> 1899: // and from half float will return the SHORT basic type, as that wouldn't be returned typically from TypeInt.
> 
> Comment needs re-formatting, "Returns the BasicType of a given convert node based on its opcode or type", not related to this patch though.

Done

> test/jdk/java/lang/Float16/FP16ScalarOperations.java line 177:
> 
>> 175:             System.out.println("Incorrest result for FP16 to double conversion. Expected value:" + expected + " Actual value: " + actual);
>> 176:         }
>> 177:     }
> 
> Adding some constant folding cases covering the following Float16 values to test newly introduced value transforms will be helpful:-
> -   Float16.NaN
> -   Float16.MAX_VALUE (0x1.ffcP+15)
> -   Float16.MIN_VALUE (0x1.0P-24)
> -   Float16.MIN_NORMAL_VALUE (0x.1P-14)
> -   Float16.POSITIVE_INFINITE 
> -   Float16.NEGATIVE_INFINITE
> -   And few cases in the normal value range.

Some of these values are already being tested through the `special_values` array. I have modified to include a few more FP16 special values.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1283#discussion_r1846924624
PR Review Comment: https://git.openjdk.org/valhalla/pull/1283#discussion_r1846925861


More information about the valhalla-dev mailing list