Hi Kito: Thanks for the review and the detailed feedback! I agree that extending this to soft-float targets and moving the logic to riscv_legitimize_const_move is a much better approach. I have implemented these changes in v2.
Kito Cheng <[email protected]> 於 2025年12月18日週四 下午5:00寫道： > > Hi Meng-Tsung: > > Thanks for your patch! that's really good insight, and that's > generally it's a good direction, > > However there is some implementation detail should be adjust a little bit: > > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > > index 96519c96a2b..b4973747cfd 100644 > > --- a/gcc/config/riscv/riscv.cc > > +++ b/gcc/config/riscv/riscv.cc > > @@ -2614,6 +2614,24 @@ riscv_const_insns (rtx x, bool allow_new_pseudos) > > if (satisfies_constraint_zfli (x)) > > return 1; > > > > + /* When target support Zfinx-like extension, we can use li to > > + materialize FP constants. */ > > + if (TARGET_ZFINX) > > ^^^ I guess it could relax to !TARGET_HARD_FLOAT > since this optimization can be applied when either F and ZFINX is not present. > > e.g. -march=rv64i -mabi=lp64 Thanks for the advice, I relaxed the condition to !TARGET_HARD_FLOAT in v2. > > > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md > > index 6f8cd26e5c9..036873d0988 100644 > > --- a/gcc/config/riscv/riscv.md > > +++ b/gcc/config/riscv/riscv.md > > @@ -2481,6 +2481,34 @@ > > MAX_MACHINE_MODE, &operands[3]); > > }) > > > > +;; Zfinx-like: Use integer instructions to materialize FP constants in GPRs > > +(define_insn_and_split "*mov<mode>_zfinx_const" > > I would suggest you can implement this logic into > riscv_legitimize_const_move, so that you don't need to add a new > pattern here. > and that probably means you don't need to modify any logic in > riscv_const_insns. > I have moved the logic into riscv_legitimize_const_move. However, I found it necessary to keep the cost calculation logic inside riscv_const_insns. I checked `riscv_rtx_cost` and found that it relies on `riscv_const_insns` to decide the cost strategy. If `riscv_const_insns` returns 0 (which was the default for CONST_DOUBLE in the previous implementation), `riscv_rtx_cost` assumes it cannot be synthesized by instructions and forces the value into the constant pool. To prevent this, I updated riscv_const_insns to report the correct cost for these constants. Also, regarding predicates.md, I updated `move_operand` to return TARGET_HARD_FLOAT for CONST_DOUBLE. This is necessary because `riscv_emit_move` checks `move_operand` before calling `riscv_legitimize_const_move`. If `move_operand` returns true, the expansion logic is bypassed, causing the constant to be forced to memory. > Another way is combine this logic into movsf_softfloat, but that might > change much more place, although I suspect this way might help RA can > do better especially on rematerialization, however this could be > deferred until we found enough evidence that could improve. (or you > can try it if you are interested in that way.) > > > > + [(set (match_operand:GPRF_XLEN 0 "register_operand" "=r") > > + (match_operand 1 "immediate_operand" " i"))] > > + "TARGET_ZFINX > > + && GET_CODE (operands[1]) == CONST_DOUBLE" > > + "#" > > + "&& 1" > > + [(const_int 0)] > > + { > > + long target_vals[2] = {0}; > > + real_to_target (target_vals, CONST_DOUBLE_REAL_VALUE (operands[1]), > > + <MODE>mode); > > + unsigned HOST_WIDE_INT val; > > + > > + int order = BYTES_BIG_ENDIAN ? 1 : 0; > > + unsigned HOST_WIDE_INT lo = target_vals[order]; > > + unsigned HOST_WIDE_INT hi = target_vals[1 - order]; > > + val = ((hi << 32) | lo); > > + val &= GET_MODE_MASK (<MODE>mode); > > + > > + machine_mode int_mode = (<MODE>mode == DFmode) ? DImode : SImode; > > + rtx int_reg = gen_lowpart (int_mode, operands[0]); > > + riscv_move_integer (int_reg, int_reg, val, int_mode); > > + DONE; > > + } > > +) > > + > > ;; Pretend to have the ability to load complex const_int in order to get > > ;; better code generation around them. > > ;; But avoid constants that are special cased elsewhere. > > diff --git a/gcc/testsuite/gcc.target/riscv/zfinx-const-li.c > > b/gcc/testsuite/gcc.target/riscv/zfinx-const-li.c > > new file mode 100644 > > index 00000000000..22c84a48e42 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/riscv/zfinx-const-li.c > > @@ -0,0 +1,52 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -march=rv64i_zfinx -mabi=lp64" { target rv64 } } */ > > +/* { dg-options "-O2 -march=rv32i_zfinx -mabi=ilp32" { target rv32 } } */ > > Could you add a testcase with -march=rv64i -mabi=lp64/-march=rv32i > -mabi=ilp32? I have added a new testsuite "softfloat-const-li.c" in v2. > > +/* { dg-skip-if "" { *-*-* } { "-O0" } } */ > > Also could you drop -O2 from dg-options since you already filter -O0 here Nice catch, also fixed in v2. I will send the v2 patch shortly. Best regards, Meng-Tsung Tsai
