From patchwork Wed Mar 6 16:59:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 1908926 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=ventanamicro.com header.i=@ventanamicro.com header.a=rsa-sha256 header.s=google header.b=Gv2qCO21; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Tqdvp6Ct5z1yWw for ; Thu, 7 Mar 2024 03:59:49 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4F6883857C48 for ; Wed, 6 Mar 2024 16:59:47 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pj1-x1030.google.com (mail-pj1-x1030.google.com [IPv6:2607:f8b0:4864:20::1030]) by sourceware.org (Postfix) with ESMTPS id A303D3857C46 for ; Wed, 6 Mar 2024 16:59:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A303D3857C46 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=ventanamicro.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ventanamicro.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A303D3857C46 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::1030 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709744366; cv=none; b=ZGgzkIWoFFGNOyQSYUPzGDDvf6YEYgmCVrvUtRXMXvhJkmEONexKTBRe1kGd13oh4BsvGwie+jqp56UY/B2WTQIcHDJqRgfH1o/C6DRxo3n+RP2tH0vRoF1v4awbiMJ9kRJyCgDsFrjJIpHI8NA6Ai9bIfu38lUVjlhz9eQm19U= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709744366; c=relaxed/simple; bh=a05q5ZwWFgjic/rq8213eOPg2A5JYXHTNiPcsDet70Q=; h=DKIM-Signature:Message-ID:Date:MIME-Version:To:From:Subject; b=AklSXA9VuXNY7CWmyjLz7TSGa7IA3XzhTDjaQ6f+72FSAL/XZS4kK6oFLHjdipluWPhlpDDApvshoq3b7GTfak0moALxsJQXP3g/evhTwga9vA01gKzv7BpoBJgLoWxa3nreefjSTwlWUYaIZNQb/xzN5ejRKwV6lG97OdZ1dMI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pj1-x1030.google.com with SMTP id 98e67ed59e1d1-299b92948a6so4350456a91.3 for ; Wed, 06 Mar 2024 08:59:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1709744363; x=1710349163; darn=gcc.gnu.org; h=subject:from:to:content-language:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=ArcOgl6tg+874VCgwxp03649PEmklIz7BROQdKT3O8I=; b=Gv2qCO21c0dBRzYghvBP5+KhXw9lgchohjSuk6ZqD/ZpofwKvoWfs0gCvc+Rj0vG2k g1EeDgpY9AGVW2Km25Ud6wHFbeqzNPIH8h/f9R/ftNMuN5ql/vQKpjFLJToxZs6Gq5/T xblPHkvYdD5AESO9UZzE/AJq22U4kF9uTTISGMiaX3baJLwzj6V9XFAc5m9eBxZPsT4t Sq8jdIkNsTF94nOtsItmmyLTGjo+HJMLn1OgdyUtmKyqo34ltNRPS1Dhlsxp2zgi9j9J nKp7LxaR9L1khrC7lObYb/vbzAj+9CvIPf+x29lSrhpCyOPRz+05l1FACUjcbj9neIG5 Ma6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709744363; x=1710349163; h=subject:from:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ArcOgl6tg+874VCgwxp03649PEmklIz7BROQdKT3O8I=; b=uL1LJ7eybNzoaSlXftryRdUkNMqkBr3PHuQ2cQUEsdrz/yCkBrB5EBrna0LdC5tQSz QMFWwL7URjVXNboM9n1ZeVabULKeANGrUKgICUKpuMTGN33ckJQDwNhejzlW6V6XLAAf eMHkn/xnBWK/tNecuEtN4ko4JaJXmWdaT5TPsW4pr/lui68+mpM9Acdt81nZUtEWAidG 3iqbHzV8pk6kEGhSQDSkSkH19cpFqAM/sRNi31/uGhT6zEfBcH/fcle52ImmPrq+RSGB JdrYcCwGLefytTtU4PC+L9ppPSufsWePCV+b9sNKKkVpmXrN6E4r5/Keneqhm33CbQE0 umUw== X-Gm-Message-State: AOJu0Yw/sQeTmBFhZgalwf0+6JFlZiatq68wr+WT/zSM7un3EO5iY5kC sBoj6Cy838nBdViKBcF1F/80Av1jlXZu+98kX+dB6Vjq0JxbAqCntwZn8PBUsPI3eRDiW8Wu0cT X X-Google-Smtp-Source: AGHT+IFWsWNr3l3WNzGK5uS9EgaecsskjqQXPYdsMMY5uQFoBe/jpG88SZIgYP8kOn+2Vy1ZJWs+ug== X-Received: by 2002:a17:90b:1bd0:b0:29b:3d08:c644 with SMTP id oa16-20020a17090b1bd000b0029b3d08c644mr9540322pjb.9.1709744362972; Wed, 06 Mar 2024 08:59:22 -0800 (PST) Received: from [172.31.0.109] ([136.36.72.243]) by smtp.gmail.com with ESMTPSA id t9-20020a17090aae0900b0029b12d6b4a3sm11145885pjq.39.2024.03.06.08.59.22 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 06 Mar 2024 08:59:22 -0800 (PST) Message-ID: <22a92171-6964-48f1-b6b9-ba471c5f9ef0@ventanamicro.com> Date: Wed, 6 Mar 2024 09:59:21 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Content-Language: en-US To: "gcc-patches@gcc.gnu.org" From: Jeff Law Subject: [PR target/113001] Fix incorrect operand swapping in conditional move X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, 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.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org This bug totally fell off my radar. Sorry about that. We have some special casing the conditional move expander to simplify a conditional move when comparing a register against zero and that same register is one of the arms. Specifically a (eq (reg) (const_int 0)) where reg is also the true arm or (ne (reg) (const_int 0)) where reg is the false arm need not use the fully generalized conditional move, thus saving an instruction for those cases. In the NE case we swapped the operands, but didn't swap the condition, which led to the ICE due to an unrecognized pattern. THe backend actually has distinct patterns for those two cases. So swapping the operands is neither needed nor advisable. Regression tested on rv64gc and verified the new tests pass. Pushing to the trunk. jeff commit 10cbfcd60f9e5bdbe486e1c0192e0f168d899b77 Author: Jeff Law Date: Wed Mar 6 09:50:44 2024 -0700 [PR target/113001] Fix incorrect operand swapping in conditional move This bug totally fell off my radar. Sorry about that. We have some special casing the conditional move expander to simplify a conditional move when comparing a register against zero and that same register is one of the arms. Specifically a (eq (reg) (const_int 0)) where reg is also the true arm or (ne (reg) (const_int 0)) where reg is the false arm need not use the fully generalized conditional move, thus saving an instruction for those cases. In the NE case we swapped the operands, but didn't swap the condition, which led to the ICE due to an unrecognized pattern. THe backend actually has distinct patterns for those two cases. So swapping the operands is neither needed nor advisable. Regression tested on rv64gc and verified the new tests pass. Pushing to the trunk. PR target/113001 PR target/112871 gcc/ * config/riscv/riscv.cc (expand_conditional_move): Do not swap operands when the comparison operand is the same as the false arm for a NE test. gcc/testsuite * gcc.target/riscv/zicond-ice-3.c: New test. * gcc.target/riscv/zicond-ice-4.c: New test. diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index 691d967de29..680c4a728e9 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -4633,8 +4633,6 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx cons, rtx alt) || (code == NE && rtx_equal_p (alt, op0))) { rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1); - if (!rtx_equal_p (cons, op0)) - std::swap (alt, cons); alt = force_reg (mode, alt); emit_insn (gen_rtx_SET (dest, gen_rtx_IF_THEN_ELSE (mode, cond, diff --git a/gcc/testsuite/gcc.target/riscv/zicond-ice-3.c b/gcc/testsuite/gcc.target/riscv/zicond-ice-3.c new file mode 100644 index 00000000000..650986825ef --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/zicond-ice-3.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gc_zicond -mabi=lp64d" { target { rv64 } } } */ +/* { dg-options "-march=rv32gc_zicond -mabi=ilp32d" { target { rv32 } } } */ + +long a, b; +int c, d; +void e(long *f) { + (b = *f) && --b; + for (; c;) + ; +} +void g() { + for (; d; d--) + e(&a); +} diff --git a/gcc/testsuite/gcc.target/riscv/zicond-ice-4.c b/gcc/testsuite/gcc.target/riscv/zicond-ice-4.c new file mode 100644 index 00000000000..2be02c78a08 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/zicond-ice-4.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gc_zicond -mabi=lp64d" { target { rv64 } } } */ +/* { dg-options "-march=rv32gc_zicond -mabi=ilp32d" { target { rv32 } } } */ + +short a, c; +int b, d, i; +volatile char e; +static int f[] = {1, 1}; +long g; +int volatile h; +short(j)() { return b ? a : 0; } +void k() { +l: + h; + g = 0; + for (; g <= 2; g++) { + d | ((i || j() & (0 == f[g])) ^ i) && e; + if (c) + goto l; + } +} +