RFR: 8345298: RISC-V: Add riscv backend for Float16 operations - scalar [v5]

[Hamlin Li]

On Mon, 10 Mar 2025 03:29:56 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - clean
>>  - renaming
>
> src/hotspot/cpu/riscv/riscv.ad line 4975:
> 
>> 4973: 
>> 4974:   ins_pipe(fp_load_constant_s);
>> 4975: %}
> 
> Can we put this two after `loadConD0`? Then we have a more consistent order for the three variants (F->D->FH) at each place.

In fact, in my mind the order should be FH->F->D, considering the width changing from 16->32->64.

> src/hotspot/cpu/riscv/riscv.ad line 8324:
> 
>> 8322: %}
>> 8323: 
>> 8324: instruct min_max_HF_reg(fRegF dst, fRegF src1, fRegF src2)
> 
> It will be easier for people to map to exitsting F/D variants if we use similar names like `sqrtHF_reg`, `minHF_reg_reg`, `maxHF_reg_reg`, `maddHF_reg_reg`, and `binOpsHF_reg_reg` for the the HF variant.

I think there is no convention of instruct naming, as long as there is no duplicate names, and normally we don't care about the instruct name too much. What really matters is the match rule, e.g. MinHF/ManHF and so on, which indicates what the instructs do.

> src/hotspot/cpu/riscv/vm_version_riscv.hpp line 301:
> 
>> 299:   static bool supports_fencei_barrier() { return ext_Zifencei.enabled(); }
>> 300: 
>> 301:   static bool supports_float16_float_conversion() {
> 
> Is it safe to simply rename this as `supports_float16` like other CPUs? I suppose it will still work in functionality even if we only have the `Zfhmin` extension, right?

In fact, I think `supports_float16` is confusing, in particular on riscv, as e.g. `AddHF` requires `UseZfh`, but `ConvF2HF` requires `UseZfh || UseZfhmin`, so I think it's better to keep supports_float16_float_conversion, as its naming is much more explicit and clearer.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23844#discussion_r1987723823
PR Review Comment: https://git.openjdk.org/jdk/pull/23844#discussion_r1987724274
PR Review Comment: https://git.openjdk.org/jdk/pull/23844#discussion_r1987725909