RFR: 8347794: RISC-V: Add Zfhmin - Float cleanup [v4]

[Fei Yang]

On Mon, 20 Jan 2025 13:48:51 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> Hey, please consider.
>> 
>> I'm looking making RVA23 support complete and I was looking into adding Zfhmin.
>> I notice Zfh instructions didn't have any asserts checking if this extensions was enabled.
>> I then notice that we had the inferior instruction description using a funct7 instead of FMT + funct5.
>> We also had this same format repeated many times.
>> 
>> This patch takes all instructions format duplicates and replaces them with 'fp_base'.
>> fadd_s is thus described in code as:
>> `fp_base<S_32_sp, 0b00000>(Rd, Rs1, Rs2, rm);`
>> Instead of:
>> `INSN(fadd_s,   0b1010011, 0b0000000);`
>> 
>> It then moves zfh/zfhmin to a sepererate section and assert checks them.
>> 
>> Also as a bonus RoundingMode uses camel case while fclass_mask C, style.
>> So I changed fclass_mask to FClassBit, now thinking about it not sure if it should be bit or mask.
>> As in the context of the instruction fclass it is a bit, but when using the helpers e.g. zero it's a mask.
>> 
>> Tested:
>> jdk/java/lang/Math
>> jdk/java/util/Formatter/
>> jdk/java/lang/Float/
>> jdk/java/lang/Double/
>> hotspot/jtreg/compiler/floatingpoint/
>> hotspot/jtreg/compiler/c2/FloatingPointFoldingTest.java
>> hotspot/jtreg/compiler/eliminateAutobox/
>> hotspot/jtreg/vmTestbase/jit/
>> 
>> And tier1 twice, no FP issues. (Using UseZfh/min)
>> 
>> Thanks, Robbin
>
> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Fix nit
>  - Merge branch 'master' into zfhmin
>  - FClassBits
>  - Merge branch 'master' into zfhmin
>  - Baseline

Thanks for the update. One minor question remain.

src/hotspot/cpu/riscv/assembler_riscv.hpp line 941:

> 939: 
> 940:   template <FmtPrecision Fmt, uint8_t funct5>
> 941:   void fp_base(FloatRegister Rd, FloatRegister Rs1, int Rs2, int8_t rm) {

Minor question: Should `int Rs2` be `uint8_t Rs2` for consistency with friends?

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

Marked as reviewed by fyang (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23130#pullrequestreview-2563291463
PR Review Comment: https://git.openjdk.org/jdk/pull/23130#discussion_r1922950560