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

Jatin Bhateja jbhateja at openjdk.org
Wed Jul 10 20:27:38 UTC 2024


On Wed, 10 Jul 2024 08:22:13 GMT, Bhavana Kilambi <bkilambi at openjdk.org> wrote:

>> 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
>
> Bhavana Kilambi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review comments

Overall non-architecture part looks good to me adding some minor comments.

test/hotspot/jtreg/compiler/vectorization/TestFP16VectorReinterpretConv.java line 1:

> 1: /*

We have another test in the same directory with TestFloat16* nomenclature, I think we should pick one naming scheme.

test/hotspot/jtreg/compiler/vectorization/TestFP16VectorReinterpretConv.java line 2:

> 1: /*
> 2:  * Copyright (c) 2022, 2024, Arm Limited. All rights reserved.

Suggestion:

 * Copyright (c) 2024, Arm Limited. All rights reserved.

test/hotspot/jtreg/compiler/vectorization/TestFP16VectorReinterpretConv.java line 75:

> 73:     }
> 74: 
> 75:     public void checkResult() {

Suggestion:

    }

    @Check(test="test")
    public void checkResult() {

Should suffice since fin and flout are fields and accessible to all instance methods. We can also lift this limitation from https://github.com/openjdk/valhalla/pull/1096/files#diff-a4b825d43ecb78aad4d8f04965592dfdb9b56ac3da88de6d4117f39c74efc2ae

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

PR Review: https://git.openjdk.org/valhalla/pull/1096#pullrequestreview-2168654025
PR Review Comment: https://git.openjdk.org/valhalla/pull/1096#discussion_r1672016850
PR Review Comment: https://git.openjdk.org/valhalla/pull/1096#discussion_r1672003762
PR Review Comment: https://git.openjdk.org/valhalla/pull/1096#discussion_r1672025350


More information about the valhalla-dev mailing list