[V2] RISC-V: Add the missed half floating-point mode patterns of local_pic_load/store when only use zfhmin or zhinxmin
Checks
Commit Message
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
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
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
Committed, thanks Robin and Palmer.
------------------ Original ------------------
From: "Palmer Dabbelt" <palmer@rivosinc.com>;
Date: Fri, Aug 18, 2023 01:51 AM
To: "rdapp.gcc"<rdapp.gcc@gmail.com>;
Cc: "lehua.ding"<lehua.ding@rivai.ai>;"gcc-patches"<gcc-patches@gcc.gnu.org>;"rdapp.gcc"<rdapp.gcc@gmail.com>;"juzhe.zhong"<juzhe.zhong@rivai.ai>;"Kito Cheng"<kito.cheng@gmail.com>;"jeffreyalaw"<jeffreyalaw@gmail.com>;
Subject: 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:
> 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
@@ -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")])
@@ -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>"
new file mode 100644
@@ -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; }
new file mode 100644
@@ -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; }