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

[Hamlin Li]

On Tue, 9 Apr 2024 11:32:52 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> 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
>
> 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`.

Thanks for catching! Fixed.

> 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.

done.

> 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.

Seems it should be unnecessary, but without it, 
1. there will test failure. e.g. test_suba which uses `vsub` intrinsic, and it does not set rounding mode explicitly.
2. an assert at `src/hotspot/os/linux/os_linux.cpp:1948` which is triggered at the end of program (when calling System.exit in the test).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17745#discussion_r1558048408
PR Review Comment: https://git.openjdk.org/jdk/pull/17745#discussion_r1558048493
PR Review Comment: https://git.openjdk.org/jdk/pull/17745#discussion_r1558048561