[lworld+fp16] RFR: 8330021: AArch64: Add backend support for FP16 add operation [v4]

[Bhavana Kilambi]

On Fri, 5 Jul 2024 10:18:36 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Bhavana Kilambi has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:
>> 
>>  - Merge branch 'lworld+fp16'
>>  - Remove couple of IR testcases + add --enable-preview flag to tests
>>  - Merge branch 'lworld+fp16'
>>  - Add a missing return statement in the ad file
>>  - 8330021: AArch64: Add backend support for FP16 add operation
>>    
>>    This commit [1] adds initial support for FP16 operations and adds
>>    backend support for FP16 add operation for X86. This task adds backend
>>    support for scalar and vector FP16 add operation on aarch64.
>>    
>>    [1] https://github.com/openjdk/valhalla/commit/f03fb4e4ee4d59ed692d0c26ddce260511f544e7#diff-a799ce8da7f3062bb3699beb65aae504840c649942032e325c2a50f88a2869ad
>
> test/hotspot/jtreg/compiler/intrinsics/float16/TestFP16ScalarAdd.java line 58:
> 
>> 56:     @Test
>> 57:     @IR(applyIfCPUFeature = {"avx512_fp16", "true"}, counts = {IRNode.ADD_HF, "> 0", IRNode.REINTERPRET_S2HF, "> 0", IRNode.REINTERPRET_HF2S, "> 0"})
>> 58:     @IR(applyIfCPUFeatureAnd = {"fphp", "true", "asimdhp", "true"}, counts = {IRNode.ADD_HF, "> 0", IRNode.REINTERPRET_S2HF, "> 0", IRNode.REINTERPRET_HF2S, "> 0"})
> 
> Please have one IR rules with unified feature list for x86 and aarach64. 
> Not sure, but asimdhp is only needed for SIMD instructions I guess.

Thanks @jatin-bhateja for your comments.
On Neon (non-SVE) machines or when we are testing scalar FP16 operations, Float16 is supported only if both `fphp` and `asimdhp` features are supported. So if we have to write it as a single rule, it needs to be something like - `avx512_fp16 == true || (fphp == true && asimdhp == true)` and I wasn't sure if this can be done in single IR rule here. Where ever I could write it as a single rule, I have done that for ex. clubbing `avx512_fp16` and `sve` in the same IR rule for `TestFloat16VectorSum.java`.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1096#discussion_r1666669442