RFR: 8321001: RISC-V: C2 SignumVF [v2]

[Hamlin Li]

On Mon, 4 Dec 2023 06:51:25 GMT, Fei Yang <fyang at openjdk.org> wrote:

> Hi, Did you check the C2 JIT code? I am wondering whether the newly-added code is covered well by the tests performed.

I did not check the JIT code, but the tests did help to catch some bugs when I implemented the intrinsic.

> src/hotspot/cpu/riscv/assembler_riscv.hpp line 1592:
> 
>> 1590:   INSN(vfsgnj_vf, 0b1010111, 0b101, 0b001000);
>> 1591:   INSN(vfsgnjx_vf, 0b1010111, 0b101, 0b001010);
>> 1592:   INSN(vfsgnjn_vf, 0b1010111, 0b101, 0b001001);
> 
> Not used anywhere?

I can remove them if you think it's better to do it.
Reason I added them is that they're quite similar instructions and it's annoying to lookup the instruction formatting in spec and add every single one when needing them.

> src/hotspot/cpu/riscv/riscv_v.ad line 3670:
> 
>> 3668:   match(Set dst (SignumVF dst (Binary zero one)));
>> 3669:   match(Set dst (SignumVD dst (Binary zero one)));
>> 3670:   effect(TEMP_DEF dst);
> 
> v0 is clobbered in `C2_MacroAssembler::signum_fp_v`. Shouldn't we add a `TEMP v0` to the effect?

Thanks for catching! Fixed.

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

PR Comment: https://git.openjdk.org/jdk/pull/16925#issuecomment-1840847990
PR Review Comment: https://git.openjdk.org/jdk/pull/16925#discussion_r1415657006
PR Review Comment: https://git.openjdk.org/jdk/pull/16925#discussion_r1415657364