[PATCH V2] RISC-V: Add the missed half floating-point mode patterns of local_pic_load/store when only use zfhmin or zhinxmin

[PATCH V2] RISC-V: Add the missed half floating-point mode patterns of local_pic_load/store when only use zfhmin or zhinxmin

[Lehua Ding]

Hi,

There is a new failed RISC-V testcase(testsuite/gcc.target/riscv/rvv/autovec/vls/const-4.c)
on the current trunk branch when use medany as default cmodel.
The reason is the load of half floating-point imm is convert from RTL 1 to RTL
2 as the cmodel be changed from medlow to medany. This change let insn 7 be
combineed with @pred_broadcast patterns (insn 8) at combine pass. However,
insn 6 and insn 7 are combined for SF and DF mode, but not for HF mode, and
the fail combined leads to insn 7 and insn 8 be combined. The reason of the
fail combined is the local_pic_loadhf pattern doesn't exist when only enable
zfhmin(implied by zvfh).

Therefore, when only zfhmin but not zfh is enabled, the define_insn of
*local_pic_load<ANYF:mode> must also be able to produce the pattern for
*load_pic_loadhf pattern, since the zfhmin extension also includes a
half floating-point load/store instructions. So, I added an ANFLSF Iterator
and applied it to local_pic_load/store define_insns. I have checked other ANYF
usage scenarios and feel that this is the only place that needs to be corrected.
I may have missed something, please correct. Thanks.

RTL 1:

(insn 6 3 7 2 (set (reg:DI 137)
        (high:DI (symbol_ref/u:DI ("*.LC0") [flags 0x82]))) "/work/home/lding/open-source/riscv-gnu-toolchain-push/gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/vls/const-4.c":7:1 discrim 3 179 {*movdi_64bit}
     (nil))
(insn 7 6 8 2 (set (reg:HF 136)
        (mem/u/c:HF (lo_sum:DI (reg:DI 137)
                (symbol_ref/u:DI ("*.LC0") [flags 0x82])) [0  S2 A16])) "/work/home/lding/open-source/riscv-gnu-toolchain-push/gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/vls/const-4.c":7:1 discrim 3 126 {*movhf_hardfloat}
     (expr_list:REG_EQUAL (const_double:HF 8.8828125e+0 [0x0.8e2p+4])
        (nil)))

RTL 2:

(insn 6 3 7 2 (set (reg/f:DI 137)
        (symbol_ref/u:DI ("*.LC0") [flags 0x82])) "/work/home/lding/open-source/riscv-gnu-toolchain-push/gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/vls/const-4.c":7:1 discrim 3 179 {*movdi_64bit}
     (nil))
(insn 7 6 8 2 (set (reg:HF 136)
        (mem/u/c:HF (reg/f:DI 137) [0  S2 A16])) "/work/home/lding/open-source/riscv-gnu-toolchain-push/gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/vls/const-4.c":7:1 discrim 3 126 {*movhf_hardfloat}
     (expr_list:REG_EQUAL (const_double:HF 8.8828125e+0 [0x0.8e2p+4])
        (nil)))
(insn 8 7 9 2 (set (reg:V2HF 135)
        (if_then_else:V2HF (unspec:V2BI [
                    (const_vector:V2BI [
                            (const_int 1 [0x1]) repeated x2
                        ])
                    (const_int 2 [0x2]) repeated x3
                    (const_int 0 [0])
                    (reg:SI 66 vl)
                    (reg:SI 67 vtype)
                ] UNSPEC_VPREDICATE)
            (vec_duplicate:V2HF (reg:HF 136))
            (unspec:V2HF [
                    (reg:SI 0 zero)
                ] UNSPEC_VUNDEF))) "/work/home/lding/open-source/riscv-gnu-toolchain-push/gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/vls/const-4.c":6:1 discrim 3 1389 {*pred_broadcastv2hf}
     (nil))

Best,
Lehua

gcc/ChangeLog:

	* config/riscv/iterators.md (TARGET_HARD_FLOAT || TARGET_ZFINX): New.
	* config/riscv/pic.md (*local_pic_load<ANYF:mode>): Change ANYF.
	(*local_pic_load<ANYLSF:mode>): To ANYLSF.
	(*local_pic_load_32d<ANYF:mode>): Ditto.
	(*local_pic_load_32d<ANYLSF:mode>): Ditto.
	(*local_pic_store<ANYF:mode>): Ditto.
	(*local_pic_store<ANYLSF:mode>): Ditto.
	(*local_pic_store_32d<ANYF:mode>): Ditto.
	(*local_pic_store_32d<ANYLSF:mode>): Ditto.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/_Float16-zfhmin-4.c: New test.
	* gcc.target/riscv/_Float16-zhinxmin-4.c: New test.

---
 gcc/config/riscv/iterators.md                 | 5 +++
 gcc/config/riscv/pic.md                       | 34 +++++++++----------
 .../gcc.target/riscv/_Float16-zfhmin-4.c      | 11 ++++++
 .../gcc.target/riscv/_Float16-zhinxmin-4.c    | 11 ++++++
 4 files changed, 44 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/_Float16-zfhmin-4.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/_Float16-zhinxmin-4.c

diff --git a/gcc/config/riscv/iterators.md b/gcc/config/riscv/iterators.md
index d374a10810c..500bbc39a6b 100644
--- a/gcc/config/riscv/iterators.md
+++ b/gcc/config/riscv/iterators.md
@@ -67,6 +67,11 @@
 			    (DF "TARGET_DOUBLE_FLOAT || TARGET_ZDINX")
 			    (HF "TARGET_ZFH || TARGET_ZHINX")])
 
+;; Iterator for hardware-supported load/store floating-point modes.
+(define_mode_iterator ANYLSF [(SF "TARGET_HARD_FLOAT || TARGET_ZFINX")
+			      (DF "TARGET_DOUBLE_FLOAT || TARGET_ZDINX")
+			      (HF "TARGET_ZFHMIN || TARGET_ZHINXMIN")])
+
 ;; Iterator for floating-point modes that can be loaded into X registers.
 (define_mode_iterator SOFTF [SF (DF "TARGET_64BIT") (HF "TARGET_ZFHMIN")])
 
diff --git a/gcc/config/riscv/pic.md b/gcc/config/riscv/pic.md
index 9507850455a..da636e31619 100644
--- a/gcc/config/riscv/pic.md
+++ b/gcc/config/riscv/pic.md
@@ -43,17 +43,17 @@
   "<SUBX:load>u\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
-;; We can support ANYF loads into X register if there is no double support
+;; We can support ANYLSF loads into X register if there is no double support
 ;; or if the target is 64-bit.
 
-(define_insn "*local_pic_load<ANYF:mode>"
-  [(set (match_operand:ANYF 0 "register_operand" "=f,*r")
-	(mem:ANYF (match_operand 1 "absolute_symbolic_operand" "")))
+(define_insn "*local_pic_load<ANYLSF:mode>"
+  [(set (match_operand:ANYLSF 0 "register_operand" "=f,*r")
+	(mem:ANYLSF (match_operand 1 "absolute_symbolic_operand" "")))
    (clobber (match_scratch:P 2 "=r,X"))]
   "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])
    && (!TARGET_DOUBLE_FLOAT || TARGET_64BIT)"
   "@
-   <ANYF:load>\t%0,%1,%2
+   <ANYLSF:load>\t%0,%1,%2
    <softload>\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
@@ -61,13 +61,13 @@
 ;; supported.  ld is not valid in that case.  Punt for now.  Maybe add a split
 ;; for this later.
 
-(define_insn "*local_pic_load_32d<ANYF:mode>"
-  [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(mem:ANYF (match_operand 1 "absolute_symbolic_operand" "")))
+(define_insn "*local_pic_load_32d<ANYLSF:mode>"
+  [(set (match_operand:ANYLSF 0 "register_operand" "=f")
+	(mem:ANYLSF (match_operand 1 "absolute_symbolic_operand" "")))
    (clobber (match_scratch:P 2 "=r"))]
   "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])
    && (TARGET_DOUBLE_FLOAT && !TARGET_64BIT)"
-  "<ANYF:load>\t%0,%1,%2"
+  "<ANYLSF:load>\t%0,%1,%2"
   [(set (attr "length") (const_int 8))])
 
 (define_insn "*local_pic_load_sf<mode>"
@@ -88,14 +88,14 @@
   "<ANYI:store>\t%z1,%0,%2"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_store<ANYF:mode>"
-  [(set (mem:ANYF (match_operand 0 "absolute_symbolic_operand" ""))
-	(match_operand:ANYF 1 "register_operand" "f,*r"))
+(define_insn "*local_pic_store<ANYLSF:mode>"
+  [(set (mem:ANYLSF (match_operand 0 "absolute_symbolic_operand" ""))
+	(match_operand:ANYLSF 1 "register_operand" "f,*r"))
    (clobber (match_scratch:P 2 "=r,&r"))]
   "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[0])
    && (!TARGET_DOUBLE_FLOAT || TARGET_64BIT)"
   "@
-   <ANYF:store>\t%1,%0,%2
+   <ANYLSF:store>\t%1,%0,%2
    <softstore>\t%1,%0,%2"
   [(set (attr "length") (const_int 8))])
 
@@ -103,13 +103,13 @@
 ;; supported.  sd is not valid in that case.  Punt for now.  Maybe add a split
 ;; for this later.
 
-(define_insn "*local_pic_store_32d<ANYF:mode>"
-  [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(mem:ANYF (match_operand 1 "absolute_symbolic_operand" "")))
+(define_insn "*local_pic_store_32d<ANYLSF:mode>"
+  [(set (match_operand:ANYLSF 0 "register_operand" "=f")
+	(mem:ANYLSF (match_operand 1 "absolute_symbolic_operand" "")))
    (clobber (match_scratch:P 2 "=r"))]
   "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])
    && (TARGET_DOUBLE_FLOAT && !TARGET_64BIT)"
-  "<ANYF:store>\t%1,%0,%2"
+  "<ANYLSF:store>\t%1,%0,%2"
   [(set (attr "length") (const_int 8))])
 
 (define_insn "*local_pic_store_sf<SOFTF:mode>"
diff --git a/gcc/testsuite/gcc.target/riscv/_Float16-zfhmin-4.c b/gcc/testsuite/gcc.target/riscv/_Float16-zfhmin-4.c
new file mode 100644
index 00000000000..42a238a0380
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/_Float16-zfhmin-4.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_zfhmin -mabi=lp64d -O3 -mcmodel=medany" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/* Make sure zfhmin behaves the same way as zfh. */
+/*
+** foo: { target { no-opts "-flto" } }
+**   flh\tfa0,\.LC0,[a-z0-9]+
+**   ...
+*/
+_Float16 foo() { return 0.8974; }
diff --git a/gcc/testsuite/gcc.target/riscv/_Float16-zhinxmin-4.c b/gcc/testsuite/gcc.target/riscv/_Float16-zhinxmin-4.c
new file mode 100644
index 00000000000..4ac49228898
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/_Float16-zhinxmin-4.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64if_zfhmin -mabi=lp64 -O3 -mcmodel=medany" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/* Make sure zfhmin behaves the same way as zfh. */
+/*
+** foo: { target { no-opts "-flto" } }
+**   lh\ta0,\.LC0
+**   ...
+*/
+_Float16 foo() { return 0.8974; }
-- 
2.36.3

Re: [PATCH V2] RISC-V: Add the missed half floating-point mode patterns of local_pic_load/store when only use zfhmin or zhinxmin

[Robin Dapp]

Indeed all ANYLSF patterns have TARGET_HARD_FLOAT (==f extension) which
is incompatible with ZHINX or ZHINXMIN anyway.  That should really be fixed
separately or at least clarified, maybe I'm missing something.

Still we can go forward with the patch itself as it improves things
independently, so LGTM.

Regards
 Robin

Re: [PATCH V2] RISC-V: Add the missed half floating-point mode patterns of local_pic_load/store when only use zfhmin or zhinxmin

[Palmer Dabbelt]

On Thu, 17 Aug 2023 10:03:04 PDT (-0700), rdapp.gcc@gmail.com wrote:
> Indeed all ANYLSF patterns have TARGET_HARD_FLOAT (==f extension) which
> is incompatible with ZHINX or ZHINXMIN anyway.  That should really be fixed
> separately or at least clarified, maybe I'm missing something.

We've also got the broader issue where these PIC patterns are likely not 
the way to go long term, they're just papering around some other issues 
(and are likely why we flip the implicit-relocs behavior implicitly).  
We should probably fix that at some point, but I don't see any reason to 
block a fix on a cleanup.

That said, given that folks are poking around in here it's probably 
worth putting together test cases for the other patterns in there.

> Still we can go forward with the patch itself as it improves things
> independently, so LGTM.

Ya, IMO it's fine to add these given they fix the issue.

> Regards
>  Robin

Re: [PATCH V2] RISC-V: Add the missed half floating-point mode patterns of local_pic_load/store when only use zfhmin or zhinxmin

[Lehua Ding]

[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]

Committed, thanks Robin and Palmer.




------------------ Original ------------------
From:                                                                                                                        "Palmer Dabbelt"                                                                                    <palmer@rivosinc.com&gt;;
Date:&nbsp;Fri, Aug 18, 2023 01:51 AM
To:&nbsp;"rdapp.gcc"<rdapp.gcc@gmail.com&gt;;
Cc:&nbsp;"lehua.ding"<lehua.ding@rivai.ai&gt;;"gcc-patches"<gcc-patches@gcc.gnu.org&gt;;"rdapp.gcc"<rdapp.gcc@gmail.com&gt;;"juzhe.zhong"<juzhe.zhong@rivai.ai&gt;;"Kito Cheng"<kito.cheng@gmail.com&gt;;"jeffreyalaw"<jeffreyalaw@gmail.com&gt;;
Subject:&nbsp;Re: [PATCH V2] RISC-V: Add the missed half floating-point mode patterns of local_pic_load/store when only use zfhmin or zhinxmin



On Thu, 17 Aug 2023 10:03:04 PDT (-0700), rdapp.gcc@gmail.com wrote:
&gt; Indeed all ANYLSF patterns have TARGET_HARD_FLOAT (==f extension) which
&gt; is incompatible with ZHINX or ZHINXMIN anyway.&nbsp; That should really be fixed
&gt; separately or at least clarified, maybe I'm missing something.

We've also got the broader issue where these PIC patterns are likely not 
the way to go long term, they're just papering around some other issues 
(and are likely why we flip the implicit-relocs behavior implicitly).&nbsp; 
We should probably fix that at some point, but I don't see any reason to 
block a fix on a cleanup.

That said, given that folks are poking around in here it's probably 
worth putting together test cases for the other patterns in there.

&gt; Still we can go forward with the patch itself as it improves things
&gt; independently, so LGTM.

Ya, IMO it's fine to add these given they fix the issue.

&gt; Regards
&gt;&nbsp; Robin