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

Message ID 20230817124354.3137796-1-lehua.ding@rivai.ai
State Accepted
Headers
Series [V2] RISC-V: Add the missed half floating-point mode patterns of local_pic_load/store when only use zfhmin or zhinxmin |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Lehua Ding Aug. 17, 2023, 12:43 p.m. UTC
  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
  

Comments

Robin Dapp Aug. 17, 2023, 5:03 p.m. UTC | #1
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
  
Palmer Dabbelt Aug. 17, 2023, 5:51 p.m. UTC | #2
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
  
Lehua Ding Aug. 18, 2023, 2:40 a.m. UTC | #3
Committed, thanks Robin and Palmer.




------------------&nbsp;Original&nbsp;------------------
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
  

Patch

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; }