123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202 |
- From ac7d33ee174d69be62cda1295324ab34338f71b1 Mon Sep 17 00:00:00 2001
- From: Jeff Law <jlaw@ventanamicro.com>
- Date: Tue, 1 Aug 2023 23:12:16 -0600
- Subject: [PATCH 05/30] [committed] [RISC-V] Avoid sub-word mode comparisons
- with Zicond
- c-torture/execute/pr59014-2.c fails with the Zicond work on rv64. We
- miscompile the "foo" routine because we have eliminated a required sign
- extension.
- The key routine looks like this:
- foo (long long int x, long long int y)
- {
- if (((int) x | (int) y) != 0)
- return 6;
- return x + y;
- }
- So we kindof do the expected thing. We IOR X and Y, sign extend the result
- from 32 to 64 bits, then emit a suitable conditional branch. ie:
- > (insn 10 4 12 2 (set (reg:DI 142)
- > (ior:DI (reg/v:DI 138 [ x ])
- > (reg/v:DI 139 [ y ]))) "j.c":6:16 99 {iordi3}
- > (nil))
- > (insn 12 10 13 2 (set (reg:DI 144)
- > (sign_extend:DI (subreg:SI (reg:DI 142) 0))) "j.c":6:6 116 {extendsidi2}
- > (nil))
- > (jump_insn 13 12 14 2 (set (pc)
- > (if_then_else (ne (reg:DI 144)
- > (const_int 0 [0]))
- > (label_ref:DI 27)
- > (pc))) "j.c":6:6 243 {*branchdi}
- > (expr_list:REG_DEAD (reg:DI 144)
- > (int_list:REG_BR_PROB 233216732 (nil)))
- When we if-convert that we generate this sequence:
- > (insn 10 4 12 2 (set (reg:DI 142)
- > (ior:DI (reg/v:DI 138 [ x ])
- > (reg/v:DI 139 [ y ]))) "j.c":6:16 99 {iordi3}
- > (nil))
- > (insn 12 10 30 2 (set (reg:DI 144)
- > (sign_extend:DI (subreg:SI (reg:DI 142) 0))) "j.c":6:6 116 {extendsidi2}
- > (nil))
- > (insn 30 12 31 2 (set (reg:DI 147)
- > (const_int 6 [0x6])) "j.c":8:12 179 {*movdi_64bit}
- > (nil))
- > (insn 31 30 33 2 (set (reg:DI 146)
- > (plus:DI (reg/v:DI 138 [ x ])
- > (reg/v:DI 139 [ y ]))) "j.c":8:12 5 {adddi3}
- > (nil))
- > (insn 33 31 34 2 (set (reg:DI 149)
- > (if_then_else:DI (ne:DI (reg:DI 144)
- > (const_int 0 [0]))
- > (const_int 0 [0])
- > (reg:DI 146))) "j.c":8:12 11368 {*czero.nez.didi}
- > (nil))
- > (insn 34 33 35 2 (set (reg:DI 148)
- > (if_then_else:DI (eq:DI (reg:DI 144)
- > (const_int 0 [0]))
- > (const_int 0 [0])
- > (reg:DI 147))) "j.c":8:12 11367 {*czero.eqz.didi}
- > (nil))
- > (insn 35 34 21 2 (set (reg:DI 137 [ <retval> ])
- > (ior:DI (reg:DI 148)
- > (reg:DI 149))) "j.c":8:12 99 {iordi3}
- > (nil))
- Which looks basically OK. The sign extended subreg is a bit worrisome though.
- And sure enough when we get into combine:
- > Failed to match this instruction:
- > (parallel [
- > (set (reg:DI 149)
- > (if_then_else:DI (eq:DI (subreg:SI (reg:DI 142) 0)
- > (const_int 0 [0]))
- > (reg:DI 146)
- > (const_int 0 [0])))
- > (set (reg:DI 144)
- > (sign_extend:DI (subreg:SI (reg:DI 142) 0)))
- > ])
- > Successfully matched this instruction:
- > (set (reg:DI 144)
- > (sign_extend:DI (subreg:SI (reg:DI 142) 0)))
- > Successfully matched this instruction:
- > (set (reg:DI 149)
- > (if_then_else:DI (eq:DI (subreg:SI (reg:DI 142) 0)
- > (const_int 0 [0]))
- > (reg:DI 146)
- > (const_int 0 [0])))
- > allowing combination of insns 12 and 33
- Since we need the side effect we first try the PARALLEL with two sets.
- That, as expected, fails. Generic combine code then tries to pull apart
- the two sets as distinct insns resulting in this conditional move:
- > (insn 33 31 34 2 (set (reg:DI 149)
- > (if_then_else:DI (eq:DI (subreg:SI (reg:DI 142) 0)
- > (const_int 0 [0]))
- > (reg:DI 146)
- > (const_int 0 [0]))) "j.c":8:12 11347 {*czero.nez.disi}
- > (expr_list:REG_DEAD (reg:DI 146)
- > (nil)))
- Bzzt. We can't actually implement this RTL in the hardware. Basically
- it's asking to do 32bit comparison on rv64, ignoring the upper 32 bits
- of the input register. That's not actually how zicond works.
- The operands to the comparison need to be in DImode for rv64 and SImode
- for rv32. That's the X iterator. Note the mode of the comparison
- operands may be different than the mode of the destination. ie, we might
- have a 64bit comparison and produce a 32bit sign extended result much
- like the setcc insns support.
- This patch changes the 6 zicond patterns to use the X iterator on the
- comparison inputs and fixes the testsuite failure.
- gcc/
- * config/riscv/zicond.md: Use the X iterator instead of ANYI
- on the comparison input operands.
- ---
- gcc/config/riscv/zicond.md | 24 ++++++++++++------------
- 1 file changed, 12 insertions(+), 12 deletions(-)
- diff --git a/gcc/config/riscv/zicond.md b/gcc/config/riscv/zicond.md
- index 723a22422e1..8f24b3a1690 100644
- --- a/gcc/config/riscv/zicond.md
- +++ b/gcc/config/riscv/zicond.md
- @@ -22,9 +22,9 @@
- (define_code_attr nez [(eq "eqz") (ne "nez")])
-
- ;; Zicond
- -(define_insn "*czero.<eqz>.<GPR:mode><ANYI:mode>"
- +(define_insn "*czero.<eqz>.<GPR:mode><X:mode>"
- [(set (match_operand:GPR 0 "register_operand" "=r")
- - (if_then_else:GPR (eq_or_ne (match_operand:ANYI 1 "register_operand" "r")
- + (if_then_else:GPR (eq_or_ne (match_operand:X 1 "register_operand" "r")
- (const_int 0))
- (match_operand:GPR 2 "register_operand" "r")
- (const_int 0)))]
- @@ -32,9 +32,9 @@
- "czero.<eqz>\t%0,%2,%1"
- )
-
- -(define_insn "*czero.<nez>.<GPR:mode><ANYI:mode>"
- +(define_insn "*czero.<nez>.<GPR:mode><X:mode>"
- [(set (match_operand:GPR 0 "register_operand" "=r")
- - (if_then_else:GPR (eq_or_ne (match_operand:ANYI 1 "register_operand" "r")
- + (if_then_else:GPR (eq_or_ne (match_operand:X 1 "register_operand" "r")
- (const_int 0))
- (const_int 0)
- (match_operand:GPR 2 "register_operand" "r")))]
- @@ -43,9 +43,9 @@
- )
-
- ;; Special optimization under eq/ne in primitive semantics
- -(define_insn "*czero.eqz.<GPR:mode><ANYI:mode>.opt1"
- +(define_insn "*czero.eqz.<GPR:mode><X:mode>.opt1"
- [(set (match_operand:GPR 0 "register_operand" "=r")
- - (if_then_else:GPR (eq (match_operand:ANYI 1 "register_operand" "r")
- + (if_then_else:GPR (eq (match_operand:X 1 "register_operand" "r")
- (const_int 0))
- (match_operand:GPR 2 "register_operand" "1")
- (match_operand:GPR 3 "register_operand" "r")))]
- @@ -53,9 +53,9 @@
- "czero.eqz\t%0,%3,%1"
- )
-
- -(define_insn "*czero.eqz.<GPR:mode><ANYI:mode>.opt2"
- +(define_insn "*czero.eqz.<GPR:mode><X:mode>.opt2"
- [(set (match_operand:GPR 0 "register_operand" "=r")
- - (if_then_else:GPR (eq (match_operand:ANYI 1 "register_operand" "r")
- + (if_then_else:GPR (eq (match_operand:X 1 "register_operand" "r")
- (const_int 0))
- (match_operand:GPR 2 "register_operand" "r")
- (match_operand:GPR 3 "register_operand" "1")))]
- @@ -63,9 +63,9 @@
- "czero.nez\t%0,%2,%1"
- )
-
- -(define_insn "*czero.nez.<GPR:mode><ANYI:mode>.opt3"
- +(define_insn "*czero.nez.<GPR:mode><X:mode>.opt3"
- [(set (match_operand:GPR 0 "register_operand" "=r")
- - (if_then_else:GPR (ne (match_operand:ANYI 1 "register_operand" "r")
- + (if_then_else:GPR (ne (match_operand:X 1 "register_operand" "r")
- (const_int 0))
- (match_operand:GPR 2 "register_operand" "r")
- (match_operand:GPR 3 "register_operand" "1")))]
- @@ -73,9 +73,9 @@
- "czero.eqz\t%0,%2,%1"
- )
-
- -(define_insn "*czero.nez.<GPR:mode><ANYI:mode>.opt4"
- +(define_insn "*czero.nez.<GPR:mode><X:mode>.opt4"
- [(set (match_operand:GPR 0 "register_operand" "=r")
- - (if_then_else:GPR (ne (match_operand:ANYI 1 "register_operand" "r")
- + (if_then_else:GPR (ne (match_operand:X 1 "register_operand" "r")
- (const_int 0))
- (match_operand:GPR 2 "register_operand" "1")
- (match_operand:GPR 3 "register_operand" "r")))]
- --
- 2.25.1
|