From patchwork Thu Jul 14 16:58:29 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Sayle X-Patchwork-Id: 56079 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 62474385700D for ; Thu, 14 Jul 2022 16:58:49 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [162.254.253.69]) by sourceware.org (Postfix) with ESMTPS id F2C703858D1E for ; Thu, 14 Jul 2022 16:58:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F2C703858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nextmovesoftware.com; s=default; h=Content-Type:MIME-Version:Message-ID: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Ifbo8R1rrMiNugUBRUOvStjayuRll+xnC1p22xJOpfU=; b=XIr27l1qiP+oh4cDHv/zSDiRr1 RFqou0LMeakheVWPbr6ybKaE53Xif3dzS//vmM9zsL+aVIgPD2PDBoY5eeVWgyL9rVrTdC/5DUj6j ZPCaAdEkAcuoq6VONs9HH25uV0jL22zM+vyeVjJizSEukMutG5VxLR/bIAynbtTMAjoUxRXOZ6pw2 E+tSlAQS9sIqzb5GETKkOY18fBz3YSKYoP48LO9J/iN1jw87rpBZVqx02WLuT0PXbTVnoAA0z73ST jblHbKzgAhwtItgw8cGwT+w0RZ2jeu5osnTozfjWbUWINTtdkjYB8BbFPt/BV+CeJT4UNyUnIJ8aa kFabmecQ==; Received: from host109-154-33-170.range109-154.btcentralplus.com ([109.154.33.170]:50260 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1oC2AZ-0002wc-84; Thu, 14 Jul 2022 12:58:31 -0400 From: "Roger Sayle" To: "'GCC Patches'" Subject: [PATCH] PR target/106278: Keep REG_EQUAL notes consistent during TImode STV. Date: Thu, 14 Jul 2022 17:58:29 +0100 Message-ID: <01b601d897a2$f1e6cac0$d5b46040$@nextmovesoftware.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AdiXonaI5QaHjWe8TT+WFR7KUbJaTQ== Content-Language: en-gb X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.nextmovesoftware.com X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - nextmovesoftware.com X-Get-Message-Sender-Via: server.nextmovesoftware.com: authenticated_id: roger@nextmovesoftware.com X-Authenticated-Sender: server.nextmovesoftware.com: roger@nextmovesoftware.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, 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+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patch resolves PR target/106278 a regression on x86_64 caused by my recent TImode STV improvements. Now that TImode STV can handle comparisons such as "(set (regs:CC) (compare:CC (reg:TI) ...))" the convert_insn method sensibly checks that the mode of the SET_DEST is TImode before setting it to V1TImode [to avoid V1TImode appearing on the hard reg CC_FLAGS. Hence the current code looks like: if (GET_MODE (dst) == TImode) { tmp = find_reg_equal_equiv_note (insn); if (tmp && GET_MODE (XEXP (tmp, 0)) == TImode) PUT_MODE (XEXP (tmp, 0), V1TImode); PUT_MODE (dst, V1TImode); fix_debug_reg_uses (dst); } break; which checks GET_MODE (dst) before calling PUT_MODE, and when a change is made updating the REG_EQUAL_NOTE tmp if it exists. The logical flaw (oversight) is that due to RTL sharing, the destination of this set may already have been updated to V1TImode, as this chain is being converted, but we still need to update any REG_EQUAL_NOTE that still has TImode. Hence the correct code is actually: if (GET_MODE (dst) == TImode) { PUT_MODE (dst, V1TImode); fix_debug_reg_uses (dst); } if (GET_MODE (dst) == V1TImode) { tmp = find_reg_equal_equiv_note (insn); if (tmp && GET_MODE (XEXP (tmp, 0)) == TImode) PUT_MODE (XEXP (tmp, 0), V1TImode); } break; While fixing this behavior, I noticed I had some indentation whitespace issues and some vestigial dead code in this function/method that I've taken the liberty of cleaning up (as obvious) in this patch. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check, both with and without --target_board=unix{-m32}, with no new failures. Ok for mainline? 2022-07-14 Roger Sayle gcc/ChangeLog PR target/106278 * config/i386/i386-features.cc (general_scalar_chain::convert_insn): Fix indentation whitespace. (timode_scalar_chain::fix_debug_reg_uses): Likewise. (timode_scalar_chain::convert_insn): Delete dead code. Update TImode REG_EQUAL_NOTE even if the SET_DEST is already V1TI. Fix indentation whitespace. (convertible_comparison_p): Likewise. (timode_scalar_to_vector_candidate_p): Likewise. gcc/testsuite/ChangeLog PR target/106278 * gcc.dg/pr106278.c: New test case. Thanks in advance, Roger diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc index f1b03c3..813b203 100644 --- a/gcc/config/i386/i386-features.cc +++ b/gcc/config/i386/i386-features.cc @@ -1054,13 +1054,13 @@ general_scalar_chain::convert_insn (rtx_insn *insn) else if (REG_P (dst) && GET_MODE (dst) == smode) { /* Replace the definition with a SUBREG to the definition we - use inside the chain. */ + use inside the chain. */ rtx *vdef = defs_map.get (dst); if (vdef) dst = *vdef; dst = gen_rtx_SUBREG (vmode, dst, 0); /* IRA doesn't like to have REG_EQUAL/EQUIV notes when the SET_DEST - is a non-REG_P. So kill those off. */ + is a non-REG_P. So kill those off. */ rtx note = find_reg_equal_equiv_note (insn); if (note) remove_note (insn, note); @@ -1246,7 +1246,7 @@ timode_scalar_chain::fix_debug_reg_uses (rtx reg) { rtx_insn *insn = DF_REF_INSN (ref); /* Make sure the next ref is for a different instruction, - so that we're not affected by the rescan. */ + so that we're not affected by the rescan. */ next = DF_REF_NEXT_REG (ref); while (next && DF_REF_INSN (next) == insn) next = DF_REF_NEXT_REG (next); @@ -1336,21 +1336,19 @@ timode_scalar_chain::convert_insn (rtx_insn *insn) rtx dst = SET_DEST (def_set); rtx tmp; - if (MEM_P (dst) && !REG_P (src)) - { - /* There are no scalar integer instructions and therefore - temporary register usage is required. */ - } switch (GET_CODE (dst)) { case REG: if (GET_MODE (dst) == TImode) { + PUT_MODE (dst, V1TImode); + fix_debug_reg_uses (dst); + } + if (GET_MODE (dst) == V1TImode) + { tmp = find_reg_equal_equiv_note (insn); if (tmp && GET_MODE (XEXP (tmp, 0)) == TImode) PUT_MODE (XEXP (tmp, 0), V1TImode); - PUT_MODE (dst, V1TImode); - fix_debug_reg_uses (dst); } break; case MEM: @@ -1410,8 +1408,8 @@ timode_scalar_chain::convert_insn (rtx_insn *insn) if (MEM_P (dst)) { tmp = gen_reg_rtx (V1TImode); - emit_insn_before (gen_rtx_SET (tmp, src), insn); - src = tmp; + emit_insn_before (gen_rtx_SET (tmp, src), insn); + src = tmp; } break; @@ -1434,8 +1432,8 @@ timode_scalar_chain::convert_insn (rtx_insn *insn) if (MEM_P (dst)) { tmp = gen_reg_rtx (V1TImode); - emit_insn_before (gen_rtx_SET (tmp, src), insn); - src = tmp; + emit_insn_before (gen_rtx_SET (tmp, src), insn); + src = tmp; } break; @@ -1448,8 +1446,8 @@ timode_scalar_chain::convert_insn (rtx_insn *insn) if (MEM_P (dst)) { tmp = gen_reg_rtx (V1TImode); - emit_insn_before (gen_rtx_SET (tmp, src), insn); - src = tmp; + emit_insn_before (gen_rtx_SET (tmp, src), insn); + src = tmp; } break; @@ -1585,7 +1583,7 @@ convertible_comparison_p (rtx_insn *insn, enum machine_mode mode) /* *cmp_doubleword. */ if ((CONST_INT_P (op1) || ((REG_P (op1) || MEM_P (op1)) - && GET_MODE (op1) == mode)) + && GET_MODE (op1) == mode)) && (CONST_INT_P (op2) || ((REG_P (op2) || MEM_P (op2)) && GET_MODE (op2) == mode))) @@ -1745,7 +1743,7 @@ timode_scalar_to_vector_candidate_p (rtx_insn *insn) if (GET_MODE (dst) != TImode || (GET_MODE (src) != TImode - && !CONST_SCALAR_INT_P (src))) + && !CONST_SCALAR_INT_P (src))) return false; if (!REG_P (dst) && !MEM_P (dst)) diff --git a/gcc/testsuite/gcc.dg/pr106278.c b/gcc/testsuite/gcc.dg/pr106278.c new file mode 100644 index 0000000..ab312b3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr106278.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +void __assert_fail(); +struct a { + int b; + int c; + int d; + int : 2; +}; +int e, f; +struct a g, i; +const struct a h; +int main() { + struct a j; + g = h; + if (e) + __assert_fail(); + if (f) + j = h; + i = j; + return 0; +}