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

[Fei Yang]

On Tue, 5 Dec 2023 15:03:47 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you review the patch to add intrinisc SignumVF/SignumVD on riscv?
>> Thanks
>> 
>> ## Test
>> test/hotspot/jtreg/compiler/intrinsics/
>> test/hotspot/jtreg/compiler/vectorapi/
>> and tests found via:
>> grep -nr test/hotspot/jtreg/ -we Math.signum
>> and test found via:
>> grep -nr test/jdk/ -we Math.signum
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   enable TestSignumVector.java on riscv

Changes requested by fyang (Reviewer).

src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1687:

> 1685:   mv(t0, fclass_mask::zero | fclass_mask::nan);
> 1686:   vand_vx(v0, v0, t0);
> 1687:   vmseq_vv(v0, v0, zero);

I don't think that the input `zero` (a vector of floating-point 0.0) is appropriate here for `vmseq_vv` which does vector integer comparison. Why not do `vmseq_vi(v0, v0, 0)` instead? This will also help remove the `zero` parameter of this function.

src/hotspot/cpu/riscv/riscv_v.ad line 3675:

> 3673:     BasicType bt = Matcher::vector_element_basic_type(this);
> 3674:     __ signum_fp_v(as_VectorRegister($dst$$reg), bt, Matcher::vector_length(this),
> 3675:                   as_VectorRegister($zero$$reg), as_VectorRegister($one$$reg));

Nit: maybe leave one extra space here to align with parameters of the preceding line.

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

PR Review: https://git.openjdk.org/jdk/pull/16925#pullrequestreview-1766524186
PR Review Comment: https://git.openjdk.org/jdk/pull/16925#discussion_r1416579404
PR Review Comment: https://git.openjdk.org/jdk/pull/16925#discussion_r1416582961