Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v9]
On Fri, 7 Aug 2026 13:03:48 GMT, Emanuel Peter wrote:
>> Jatin Bhateja has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains nine additional
>> commits since the last revision:
>>
>> - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523
>> - Review comments resolution
>> - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523
>> - Review comments resolution
>> - Review comments resolution
>> - Review comments resolution
>> - Review comments resolution
>> - Review comments resolution
>> - 8382523: Optimize Float16 to integral conversion operations for
>> AVX512-FP16 targets
>
> test/micro/org/openjdk/bench/jdk/incubator/vector/Float16ToIntegralConvBenchmark.java
> line 60:
>
>> 58: i -> {
>> 59: if ((i % 100) == 0) {
>> 60: fp16inp[i] = float16ToRawShortBits(specialValues[i %
>> specialValues.length]);
>
> This comment comes a bit too late, but you will only ever have `i = k * 100`.
> And `specialValues.length = 5`. So you are really always picking
> `specialValues[0] = Float.NaN` here.
>
> It should be `specialValues[(i / 100) % specialValues.length]` instead.
>
> @jatin-bhateja @sviswa7 @missa-prime Could this have an impact on the
> performance measurement? I'm especially thinking about NaN/infinity fixup.
Thanks for pointing this, but it should not impact the benchmark performance
since special case blob short circuits if the conversion result is not
0x8000 ( i.e. max -ve value) otherwise it executes monolithic instruction
sequence for NaN and other value in infinity ranges.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3736262152
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v9]
On Thu, 6 Aug 2026 03:31:58 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 with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains nine additional
> commits since the last revision:
>
> - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523
> - Review comments resolution
> - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523
> - Review comments resolution
> - Review comments resolution
> - Review comments resolution
> - Review comments resolution
> - Review comments resolution
> - 8382523: Optimize Float16 to integral conversion operations for
> AVX512-FP16 targets
test/micro/org/openjdk/bench/jdk/incubator/vector/Float16ToIntegralConvBenchmark.java
line 60:
> 58: i -> {
> 59: if ((i % 100) == 0) {
> 60: fp16inp[i] = float16ToRawShortBits(specialValues[i %
> specialValues.length]);
This comment comes a bit too late, but you will only ever have `i = k * 100`.
And `specialValues.length = 5`. So you are really always picking
`specialValues[0] = Float.NaN` here.
It should be `specialValues[(i / 100) % specialValues.length]` instead.
@jatin-bhateja @sviswa7 @missa-prime Could this have an impact on the
performance measurement? I'm especially thinking about NaN/infinity fixup.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3735927775
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 [v9]
On Thu, 6 Aug 2026 03:31:58 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 with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains nine additional > commits since the last revision: > > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523 > - Review comments resolution > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523 > - Review comments resolution > - Review comments resolution > - Review comments resolution > - Review comments resolution > - Review comments resolution > - 8382523: Optimize Float16 to integral conversion operations for > AVX512-FP16 targets Still good. - Marked as reviewed by sviswanathan (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/30928#pullrequestreview-4876707108
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 [v9]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains nine additional commits since the last revision: - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523 - Review comments resolution - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523 - Review comments resolution - Review comments resolution - Review comments resolution - Review comments resolution - Review comments resolution - 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets - Changes: - all: https://git.openjdk.org/jdk/pull/30928/files - new: https://git.openjdk.org/jdk/pull/30928/files/8405c3da..c57e8b67 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=08 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=07-08 Stats: 11037 lines in 303 files changed: 7215 ins; 2802 del; 1020 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
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v8]
On Wed, 5 Aug 2026 04:48:54 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 missa (Author). - PR Review: https://git.openjdk.org/jdk/pull/30928#pullrequestreview-4866243184
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v7]
On Tue, 4 Aug 2026 18:55:59 GMT, Mohamed Issa wrote: >> Jatin Bhateja has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains seven additional >> commits since the last revision: >> >> - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523 >> - Review comments resolution >> - Review comments resolution >> - Review comments resolution >> - Review comments resolution >> - Review comments resolution >> - 8382523: Optimize Float16 to integral conversion operations for >> AVX512-FP16 targets > > I ran the new tests and micros on machine that supports AVX512-FP16. All runs > went through without issue. Once minor code updates are made, I'll add my > approval to PR. Hi @missa-prime , your comment have been addressed. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5187682541
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v7]
On Tue, 4 Aug 2026 18:48:55 GMT, Mohamed Issa wrote:
>> Jatin Bhateja has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains seven additional
>> commits since the last revision:
>>
>> - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523
>> - Review comments resolution
>> - Review comments resolution
>> - Review comments resolution
>> - Review comments resolution
>> - Review comments resolution
>> - 8382523: Optimize Float16 to integral conversion operations for
>> AVX512-FP16 targets
>
> src/hotspot/cpu/x86/assembler_x86.cpp line 2375:
>
>> 2373: }
>> 2374:
>> 2375: void Assembler::evcvttph2dq(XMMRegister dst, XMMRegister src, int
>> vector_len) {
>
> Please move evcvttph2dq and evcvttph2qq definitions to be just after
> vcvttps2dq definition. That way the two evcvttps2dqs definitions stay next to
> each other.
Addressed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3717997911
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v8]
> 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/5af9c53c..8405c3da Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=07 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=06-07 Stats: 18 lines in 2 files changed: 10 ins; 8 del; 0 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
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v7]
On Mon, 3 Aug 2026 05:52:53 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 with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains seven additional
> commits since the last revision:
>
> - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523
> - Review comments resolution
> - Review comments resolution
> - Review comments resolution
> - Review comments resolution
> - Review comments resolution
> - 8382523: Optimize Float16 to integral conversion operations for
> AVX512-FP16 targets
I ran the new tests and micros on machine that supports AVX512-FP16. All runs
went through without issue. Once minor code updates are made, I'll add my
approval to PR.
src/hotspot/cpu/x86/assembler_x86.cpp line 2375:
> 2373: }
> 2374:
> 2375: void Assembler::evcvttph2dq(XMMRegister dst, XMMRegister src, int
> vector_len) {
Please move evcvttph2dq and evcvttph2qq definitions to be just after vcvttps2dq
definition. That way the two evcvttps2dqs definitions stay next to each other.
src/hotspot/cpu/x86/assembler_x86.hpp line 1328:
> 1326: void evcvttsd2sisq(Register dst, Address src);
> 1327:
> 1328: void evcvttsh2sil(Register dst, XMMRegister src);
Please add a comment above these declarations that is similar to other ones
(e.g., "//Convert with Truncation Scalar Half-Precision Floating-Point Value to
Doubleword Integer").
-
Changes requested by missa (Author).
PR Review: https://git.openjdk.org/jdk/pull/30928#pullrequestreview-4857832909
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3715296357
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3715269859
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v7]
On Mon, 3 Aug 2026 05:52:53 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 with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains seven additional > commits since the last revision: > > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523 > - Review comments resolution > - Review comments resolution > - Review comments resolution > - Review comments resolution > - Review comments resolution > - 8382523: Optimize Float16 to integral conversion operations for > AVX512-FP16 targets > Change must be properly reviewed (2 reviews required, with at least 1 > [Reviewer](https://openjdk.org/bylaws#reviewer), 1 > [Author](https://openjdk.org/bylaws#author)) ^ @jatin-bhateja - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5179048941
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v6]
On Mon, 3 Aug 2026 05:46:08 GMT, Jatin Bhateja wrote: >> Jatin Bhateja has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Review comments resolution > > Hi @iwanowww , @mhaessig , @chhagedorn , > Can you kindly review this patch, we already have one approval from @sviswa7 > on x86 backend part. > > @jatin-bhateja mine was just a minor comment that has been addressed, i'm > > not in a position to do a full review for this issue > > Hi @galderz , No problems, but it seems you added a requested change on the > PR with your comments which blocks the PR from integration (i guess) even if > we have required number of approvals. I'm not aware of that limitation. Seems to me that, as per all the hotspot-compiler PRs, you need 2 approvals and right now you only have one? - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5179043761
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v6]
On Mon, 13 Jul 2026 09:47:26 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 Hi @iwanowww , @mhaessig , @chhagedorn , Can you kindly review this patch, we already have one approval from @sviswa7 on x86 backend part. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5162758706
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v7]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523 - Review comments resolution - Review comments resolution - Review comments resolution - Review comments resolution - Review comments resolution - 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets - Changes: - all: https://git.openjdk.org/jdk/pull/30928/files - new: https://git.openjdk.org/jdk/pull/30928/files/24ec724a..5af9c53c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=05-06 Stats: 628843 lines in 7107 files changed: 395362 ins; 195145 del; 38336 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
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v6]
On Mon, 27 Jul 2026 04:28:05 GMT, Jatin Bhateja wrote: >> Changes requested by galder (Committer). > > Hi @galderz , can you kindly review, your comments have been addressed. > @jatin-bhateja mine was just a minor comment that has been addressed, i'm not > in a position to do a full review for this issue Hi @galderz , you added a requested change on the PR with your comments which blocks the PR from integration (i guess) even if we have required number of approvals. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5162551578
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v6]
On Mon, 27 Jul 2026 04:28:05 GMT, Jatin Bhateja wrote: >> Changes requested by galder (Committer). > > Hi @galderz , can you kindly review, your comments have been addressed. @jatin-bhateja mine was just a minor comment that has been addressed, i'm not in a position to do a full review for this issue - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5162536916
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v6]
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 , can you kindly review, your comments have been addressed. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5087268400
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v6]
On Mon, 13 Jul 2026 09:47:26 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 Hi @iwanowww , Can you kindly review this patch, we already have one approval from @sviswa7 on x86 backend part. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-5071898389
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 [v6]
> 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/84b8ff5e..24ec724a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=04-05 Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 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
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 [v3]
On Thu, 9 Jul 2026 15:24:30 GMT, Sandhya Viswanathan wrote: >> If you permit, I would like to retain the static cost adjustments. They are >> functionally harmless and act as a safeguard against cost-driven selection >> of the component matcher rules, which this patch marks as high cost, during >> the labeling phase. > > It is better to remove these cost adjustment. Addressed, matcher is able to select newly added rules without cost so removed explicit cost. - PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3553183428
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v4]
On Thu, 9 Jul 2026 05:29:14 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 Hi @sviswa7 , your comments have been addressed. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4927292358
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
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v3]
On Thu, 9 Jul 2026 05:23:58 GMT, Jatin Bhateja wrote: >> src/hotspot/cpu/x86/x86.ad line 14731: >> >>> 14729: match(Set dst (ConvF2L (ConvHF2F src))); >>> 14730: effect(TEMP dst, KILL cr, TEMP xtmp); >>> 14731: ins_cost(50); >> >> It looks to me that even without the ins_cost(50) and other ins_cost(200) >> changes, we will still get the direct HF conversion instruction selection. >> Better to remove the ins_cost related changes in this PR. > > If you permit, I would like to retain the static cost adjustments. They are > functionally harmless and act as a safeguard against cost-driven selection of > the component matcher rules, which this patch marks as high cost, during the > labeling phase. It is better to remove these cost adjustment. - PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3552803533
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v3]
On Wed, 8 Jul 2026 17:44:09 GMT, Sandhya Viswanathan
wrote:
>> Jatin Bhateja has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments resolution
>
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 5025:
>
>> 5023: // The sign-flip load is unmasked and vpternlogd applies its
>> immediate bitwise, so
>> 5024: // doubleword granularity is used uniformly across all source types
>> (it is semantically
>> 5025: // equivalent to the quadword form for double here as no mask
>> register is involved).
>
> This comment is not required. The destination element size here is 32 bit so
> we need to generate 32 bit positive saturation values and hence vpternlogd.
Removed
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 5339:
>
>> 5337: XMMRegister xtmp2,
>> KRegister ktmp1, KRegister ktmp2,
>> 5338: AddressLiteral
>> float_sign_flip, Register rscratch, int vec_enc) {
>> 5339: assert(type2aelembytes(to_elem_bt) <= 4, "");
>
> It will be good to add an assert here for avx512vl() for less than 512 bit
> vector length. Same in vector_castHF2L_evex.
Addressed
> src/hotspot/cpu/x86/x86.ad line 14731:
>
>> 14729: match(Set dst (ConvF2L (ConvHF2F src)));
>> 14730: effect(TEMP dst, KILL cr, TEMP xtmp);
>> 14731: ins_cost(50);
>
> It looks to me that even without the ins_cost(50) and other ins_cost(200)
> changes, we will still get the direct HF conversion instruction selection.
> Better to remove the ins_cost related changes in this PR.
If you permit, I would like to retain the static cost adjustments. They are
functionally harmless and act as a safeguard against cost-driven selection of
the component matcher rules, which this patch marks as high cost, during the
labeling phase.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3549098175
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3549097686
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3549093745
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v4]
> 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/a4306626..a42f4dc4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=02-03 Stats: 5 lines in 1 file changed: 2 ins; 3 del; 0 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
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v3]
On Tue, 30 Jun 2026 05:06:41 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
src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 5025:
> 5023: // The sign-flip load is unmasked and vpternlogd applies its
> immediate bitwise, so
> 5024: // doubleword granularity is used uniformly across all source types
> (it is semantically
> 5025: // equivalent to the quadword form for double here as no mask
> register is involved).
This comment is not required. The destination element size here is 32 bit so we
need to generate 32 bit positive saturation values and hence vpternlogd.
src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 5339:
> 5337: XMMRegister xtmp2,
> KRegister ktmp1, KRegister ktmp2,
> 5338: AddressLiteral
> float_sign_flip, Register rscratch, int vec_enc) {
> 5339: assert(type2aelembytes(to_elem_bt) <= 4, "");
It will be good to add an assert here for avx512vl() for less than 512 bit
vector length. Same in vector_castHF2L_evex.
src/hotspot/cpu/x86/x86.ad line 14731:
> 14729: match(Set dst (ConvF2L (ConvHF2F src)));
> 14730: effect(TEMP dst, KILL cr, TEMP xtmp);
> 14731: ins_cost(50);
It looks to me that even without the ins_cost(50) and other ins_cost(200)
changes, we will still get the direct HF conversion instruction selection.
Better to remove the ins_cost related changes in this PR.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3546109612
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3546388104
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3546263636
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v3]
On Tue, 30 Jun 2026 05:06:41 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 Hi @sviswa7 , your comments have been addressed. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4913235323
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v3]
On Tue, 30 Jun 2026 05:06:41 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 Hi @sviswa7 , your comments have been addressed. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4889405289
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v3]
On Tue, 30 Jun 2026 05:06:41 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 Hi @sviswa7 , your comments have been addressed. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4862317780
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v2]
On Fri, 5 Jun 2026 20:58:17 GMT, Sandhya Viswanathan wrote: >> Jatin Bhateja has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Review comments resolution > > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4543: > >> 4541: __ bind(stub.entry()); >> 4542: __ subptr(rsp, 8); >> 4543: __ movl(Address(rsp), src); > > The src also could be a higher bank register for APX, which could increase > the stub size by another byte. Done > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4568: > >> 4566: >> 4567: // Using the APX extended general purpose registers increases the >> instruction encoding size by 1 byte. >> 4568: int max_size = 23 + (UseAPX ? 1 : 0); > > This should be increase by 2 bytes here for APX. Done > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 5070: > >> 5068: } >> 5069: >> 5070: void >> C2_MacroAssembler::vector_cast_float16_to_int_special_cases(XMMRegister dst, >> XMMRegister src, XMMRegister xtmp1, > > This function is very similar to existing > vector_cast_float_to_int_special_cases_evex and > vector_cast_double_to_int_special_cases_evex. It would be good to combine > these with a parameter. Done > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 5092: > >> 5090: } >> 5091: >> 5092: void >> C2_MacroAssembler::vector_cast_float16_to_long_special_cases_evex(XMMRegister >> dst, XMMRegister src, XMMRegister xtmp1, > > This function is very similar to existing > vector_cast_float_to_long_special_cases_evex and > vector_cast_double_to_long_special_cases_evex. It would be good to combine > these with a parameter. Done - PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3496277185 PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3496276922 PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3496277618 PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3496276674
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v2]
On Tue, 12 May 2026 04:15:53 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 Hi @sviswa7 , your comments have been addressed. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4840047253
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v3]
> 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/da2ff1b0..a4306626 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=01-02 Stats: 142 lines in 2 files changed: 6 ins; 91 del; 45 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
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v2]
On Tue, 12 May 2026 04:15:53 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 src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4543: > 4541: __ bind(stub.entry()); > 4542: __ subptr(rsp, 8); > 4543: __ movl(Address(rsp), src); The src also could be a higher bank register for APX, which could increase the stub size by another byte. src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4568: > 4566: > 4567: // Using the APX extended general purpose registers increases the > instruction encoding size by 1 byte. > 4568: int max_size = 23 + (UseAPX ? 1 : 0); This should be increase by 2 bytes here for APX. src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 5070: > 5068: } > 5069: > 5070: void > C2_MacroAssembler::vector_cast_float16_to_int_special_cases(XMMRegister dst, > XMMRegister src, XMMRegister xtmp1, This function is very similar to existing vector_cast_float_to_int_special_cases_evex and vector_cast_double_to_int_special_cases_evex. It would be good to combine these with a parameter. src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 5092: > 5090: } > 5091: > 5092: void > C2_MacroAssembler::vector_cast_float16_to_long_special_cases_evex(XMMRegister > dst, XMMRegister src, XMMRegister xtmp1, This function is very similar to existing vector_cast_float_to_long_special_cases_evex and vector_cast_double_to_long_special_cases_evex. It would be good to combine these with a parameter. - PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3365352480 PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3365381872 PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3365780560 PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3365779714
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v2]
On Tue, 12 May 2026 04:15:53 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 Hi @sviswa7 , can you kindly review this pull request. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4619479546
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v2]
On Tue, 12 May 2026 04:15:53 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 Hi @sviswa7 , can you kindly review this pull request. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4540640901
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v2]
On Tue, 12 May 2026 04:15:53 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 Hi @sviswa7 , can you kindly review this pull request. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4474519293
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v2]
On Thu, 7 May 2026 09:58:15 GMT, Galder Zamarreño wrote:
>> Jatin Bhateja has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments resolution
>
> src/hotspot/cpu/x86/x86.ad line 14734:
>
>> 14732: format %{ "convert_hf2l $dst, $src !\t using $xtmp as TEMP" %}
>> 14733: ins_encode %{
>> 14734: __ convertHF2I(T_LONG, $dst$$Register, $src$$Register,
>> $xtmp$$XMMRegister);
>
> Minor comment: isn't it a bit confusing to call `convertHF2I` with a
> `T_LONG`? Maybe `convertHF2I` could be renamed to `convertHF2X` to not commit
> to the type?
Addressed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3223635348
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets [v2]
> 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/bf170d93..da2ff1b0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30928&range=00-01 Stats: 6 lines in 3 files changed: 0 ins; 0 del; 6 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
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets
On Fri, 24 Apr 2026 16:28:50 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).
Changes requested by galder (Committer).
src/hotspot/cpu/x86/x86.ad line 14734:
> 14732: format %{ "convert_hf2l $dst, $src !\t using $xtmp as TEMP" %}
> 14733: ins_encode %{
> 14734: __ convertHF2I(T_LONG, $dst$$Register, $src$$Register,
> $xtmp$$XMMRegister);
Minor comment: isn't it a bit confusing to call `convertHF2I` with a `T_LONG`?
Maybe `convertHF2I` could be renamed to `convertHF2X` to not commit to the type?
-
PR Review: https://git.openjdk.org/jdk/pull/30928#pullrequestreview-4242968982
PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3200551189
Re: RFR: 8382523: Optimize Float16 to integral conversion operations for AVX512-FP16 targets
On Fri, 24 Apr 2026 16:28:50 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). Hi @sviswa7 , can you kindly review this pull request. - PR Comment: https://git.openjdk.org/jdk/pull/30928#issuecomment-4386034726
