[lworld+fp16] RFR: 8338061: Add support for FP16 unary and ternary operations

[Jatin Bhateja]

On Mon, 19 Aug 2024 13:08:06 GMT, Bhavana Kilambi <bkilambi at openjdk.org> wrote:

> This patch adds support for three unary operators - abs, neg and sqrt and a ternary operator - fma for FP16.
> Both scalar and vector mid-end support along with aarch64 backend support are added.
> Tested all FP16 tests successfully on aarch64.

Looks good to me, apart from one test comment.

src/hotspot/share/opto/compile.cpp line 3691:

> 3689:   case Op_SqrtHF:
> 3690:   case Op_FmaHF:
> 3691:   // Count all float operations that may use FPU

Hey @Bhavana-Kilambi , I am little worried about introducing specialized IR nodes as discussed in [pervious thread](https://github.com/openjdk/valhalla/pull/1196#issuecomment-2283627677),  even though our current strategy fits well with C2 compilation flow without causing much disturbance, going forward we may need to support other reduced precision types BF16 (AVX10.2 has several ISA extensions for BF16), FP8 and INT8, creating unique IR may not be desirable, lets revisit this after features completion with conversion ops.

test/hotspot/jtreg/compiler/c2/irTests/ConvF2HFIdealizationTests.java line 49:

> 47: 
> 48:     @DontCompile
> 49:     public void assertResult(Float16 fp16) {

We can use @Check annotation to comparing the actual and expected results on the lines of following existing test https://github.com/openjdk/valhalla/blob/lworld%2Bfp16/test/hotspot/jtreg/compiler/vectorization/TestFloat16VectorOps.java 


jshell> for (int i = 0; i < 65504; i++)  {
   ...>     float fi = Float.float16ToFloat(Float.floatToFloat16((float)i));
   ...>     if (Float16.float16ToRawShortBits(Float16.sqrt(Float16.valueOf(fi))) != Float.floatToFloat16((float)Math.sqrt(fi))) {
   ...>        System.out.println("Fist precision mismatch at = " + fi); break;
   ...>     }
   ...> }


We can even run through entire value ranges of Float16 type to empirically prove the equivalence..

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

PR Review: https://git.openjdk.org/valhalla/pull/1211#pullrequestreview-2247332113
PR Review Comment: https://git.openjdk.org/valhalla/pull/1211#discussion_r1722947829
PR Review Comment: https://git.openjdk.org/valhalla/pull/1211#discussion_r1722915975