Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
On Thu, 9 Jul 2026 22:33:21 GMT, Sandhya Viswanathan wrote: >> Jatin Bhateja has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Review comments resolution > > The PR looks good to me now. Please also do a merge with master. Thanks @sviswa7 and @missa-prime for review approvals - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5211154582
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
On Thu, 9 Jul 2026 22:33:21 GMT, Sandhya Viswanathan wrote: >> Jatin Bhateja has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Review comments resolution > > The PR looks good to me now. Please also do a merge with master. Hi @sviswa7 , we need a reapproval from you here to transition this to ready for integration - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5200247360
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
On Fri, 10 Jul 2026 16:00:13 GMT, Emanuel Peter wrote: > Just a few drive-by nits. May review more next week. Hi @eme64 , your comments have been addressed. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5030530982
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
On Fri, 10 Jul 2026 16:00:13 GMT, Emanuel Peter wrote: > Just a few drive-by nits. May review more next week. Hi @eme64 , your comments have been addressed. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4990242745
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
On Mon, 13 Jul 2026 09:42:00 GMT, Jatin Bhateja wrote:
>> test/micro/org/openjdk/bench/jdk/incubator/vector/Float16ToIntegralConvBenchmark.java
>> line 53:
>>
>>> 51: bout = new byte[size];
>>> 52:
>>> 53: IntStream.range(0, size).forEach(i -> {fp16inp[i] =
>>> Float.floatToFloat16((float)i);});
>>
>> Suggestion:
>>
>> Arrays.setAll(fp16inp, i -> Float.floatToFloat16((float)i));
>>
>> Would that not be a bit simpler?
>
> The issue: java.util.Arrays.setAll has no overload accepting a short[]
Ah, unfortunate. Ok.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3570705898
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
On Fri, 10 Jul 2026 15:58:47 GMT, Emanuel Peter wrote:
>> Jatin Bhateja has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments resolution
>
> test/micro/org/openjdk/bench/jdk/incubator/vector/Float16ToIntegralConvBenchmark.java
> line 53:
>
>> 51: bout = new byte[size];
>> 52:
>> 53: IntStream.range(0, size).forEach(i -> {fp16inp[i] =
>> Float.floatToFloat16((float)i);});
>
> Suggestion:
>
> Arrays.setAll(fp16inp, i -> Float.floatToFloat16((float)i));
>
> Would that not be a bit simpler?
The issue: java.util.Arrays.setAll has no overload accepting a short[]
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3569574956
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
On Thu, 9 Jul 2026 16:27:13 GMT, Jatin Bhateja wrote:
>> Patch optimizes Float16 to integral conversion operations. Currently, its a
>> two step process where by first a Float16 value is
>> converted to a single precision floating point value followed by a
>> conversion to an integral value.
>>
>> x86 targets supporting AVX512-FP16 feature (Intel Sapphire Rapids+ and
>> upcoming AMD Zen6) provides direct instruction to convert a Float16 value to
>> integral value.
>>
>> Following are the performance numbers of micro benchmark included with the
>> patch on Granite Rapids with and without auto-vectorization.
>>
>> > src="https://github.com/user-attachments/assets/ca6e6757-1579-475f-8307-9454c7c025c1";
>> />
>>
>> Kindly review and share your feedback.
>>
>> Best Regards,
>> Jatin
>>
>> -
>> - [x] I confirm that I make this contribution in accordance with the
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Jatin Bhateja has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Review comments resolution
Just a few drive-by nits. May review more next week.
test/hotspot/jtreg/compiler/vectorapi/TestFloat16ToIntegralConv.java line 40:
> 38: * @requires vm.compiler2.enabled
> 39: * @library /test/lib /
> 40: * @run driver compiler.vectorapi.TestFloat16ToIntegralConv
Suggestion:
* @run driver ${test.main.class}
Please always use this.
test/micro/org/openjdk/bench/jdk/incubator/vector/Float16ToIntegralConvBenchmark.java
line 43:
> 41: long [] lout;
> 42: short [] sout;
> 43: byte [] bout;
Suggestion:
short[] fp16inp;
int[] iout;
long[] lout;
short[] sout;
byte[] bout;
Formatting nit.
test/micro/org/openjdk/bench/jdk/incubator/vector/Float16ToIntegralConvBenchmark.java
line 53:
> 51: bout = new byte[size];
> 52:
> 53: IntStream.range(0, size).forEach(i -> {fp16inp[i] =
> Float.floatToFloat16((float)i);});
Suggestion:
Arrays.setAll(fp16inp, i -> Float.floatToFloat16((float)i));
Would that not be a bit simpler?
-
Changes requested by epeter (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/30928#pullrequestreview-4672995199
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3560255775
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3560258363
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3560271373
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
On Thu, 7 May 2026 09:58:22 GMT, Galder Zamarreño wrote: >> Jatin Bhateja has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Review comments resolution > > Changes requested by galder (Committer). Hi @galderz , we need your approval to transition this to ready for integration state. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4936805027
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
On Thu, 9 Jul 2026 16:27:13 GMT, Jatin Bhateja wrote: >> Patch optimizes Float16 to integral conversion operations. Currently, its a >> two step process where by first a Float16 value is >> converted to a single precision floating point value followed by a >> conversion to an integral value. >> >> x86 targets supporting AVX512-FP16 feature (Intel Sapphire Rapids+ and >> upcoming AMD Zen6) provides direct instruction to convert a Float16 value to >> integral value. >> >> Following are the performance numbers of micro benchmark included with the >> patch on Granite Rapids with and without auto-vectorization. >> >> > src="https://github.com/user-attachments/assets/ca6e6757-1579-475f-8307-9454c7c025c1"; >> /> >> >> Kindly review and share your feedback. >> >> Best Regards, >> Jatin >> >> - >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Jatin Bhateja has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments resolution Marked as reviewed by sviswanathan (Reviewer). The PR looks good to me now. Please also do a merge with master. - PR Review: https://git.openjdk.org/jdk/pull/30928#pullrequestreview-4667223531 PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4930150529
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v5]
> Patch optimizes Float16 to integral conversion operations. Currently, its a > two step process where by first a Float16 value is > converted to a single precision floating point value followed by a conversion > to an integral value. > > x86 targets supporting AVX512-FP16 feature (Intel Sapphire Rapids+ and > upcoming AMD Zen6) provides direct instruction to convert a Float16 value to > integral value. > > Following are the performance numbers of micro benchmark included with the > patch on Granite Rapids with and without auto-vectorization. > > src="https://github.com/user-attachments/assets/ca6e6757-1579-475f-8307-9454c7c025c1"; > /> > > Kindly review and share your feedback. > > Best Regards, > Jatin > > - > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision: Review comments resolution - Changes: - all: https://git.openjdk.org/jdk/pull/30928/files - new: https://git.openjdk.org/jdk/pull/30928/files/a42f4dc4..84b8ff5e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=03-04 Stats: 9 lines in 1 file changed: 0 ins; 8 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/30928.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/30928/head:pull/30928 PR: https://git.openjdk.org/jdk/pull/30928
