On Tue, 30 Jun 2026 05:06:41 GMT, Jatin Bhateja <[email protected]> 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. >> >> <img width="1125" height="636" alt="image" >> 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
