RFR: 8347981: RISC-V: Add Zfa zli imm loads

[Robbin Ehn]

On Fri, 17 Jan 2025 15:54:00 GMT, Ludovic Henry <luhenry at openjdk.org> wrote:

>> Hi, please consider!
>> 
>> This patch the Zfa zli floating point immediate load.
>> As a bonus it adds fmv_?_x(Rd, zr); for loading fp/dp 0x0.
>> There are some more instruction in Zfa, but this was such a clear use-case so I only did fli as a start.
>> 
>> When using one of the 32 'popular' floats we can now materialze them without a load.
>> E.g.
>> `float f = f1 * 0.5 + f2 * 2.0;`
>> Only require 2 loads instead of 4: as '0.5' and '2.0' is such popular float values.
>> 
>> As Java is often memory bound we should also investigate doing lui+ssli+fmv for float/doubles instead of a load when materializing.
>> 
>> Note the _fli_s/_fli_d will be proper merged on the 8347794: RISC-V: Add Zfhmin - Float cleanup.
>> 
>> Passes:
>> ./test/jdk/java/lang/Math
>> ./test/hotspot/jtreg/compiler/floatingpoint/
>> ./test/jdk/java/util/Formatter/
>> ./test/jdk/java/lang/Float/
>> ./test/jdk/java/lang/Double/
>> ./test/hotspot/jtreg/compiler/c2/FloatingPointFoldingTest.java
>> ./test/hotspot/jtreg/compiler/eliminateAutobox/
>> ./test/hotspot/jtreg/vmTestbase/jit/
>> 
>> Running tier1
>> 
>> Thanks!
>
> src/hotspot/cpu/riscv/assembler_riscv.hpp line 335:
> 
>> 333: private:
>> 334: 
>> 335:   bool static zfa_zli_lookup_double(uint64_t value, int* Rs) {
> 
> Suggestion:
> 
>   static bool zfa_zli_lookup_double(uint64_t value, int* Rs) {

Yes, thanks!

> src/hotspot/cpu/riscv/assembler_riscv.hpp line 376:
> 
>> 374: 
>> 375: 
>> 376:   bool static zfa_zli_lookup_float(uint32_t value, int* Rs = nullptr) {
> 
> Suggestion:
> 
>   static bool zfa_zli_lookup_float(uint32_t value, int* Rs = nullptr) {

Yes, thanks!

> src/hotspot/cpu/riscv/assembler_riscv.hpp line 431:
> 
>> 429:     }
>> 430:     uint64_t d_bits = (uint64_t)julong_cast(d);
>> 431:     return zfa_zli_lookup_double(d_bits, Rs);
> 
> Would it better to inline the switch here directly? Is it used anywhere else?

I think that would make the code messier.
Now this method is very easy to read, check ZFA on and see if we have the bit pattern in lookup 'table', no?

> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 2634:
> 
>> 2632:   }
>> 2633:   int Rs = -1;
>> 2634:   can_zfa_zli_float(imm, &Rs);
> 
> should that be a `guarantee(can_zfa_zli_float(imm, &Rs));`?

Same, my bad. (guarantee in zli_s on that Rs is uimm5.)

As we already an guarantee, maybe assert(can_zfa..) ?

> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 2645:
> 
>> 2643:   }
>> 2644:   int Rs = -1;
>> 2645:   can_zfa_zli_double(imm, &Rs);
> 
> should that be a `guarantee(can_zfa_zli_double(imm, &Rs));`?

Sorry my bad for not adding a comment.
I'm using the guarantee in zli_d on that Rs is uimm5.

As we already an guarantee, maybe assert(can_zfa..) ?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23171#discussion_r1921023289
PR Review Comment: https://git.openjdk.org/jdk/pull/23171#discussion_r1921023243
PR Review Comment: https://git.openjdk.org/jdk/pull/23171#discussion_r1921023199
PR Review Comment: https://git.openjdk.org/jdk/pull/23171#discussion_r1921022978
PR Review Comment: https://git.openjdk.org/jdk/pull/23171#discussion_r1921022923