[lworld+fp16] RFR: 8330021: AArch64: Add backend support for FP16 add operation [v4]
Bhavana Kilambi
bkilambi at openjdk.org
Fri Jul 5 13:36:35 UTC 2024
On Fri, 5 Jul 2024 13:26:22 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:
>> 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"})
>>
>> 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`.
>
> Current rules looks ok to me, I just wrongly interpreted the results on x86 where one of the two IR rules failed, and that is bound to happene since these features are AARCH64 specific.
By "IR rules failed" you mean IR rule was not applied due to "feature constraints not met" right (for the second IR rule)? Hope the test did not fail. It should not ideally.
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/1096#discussion_r1666816517
More information about the valhalla-dev
mailing list