RFR: 8321010: RISC-V: C2 RoundVF [v3]

[Fei Yang]

On Thu, 14 Mar 2024 11:41:52 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you have a review on this patch to add RoundVF/RoundDF intrinsics?
>> Thanks!
>> 
>> ## Tests
>> 
>> test/hotspot/jtreg/compiler/vectorization/TestRoundVectRiscv64.java test/hotspot/jtreg/compiler/c2/cr6340864/TestFloatVect.java test/hotspot/jtreg/compiler/c2/cr6340864/TestDoubleVect.java test/hotspot/jtreg/compiler/floatingpoint/TestRound.java
>> 
>> test/jdk/java/lang/Math/RoundTests.java
>
> Hamlin Li has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits:
> 
>  - merge master
>  - fix space
>  - add tests
>  - add test cases
>  - v2: (src + 0.5) + rdn
>  - Fix corner cases
>  - Merge branch 'master' into round-F+D-v
>  - refine code
>  - RoundVF/D: Initial commit

Changes requested by fyang (Reviewer).

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

> 3668: instruct vround_f(vReg dst, vReg src, fRegF tmp, vRegMask_V0 v0) %{
> 3669:   match(Set dst (RoundVF src));
> 3670:   effect(TEMP_DEF dst, TEMP tmp);

You might want to add `TEMP v0` in effect as v0 is clobbered in `java_round_float_v`. Similar for `vround_d`.

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

> 3673:     __ csrwi(CSR_FRM, C2_MacroAssembler::rdn);
> 3674:     BasicType bt = Matcher::vector_element_basic_type(this);
> 3675:     __ vsetvli_helper(bt, Matcher::vector_length(this));

I think the code will be more readable if you put `csrwi` and `vsetvli_helper` instructions in `java_round_float_v`. We can add `BasicType bt` as parameter for `java_round_float_v`. Similar for vround_d.

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

> 3676:     __ java_round_float_v(as_VectorRegister($dst$$reg), as_VectorRegister($src$$reg),
> 3677:                           as_FloatRegister($tmp$$reg));
> 3678:     __ csrwi(CSR_FRM, C2_MacroAssembler::rne);

I don't think it's necessary to restore `CSR_FRM` to `rne` after `java_round_float_v`. As I remembered, we always set the required rounding mode in other places where it makes an effect. Similar for vround_d.

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

PR Review: https://git.openjdk.org/jdk/pull/17745#pullrequestreview-1988809339
PR Review Comment: https://git.openjdk.org/jdk/pull/17745#discussion_r1557488393
PR Review Comment: https://git.openjdk.org/jdk/pull/17745#discussion_r1557492135
PR Review Comment: https://git.openjdk.org/jdk/pull/17745#discussion_r1557494390