Message ID | 20230901195311.761131-1-vineetg@rivosinc.com |
---|---|
State | Unresolved |
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c792:0:b0:3f2:4152:657d with SMTP id b18csp1110448vqu; Fri, 1 Sep 2023 12:53:48 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGAXFM6n5ROcnNaHYVPrUcGXrMNG4CVGSiO/qYwpewj3snAqQlUM4mocivEkhNYTRVRcUkZ X-Received: by 2002:a19:5005:0:b0:500:d8d6:fbe4 with SMTP id e5-20020a195005000000b00500d8d6fbe4mr1992550lfb.49.1693598028400; Fri, 01 Sep 2023 12:53:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1693598028; cv=none; d=google.com; s=arc-20160816; b=wY1JAsV1P0kTwWngs5TVs1kYhGpJ7zE1dNVrIMFo5yVO1w5sOX7TqDhyWcjmYz5j6v SStpk9+Nf6I0+IzzV8+7J/fwP770xLl8W1bokf+ddcFRjBD31GzTpiEXU+vcgNx3bRTm B+ELzmYzyU/AVllzvoY4zd+xQWIUg6vICX2XgPqylo6i+7LugcUPmbpzLYZktcEBCiEg 8H3zxY7M+mPk+GgwdX1b3Ds4c2ZywZeCnBuDruaNVB0RKBd7cheVFzFtP0FV2i3gTtNg KXq6FY7G1hyg/HGTzq0u8CbZ7gCyUewTZozQehHZ5GByTTiXGBYhk/NmG5d4up4AM7I/ +gRg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:to:from:dkim-signature :dmarc-filter:delivered-to; bh=NOyj8diTypVqWL8R426SlJ/QiZy5NfC/KNWIaedp3c4=; fh=4XdL/aoQAs5cD/nMCr+td5MN03rWKHD/IZdQaCeIJK0=; b=Q8m9r/JBD63K52IXN9rtHg6ukq53yFlNwSoFpAWclx/LZ+SUEr2ZtmXwA26lAfQ+/E 0qGzDKmycrax58H29hRIM8cVbFm5yI96sgfytX8LK3wtHE3yqHetcQ3aFEmsq/ReFjRJ 2hvjZrjjTFtnKjjjB+s8EArcgUQREWxbDOkC4wxwkFaqzK8lJnp6BWn5xKHJdP2HE5o9 4MikL6BXA+aI7sCLF1wh/IaDWbR/uIdNsrVWRXZ3w6gA9ubkxACa2G+ZCs9AANnStKh9 +aua0MXFvJ5/BSe2zpoUnRK/CFreqd6PxiksNzKr3dv4+SrO23TPpE24wO29YylhbsyB YJmg== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@rivosinc-com.20230601.gappssmtp.com header.s=20230601 header.b=FED81VLQ; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org" Received: from server2.sourceware.org (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id n7-20020a05640206c700b0052c18a8212dsi3111615edy.533.2023.09.01.12.53.48 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Sep 2023 12:53:48 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=fail header.i=@rivosinc-com.20230601.gappssmtp.com header.s=20230601 header.b=FED81VLQ; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org" Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 089E338582A9 for <ouuuleilei@gmail.com>; Fri, 1 Sep 2023 19:53:43 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pl1-x630.google.com (mail-pl1-x630.google.com [IPv6:2607:f8b0:4864:20::630]) by sourceware.org (Postfix) with ESMTPS id 4D1C83858D1E for <gcc-patches@gcc.gnu.org>; Fri, 1 Sep 2023 19:53:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4D1C83858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com Received: by mail-pl1-x630.google.com with SMTP id d9443c01a7336-1bf078d5f33so19606955ad.3 for <gcc-patches@gcc.gnu.org>; Fri, 01 Sep 2023 12:53:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1693597994; x=1694202794; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=NOyj8diTypVqWL8R426SlJ/QiZy5NfC/KNWIaedp3c4=; b=FED81VLQ/uVEnYXCobn11mLz4VQ1eJsJIuzSL5iLVgAwjiOv+kp1SaTH4oZTuMIrg/ 84WnZ0wfXol0f2KD52YxBxYZbzP+0bIctp4Rbv05K5b/Yr1ORjnJ4hXk47GFECkv/JZc 122AG9/4mIaRNqJ+K4G9QrNTe7SUG2K8WHoowcLgjxqg5nLahoB+HnvH7KTeNucmPWqG Ppk4Z2HeNaktnO4L00ZYmP0BjiEXlEe/TMXhYEZqXaLzjU5TeeUY628hF0ZTZWbAmvcc /IVtwfQ5Wc4I3HEzRx/9wkIbz2wU1e0FTB0/iu8RyV3xyyEaSgNWthPMVGARM3i9XkHq QxOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693597994; x=1694202794; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=NOyj8diTypVqWL8R426SlJ/QiZy5NfC/KNWIaedp3c4=; b=UlT0+8IcR+yw7W77ktFucd1VLFSGAjEruQJnXpu1P73P2DSc4NkpO53vOgbuo7QRlr 7DwxpFecE6GOQZvsaL/zViAclinUmN8td/v6yuAMjOrK+mQOqypfRvVbigWYjVVWxpdt HojNl2z5Wh5r3XcQ9E8/yotCi+s3Qt3lCcS9lnfI7E5V6FtIeBof/pkVuGVoiLVFZIup ppvOtlE/vSb+Gb7i8R2P+0uu7/vs3RR48W+BuZCVWzl8uTJZrOndrHxo5/z2RYZblKMP /q2hbVmLksniKiFWfuOa2LrxZ5xGJ2ukobZ603Y3xBaeXjS1//ko3KqQ7g3xEoxHizxy MaJw== X-Gm-Message-State: AOJu0YxHMnXftsma8WMLifnA5C0XRTckZ9enLNtYFiuJ0Ful/e3h2bHf 3i9TgMcYvZPYyg6vkEhluz/KzBxGv9c5/wdGmQM= X-Received: by 2002:a17:903:22ce:b0:1bb:a4e4:54b6 with SMTP id y14-20020a17090322ce00b001bba4e454b6mr4202057plg.62.1693597993703; Fri, 01 Sep 2023 12:53:13 -0700 (PDT) Received: from vineet-framework.ba.rivosinc.com (c-98-210-197-24.hsd1.ca.comcast.net. [98.210.197.24]) by smtp.gmail.com with ESMTPSA id ix21-20020a170902f81500b001bba7aab826sm3359155plb.163.2023.09.01.12.53.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Sep 2023 12:53:13 -0700 (PDT) From: Vineet Gupta <vineetg@rivosinc.com> To: gcc-patches@gcc.gnu.org Subject: [PATCH v2] RISC-V: zicond: Fix opt2 pattern Date: Fri, 1 Sep 2023 12:53:11 -0700 Message-Id: <20230901195311.761131-1-vineetg@rivosinc.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Cc: gnu-toolchain@rivosinc.com, Vineet Gupta <vineetg@rivosinc.com>, kito.cheng@gmail.com Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1775866246554256349 X-GMAIL-MSGID: 1775866246554256349 |
Series |
[v2] RISC-V: zicond: Fix opt2 pattern
|
|
Checks
Context | Check | Description |
---|---|---|
snail/gcc-patch-check | warning | Git am fail log |
Commit Message
Vineet Gupta
Sept. 1, 2023, 7:53 p.m. UTC
This was tripping up gcc.c-torture/execute/pr60003.c at -O1 since in
failing case, pattern's asm czero.nez gets both rs2 and rs1 as non zero.
We start with the following src code snippet:
if (a == 0)
return 0;
else
return x;
}
which is equivalent to: "x = (a != 0) ? x : a" where x is NOT 0.
^^^^^^^^^^^^^^^^
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
which implies
"x = (a != 0) ? 0 : a"
clearly not what the pattern wants to do.
Essentially "(a != 0) ? x : a" cannot be expressed with CZERO.nez if X
is not guaranteed to be 0.
However this can be fixed with a small tweak
"x = (a != 0) ? x : a"
is same as
"x = (a == 0) ? a : x" since middle operand is 0 when a == 0.
which can be expressed with CZERO.eqz
before fix after fix
----------------- -----------------
li a5,1 li a5,1
ld a4,8(sp) ld a4,8(sp) # a4 is runtime non zero
czero.nez a0,a4,a5 # a0=0 NOK czero.eqz a0,a4,a5 # a0=a4!=0 OK
The issue only happens at -O1 as at higher optimization levels, the
whole conditional move gets optimized away.
This fixes 4 testsuite failues in a zicond build:
FAIL: gcc.c-torture/execute/pr60003.c -O1 execution test
FAIL: gcc.dg/setjmp-3.c execution test
FAIL: gcc.dg/torture/stackalign/setjmp-3.c -O1 execution test
FAIL: gcc.dg/torture/stackalign/setjmp-3.c -O1 -fpic execution test
gcc/ChangeLog:
* config/riscv/zicond.md: Fix op2 pattern.
Fixes: 1d5bc3285e8a ("[committed][RISC-V] Fix 20010221-1.c with zicond")
Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
changes since v1
- instead of discarding opt2 pattern, fix the asm
---
gcc/config/riscv/zicond.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 9/1/23 13:53, Vineet Gupta wrote: > This was tripping up gcc.c-torture/execute/pr60003.c at -O1 since in > failing case, pattern's asm czero.nez gets both rs2 and rs1 as non zero. > > We start with the following src code snippet: > > if (a == 0) > return 0; > else > return x; > } > > which is equivalent to: "x = (a != 0) ? x : a" where x is NOT 0. > ^^^^^^^^^^^^^^^^ > > 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 > > which implies > "x = (a != 0) ? 0 : a" > > clearly not what the pattern wants to do. > > Essentially "(a != 0) ? x : a" cannot be expressed with CZERO.nez if X > is not guaranteed to be 0. > > However this can be fixed with a small tweak > > "x = (a != 0) ? x : a" > > is same as > > "x = (a == 0) ? a : x" since middle operand is 0 when a == 0. > > which can be expressed with CZERO.eqz > > before fix after fix > ----------------- ----------------- > li a5,1 li a5,1 > ld a4,8(sp) ld a4,8(sp) # a4 is runtime non zero > czero.nez a0,a4,a5 # a0=0 NOK czero.eqz a0,a4,a5 # a0=a4!=0 OK > > The issue only happens at -O1 as at higher optimization levels, the > whole conditional move gets optimized away. > > This fixes 4 testsuite failues in a zicond build: > > FAIL: gcc.c-torture/execute/pr60003.c -O1 execution test > FAIL: gcc.dg/setjmp-3.c execution test > FAIL: gcc.dg/torture/stackalign/setjmp-3.c -O1 execution test > FAIL: gcc.dg/torture/stackalign/setjmp-3.c -O1 -fpic execution test > > gcc/ChangeLog: > * config/riscv/zicond.md: Fix op2 pattern. > > Fixes: 1d5bc3285e8a ("[committed][RISC-V] Fix 20010221-1.c with zicond") > Signed-off-by: Vineet Gupta <vineetg@rivosinc.com> OK. Still not sure why I didn't trip over it in my own testing (execute.exp runs pr60003.c), but regardless, glad to have it fixed. jeff
Sorry, I want to directly reply to Jeff but I couldn't because I haven't subscribed to gcc-patches and Jeff's recent reply hasn't archived yet. Bug confirmed for me. I tried the full test with following configuration (I found another bug [ICE] as I submitted a quick fix while testing this and requires following patch set to be applied; will make a PATCH v2 though): <https://gcc.gnu.org/pipermail/gcc-patches/2023-September/629175.html> Possibly, ICE, simulator configuration and/or dirty build tree might be the reason Jeff couldn't reproduce the bug. # ZiCond enabled # Remove "_zicond" to disable ZiCond. # ${SYSROOT} points to the prebuilt sysroot with # glibc + libgcc with -march=rv64imafdc -mabi=lp64d ${GCC_SRCDIR}/configure \ --target=riscv64-unknown-linux-gnu \ --prefix=${PREFIX} \ --with-sysroot=${SYSROOT} \ --with-system-zlib \ --disable-shared \ --enable-tls \ --enable-languages=c,c++ \ --disable-libmudflap \ --disable-libssp \ --disable-libquadmath \ --disable-libsanitizer \ --disable-nls \ --disable-bootstrap \ --disable-multilib \ --with-tune=rocket \ --with-arch=rv64imafdc_zicond \ --with-abi=lp64d \ --with-isa-spec=20191213 Then I ran "make; make check RUNTESTFLAGS='--target_board=riscv-sim'". Note that I configured DejaGnu (riscv-sim.exp) to execute tests with: "qemu-riscv64 -L ${SYSROOT} -cpu rv64,g=on,x-zicond=on" (QEMU 8.1.0 Linux user emulation). Warning: abort() on QEMU with Linux user emulation causes QEMU to abort, too (possibly making many coredumps). The diff of test failures are as follows. -: Occurs only when ZiCond is disabled +: Occurs only when ZiCond is enabled -FAIL: 30_threads/async/async.cc execution test +FAIL: gcc.c-torture/execute/pr60003.c -O1 execution test +FAIL: gcc.dg/setjmp-3.c execution test +FAIL: gcc.dg/torture/stackalign/setjmp-3.c -O1 execution test +FAIL: gcc.dg/torture/stackalign/setjmp-3.c -O1 -fpic execution test I'm not sure why 30_threads/async/async.cc succeeds after enabling the 'Zicond' extension but I am sure that setjmp-3.c failures are caused by this very bug. Smaller example (not involving setjmp / longjmp) to reproduce this bug in my environment is as follows (you *don't* have to apply my patch above, make all-gcc && make install-gcc overwriting existing RV64 GCC prefix will work): > #include <stdio.h> > > __attribute__((noinline, noclone)) > void sample(long* a) > { > *a = 1; > } > > __attribute__((noinline, noclone)) > long foo(long x) > { > long a = 0; > sample(&a); // a is overwritten to 1. > if (a == 0) > return 0; > else > return x; // should be always taken > } > > int main(int argc, char** argv) > { > printf("%ld\n", foo(5)); // should print 5 > return 0; > } Note that we have to make sure that variables are not easily inferred by another optimization pass (that's why I needed two functions). > riscv64-unknown-linux-gnu-gcc -march=rv64gc_zicond -O1 -static a.c > qemu-riscv64 -cpu rv64,g=on,x-zicond=on ./a.out printed 0, not 5 as I expected. I support Vineet's patch set (v2). Thanks, Tsukasa
On 9/4/23 20:19, Tsukasa OI wrote: > > -FAIL: 30_threads/async/async.cc execution test > +FAIL: gcc.c-torture/execute/pr60003.c -O1 execution test > +FAIL: gcc.dg/setjmp-3.c execution test > +FAIL: gcc.dg/torture/stackalign/setjmp-3.c -O1 execution test > +FAIL: gcc.dg/torture/stackalign/setjmp-3.c -O1 -fpic execution test FWIW, I've got async.cc marked here as flakey when run under QEMU. That's also consistent with what I found at a prior employer when working on a private GCC port. Jeff
diff --git a/gcc/config/riscv/zicond.md b/gcc/config/riscv/zicond.md index 4619220ef8ac..1721e1011ea8 100644 --- a/gcc/config/riscv/zicond.md +++ b/gcc/config/riscv/zicond.md @@ -60,7 +60,7 @@ (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" + "czero.eqz\t%0,%2,%1" ) ;; Combine creates this form in some cases (particularly the coremark