public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Vineet Gupta <vineetg@rivosinc.com>
To: gcc-patches@gcc.gnu.org
Cc: kito.cheng@gmail.com, Jeff Law <jeffreyalaw@gmail.com>,
	Palmer Dabbelt <palmer@rivosinc.com>,
	gnu-toolchain@rivosinc.com, Vineet Gupta <vineetg@rivosinc.com>
Subject: [PATCH] RISC-V: zicond: remove bogus opt2 pattern
Date: Wed, 30 Aug 2023 14:57:08 -0700	[thread overview]
Message-ID: <20230830215708.369610-1-vineetg@rivosinc.com> (raw)

This was tripping up gcc.c-torture/execute/pr60003.c at -O1 since the
pattern semantics can't be expressed by zicond instructions.

This involves test code snippet:

      if (a == 0)
	return 0;
      else
	return x;
    }

which is equivalent to:  "x = (a != 0) ? x : a"

and matches define_insn "*czero.nez.<GPR:mode><X:mode>.opt2"

| (insn 41 20 38 3 (set (reg/v:DI 136 [ x ])
|        (if_then_else:DI (ne (reg/v:DI 134 [ a ])
|                (const_int 0 [0]))
|            (reg/v:DI 136 [ x ])
|            (reg/v:DI 134 [ a ]))) {*czero.nez.didi.opt2}

The corresponding asm pattern generates
    czero.nez x, x, a   ; %0, %2, %1
implying
    "x = (a != 0) ? 0 : a"

which is not what the pattern semantics are.

Essentially "(a != 0) ? x : a" cannot be expressed with CZERO.nez

As a side note, while correctness prevails, this test still gets a
czero in the end, albeit a different one.

if-convert generates two if_then_else

| (insn 43 20 44 3 (set (reg:DI 143)
|        (reg/v:DI 136 [ x ])) "pr60003.c":36:9 179 {*movdi_64bit}
| (insn 44 43 46 3 (set (reg:DI 142)
|        (reg/v:DI 134 [ a ])) "pr60003.c":36:9 179 {*movdi_64bit}
|
| (insn 46 44 47 3 (set (reg:DI 145)
|        (if_then_else:DI (ne:DI (reg/v:DI 134 [ a ])
|                (const_int 0 [0]))
|            (const_int 0 [0])
|            (reg:DI 142))) "pr60003.c":36:9 14532 {*czero.nez.didi}
|
| (insn 47 46 48 3 (set (reg:DI 144)
|        (if_then_else:DI (eq:DI (reg/v:DI 134 [ a ])
|                (const_int 0 [0]))
|            (const_int 0 [0])
|            (reg:DI 143))) "pr60003.c":36:9 14531 {*czero.eqz.didi}

and combine is able to fuse them together

| (insn 38 48 39 3 (set (reg/i:DI 10 a0)
|        (if_then_else:DI (eq:DI (reg/v:DI 134 [ a ])
|                (const_int 0 [0]))
|            (const_int 0 [0])
|            (reg:DI 143))) "pr60003.c":40:1 14531 {*czero.eqz.didi}

before fix		after fix
-----------------	-----------------
li        a5,1		li        a0,1
ld        a4,8(sp)	ld        a5,8(sp)
czero.nez a0,a4,a5	czero.eqz a0,a5,a0

The issue only happens at -O1 as at higher optimization levels, the
whole conditional move gets optimized away.

gcc/ChangeLog:
	* config/riscv/zicond.md: Remove incorrect op2 pattern.

Fixes: 1d5bc3285e8a ("[committed][RISC-V] Fix 20010221-1.c with zicond")
Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
 gcc/config/riscv/zicond.md | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/gcc/config/riscv/zicond.md b/gcc/config/riscv/zicond.md
index 25f21d33487e..aa5607a9efd8 100644
--- a/gcc/config/riscv/zicond.md
+++ b/gcc/config/riscv/zicond.md
@@ -52,13 +52,3 @@
   "TARGET_ZICOND && rtx_equal_p (operands[1], operands[2])"
   "czero.eqz\t%0,%3,%1"
 )
-
-(define_insn "*czero.nez.<GPR:mode><X:mode>.opt2"
-  [(set (match_operand:GPR 0 "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")))]
-  "TARGET_ZICOND && rtx_equal_p (operands[1], operands[3])"
-  "czero.nez\t%0,%2,%1"
-)
-- 
2.34.1


             reply	other threads:[~2023-08-30 21:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-30 21:57 Vineet Gupta [this message]
2023-08-31 13:51 ` Jeff Law
2023-08-31 17:57   ` Vineet Gupta
2023-09-01 13:13     ` Jeff Law
2023-09-01 18:55       ` Vineet Gupta
2023-09-01 17:40     ` Palmer Dabbelt
2023-09-01 19:17       ` Vineet Gupta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230830215708.369610-1-vineetg@rivosinc.com \
    --to=vineetg@rivosinc.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gnu-toolchain@rivosinc.com \
    --cc=jeffreyalaw@gmail.com \
    --cc=kito.cheng@gmail.com \
    --cc=palmer@rivosinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).