From patchwork Wed Aug 2 05:41:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 129613 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f41:0:b0:3e4:2afc:c1 with SMTP id v1csp226991vqx; Tue, 1 Aug 2023 22:42:28 -0700 (PDT) X-Google-Smtp-Source: APBJJlFJnsvtXuM7fD1uUZaNrNW4gb2DuUUraebOrLnSEppQf7TMeelkgkyV/IvD3ZdqhXs11dBp X-Received: by 2002:a17:906:108:b0:99b:d599:5085 with SMTP id 8-20020a170906010800b0099bd5995085mr4139854eje.64.1690954948364; Tue, 01 Aug 2023 22:42:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690954948; cv=none; d=google.com; s=arc-20160816; b=xlFlvxQI+S5HXBa7h5HKAHXPINcg8fxkvsJ/hFeCWkd56Gby9kqvUiDdKZORwiUmFP /NZhWXb/Dw3pkkyyyLefy24lTO9DkLBkqyPs8thoBUnXdcD249AGmJmEo0IaGF0FzJcx /9hqgXBnTTDw3m48fDXyRz6W0LukggGrE/YL8k6rRQ+GN6uGQDJ0UCWdJgRwT7JNdAUE 6cuu+61tU4qus9q6ilMCyc8W35s2ABA5/yb/dJviy5VG5azBhELj7eJlN6yVB74c4DQH sD18chy+Neuz7V+a7gbTfx/hF4SLhBmFiVuyoBEYoBbLBpd7sKvul+h72c7Ct+D380pp Z7EQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:cc:to:subject:from :content-language:user-agent:mime-version:date:message-id :dkim-signature:dmarc-filter:delivered-to; bh=VLZMEvWJ4I74KVRfxRNxt78elILiMExwpimsSp/OBJ0=; fh=JjFzwILdutYfh4o7mSsDQzSmkgMIjQYf5s8fipe3UL4=; b=Bno4gTo8DD0Ld/097lLMyVF4QRzFuBOtfhRopYdXFq66lDbtpuVOx2+WGmGlQMVWT7 fqZVhRqIl76jWyjGnOl8uC6XyI1W6sLVshChnCjpBNZlw8kX0xUIqNURWukWpFu1MOQJ enPDag+hwbqOY4Vj895Up3TJx/TK/cSOcNir2XFUwUYnE4CyA9g9rgHIxQkZQK5tM6nx olp44NIY0yppOBMJMcfl2TzxXHOzlBvGbWEP5Dcu8/WsdfJmB8oICruKOeJnrAK7E54t dVLTgYBQVrCTTnHj/wQlJeMCNS2qAaSMsgnH28iRIi7CcI5dMCxMHizPSxNv1T+ltHwx Vymw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ventanamicro.com header.s=google header.b=YKw6VkM+; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org" Received: from server2.sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id b22-20020a170906491600b0098804fbcff0si9367514ejq.709.2023.08.01.22.42.27 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Aug 2023 22:42:28 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@ventanamicro.com header.s=google header.b=YKw6VkM+; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c 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 E7F663857C40 for ; Wed, 2 Aug 2023 05:42:17 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pl1-x636.google.com (mail-pl1-x636.google.com [IPv6:2607:f8b0:4864:20::636]) by sourceware.org (Postfix) with ESMTPS id 1D9CE3858CDA for ; Wed, 2 Aug 2023 05:41:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1D9CE3858CDA Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=ventanamicro.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ventanamicro.com Received: by mail-pl1-x636.google.com with SMTP id d9443c01a7336-1bbf0f36ce4so29818585ad.0 for ; Tue, 01 Aug 2023 22:41:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1690954910; x=1691559710; h=cc:to:subject:from:content-language:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=VLZMEvWJ4I74KVRfxRNxt78elILiMExwpimsSp/OBJ0=; b=YKw6VkM+E8KyVmvpmsOLF1vbrk3Qnprq0V+KvbbcbZk/JHDoc7PdRBYtPEDDKduOPh vmiNfWThTpfdpxfiwkTbchqUTfyLMBituh+5G7vs3buQIxskzs8zcLe9OdgLBYUguq+v ZNJLB72huoxil4l6OQkUp/eV177BHmmzwJHxEEEZJUFT50iB+V2p3YNfBQ43zP9BxT7m 6SsQ+OF0HNV70cGLlxc1wvosLJG4I5JGveHvsqoiGptuXfXxa8oY2LyUrIXbhFUPX7U2 W/OBPA/NqmCs77IA1WZkwLoaH+/E+UbDtVhfEcvw93trCkIgLkrpb7/1rYxc8la5O3LF fxyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690954910; x=1691559710; h=cc:to:subject:from:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=VLZMEvWJ4I74KVRfxRNxt78elILiMExwpimsSp/OBJ0=; b=GOcOZK0P0vqjuCZiJPSn9DWB5IbXl4Zb+hyW8MZyo3+p4Ur4+K87WjbbOiIHA//TMD S4ptvIs3Yz43Uj9mjHp96S6CnFCtehcblJWWAppppFz0AJDmXPSTpL6diTz2MXoR9AC2 hM6rFw0SU/heXmwmZjjr9bNJts8f583LyI45nj3LWTIqjXLSHrjePbWtvoPd7DZHtbPn d/E680hL2OTuvOEmPo5ScIMbpfSi7zVBEMOBPIAUeEXeS1DHwWNpiwOfVfWkwjhZQwVi TmoL+zKf7jkx2+nvxGOpL+tlkl4HI6FyShKKP2X7WCsptBEmkDyQEKTDi9rkEuWeLnvn NyGw== X-Gm-Message-State: ABy/qLb2JdrZa6Mt8gDPSKpFboBuPuKADa36YSEkXm2l+/waa0jjhf8T jcgYPeWxA21hFreRWs8Pf9mOCrkGWopVtcns8ydCKw== X-Received: by 2002:a17:902:ce8b:b0:1bb:e74b:39ff with SMTP id f11-20020a170902ce8b00b001bbe74b39ffmr16739907plg.0.1690954910547; Tue, 01 Aug 2023 22:41:50 -0700 (PDT) Received: from [172.31.1.103] ([172.56.168.109]) by smtp.gmail.com with ESMTPSA id ji20-20020a170903325400b001ac591b0500sm11414596plb.134.2023.08.01.22.41.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Aug 2023 22:41:49 -0700 (PDT) Message-ID: Date: Tue, 1 Aug 2023 23:41:47 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Content-Language: en-US From: Jeff Law Subject: [committed] [RISC-V] Avoid sub-word mode comparisons with Zicond To: "gcc-patches@gcc.gnu.org" Cc: Xiao Zeng X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773094775764896724 X-GMAIL-MSGID: 1773094775764896724 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 at expansion time. We IOR X and Y, sign extend the result from 32 to 64 bits (note how the values in the source are casted from long long to ints), 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 [ ]) > (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. That at least makes the patterns correct and fixes this particular testcase. There's a few other lurking problems that I'll address in additional patches. Committed to the trunk, Jeff commit 2d73f2eb80caf328bc4dd1324d475e7bf6b56837 Author: Jeff Law Date: Tue Aug 1 23:12:16 2023 -0600 [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 [ ]) > (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. 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 eqz [(eq "nez") (ne "eqz")]) (define_code_attr nez [(eq "eqz") (ne "nez")]) ;; Zicond -(define_insn "*czero.." +(define_insn "*czero.." [(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 @@ (define_insn "*czero.." "czero.\t%0,%2,%1" ) -(define_insn "*czero.." +(define_insn "*czero.." [(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 @@ (define_insn "*czero.." ) ;; Special optimization under eq/ne in primitive semantics -(define_insn "*czero.eqz..opt1" +(define_insn "*czero.eqz..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 @@ (define_insn "*czero.eqz..opt1" "czero.eqz\t%0,%3,%1" ) -(define_insn "*czero.eqz..opt2" +(define_insn "*czero.eqz..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 @@ (define_insn "*czero.eqz..opt2" "czero.nez\t%0,%2,%1" ) -(define_insn "*czero.nez..opt3" +(define_insn "*czero.nez..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 @@ (define_insn "*czero.nez..opt3" "czero.eqz\t%0,%2,%1" ) -(define_insn "*czero.nez..opt4" +(define_insn "*czero.nez..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")))]