On Thu, 12 Mar 2026 23:27:20 GMT, Sandhya Viswanathan
<[email protected]> wrote:
>> Mohamed Issa has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains five commits:
>>
>> - Merge branch 'master' into user/missa-prime/avx10_2
>> - Remove half precision min/max reduction definitions and adjust
>> corresponding benchmarks.
>> - Use alternative instruction flow for half precision reduction loops and
>> add supporting infrastructure.
>> - Merge branch 'master' into user/missa-prime/avx10_2
>> - Replace scalar AVX10.2 floating point min/max instructions with more
>> efficient sequence
>
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 7071:
>
>> 7069: void C2_MacroAssembler::scalar_max_min_fp16_avx10_2(int opcode,
>> XMMRegister dst, XMMRegister src1, XMMRegister src2,
>> 7070: KRegister ktmp) {
>> 7071: vector_max_min_fp16_avx10_2(opcode, dst, src1, src2, ktmp,
>> Assembler::AVX_128bit);
>
> We could use the scalar version of instruction here.
Ok, I switched to the scalar instruction.
> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 1969:
>
>> 1967: vmovw(rscratch, src);
>> 1968: vmovw(dst, rscratch);
>> 1969: }
>
> There is a direct vmovw xmm1, xmm2 instruction in AVX10_2. We should use that
> here.
Yes, this is updated now.
> src/hotspot/cpu/x86/x86.ad line 1743:
>
>> 1741: // Math.min() # Math.max()
>> 1742: // -----------------------------
>> 1743: // (v)ucomis[s/d]. #
>
> Add sh to the comment here.
Ok, it's there now.
> src/hotspot/cpu/x86/x86.ad line 1763:
>
>> 1761: } else {
>> 1762: emit_fp_ucom_double(masm, a, b);
>> 1763: }
>
> It would be good to have a function like emit_fp_ucom(masm, pt, a, b) and use
> that here.
This is modified now.
> src/hotspot/cpu/x86/x86.ad line 1791:
>
>> 1789: } else {
>> 1790: __ movdbl(dst, a);
>> 1791: }
>
> Likewise a function movfp(prec, dst, src) would be good to define and use
> here.
This is modified now.
> src/hotspot/cpu/x86/x86.ad line 7408:
>
>> 7406:
>> 7407: // max = java.lang.Math.max(float a, float b)
>> 7408: instruct maxF_reg_avx10_2(regF dst, regF a, regF b)
>
> We can merge the maxF_reg_avx10_2 and minF_reg_avx10_2 into one instruct say
> minmaxF_reg_avx10_2 with two match rules:
> match(Set dst (MaxF a b));
> match(Set dst (MinF a b));
> Likewise for double minmax.
These are merged now.
> src/hotspot/cpu/x86/x86.ad line 7420:
>
>> 7418: %}
>> 7419:
>> 7420: instruct maxF_reduction_reg_avx10_2(regF dst, regF a, regF b, regF
>> xtmp, rRegI rtmp, rFlagsReg cr)
>
> We can merge the maxF_reduction_reg_avx10_2 and minF_reduction_reg_avx10_2
> into one instruct say minmaxF_reduction_reg_avx10_2 with two match rules:
> match(Set dst (MaxF a b));
> match(Set dst (MinF a b));
> Likewise for double minmax.
These are merged now.
> src/hotspot/cpu/x86/x86.ad line 7435:
>
>> 7433:
>> 7434: // max = java.lang.Math.max(float a, float b)
>> 7435: instruct maxF_reg(legRegF dst, legRegF a, legRegF b, legRegF tmp,
>> legRegF atmp, legRegF btmp)
>
> We can merge the maxF_reg and minF_reg into one instruct say minmaxF_reg with
> two match rules:
> match(Set dst (MaxF a b));
> match(Set dst (MinF a b));
> Likewise for double minmax.
These are merged now.
> src/hotspot/cpu/x86/x86.ad line 7448:
>
>> 7446: %}
>> 7447:
>> 7448: instruct maxF_reduction_reg(legRegF dst, legRegF a, legRegF b, legRegF
>> xtmp, rRegI rtmp, rFlagsReg cr)
>
> We can merge the maxF_reduction_reg and minF_reduction_reg into one instruct
> say minmaxF_reduction_reg with two match rules:
> match(Set dst (MaxF a b));
> match(Set dst (MinF a b));
These are merged now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29831#discussion_r2950027727
PR Review Comment: https://git.openjdk.org/jdk/pull/29831#discussion_r2950028871
PR Review Comment: https://git.openjdk.org/jdk/pull/29831#discussion_r2950032017
PR Review Comment: https://git.openjdk.org/jdk/pull/29831#discussion_r2950030567
PR Review Comment: https://git.openjdk.org/jdk/pull/29831#discussion_r2950030783
PR Review Comment: https://git.openjdk.org/jdk/pull/29831#discussion_r2950032975
PR Review Comment: https://git.openjdk.org/jdk/pull/29831#discussion_r2950033220
PR Review Comment: https://git.openjdk.org/jdk/pull/29831#discussion_r2950033483
PR Review Comment: https://git.openjdk.org/jdk/pull/29831#discussion_r2950034066
