From patchwork Wed Jul 26 11:40:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Sayle X-Patchwork-Id: 126275 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a985:0:b0:3e4:2afc:c1 with SMTP id t5csp339518vqo; Wed, 26 Jul 2023 04:41:45 -0700 (PDT) X-Google-Smtp-Source: APBJJlEm3i+gZSyLlC3azMuCWDMSEvb+z80ofHgJm8H7z8U8SdzQsc08ZR2t1kyhUhCpnv8FIGZf X-Received: by 2002:a19:6747:0:b0:4fa:5255:4fa3 with SMTP id e7-20020a196747000000b004fa52554fa3mr1336626lfj.5.1690371701741; Wed, 26 Jul 2023 04:41:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690371701; cv=none; d=google.com; s=arc-20160816; b=iheZGjKZe1z7u3Z2R3zHUhL8o2ZGWhcgsLzHopjmrMqHtHJF67zizswSZQN4l8hPjJ zVSx3bg59hZenGJ9c07RrOmgAeVr+E+YjLs4DWoZCivqXMb+kz1vrlHK2pnoldakA8PE wsHEcqkNudSDFJNVI/GRqFXu9r3cKYFJXRSbnTuikEEw0QlCbs1qujXBF8gys3HoeZBQ p3ttchvCe6WkWYgvGtt3n48Lk2gLOt7UQO+EhT1MQ8+PP0Xgnp0GnPktNCNkJS3J/Ipl 4Fevn33PMWtY/ezy1hPjp7DlpxCG2bPCJTQCiTSMD6LzY4lvxUMm7/Ee8lWz1NBmK0Hn 6xkQ== 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:thread-index:content-language :mime-version:message-id:date:subject:cc:to:from:dkim-signature :dmarc-filter:delivered-to; bh=MiwxTS822+WDzlgmKAmeJkncletdlMjFTw4QAyD3lVI=; fh=KBjany5y0us5KHnwAuP5cnfJZBuIWoDNHJrigouSlvk=; b=0iFdjNa4PLhATWWekW11SRJCACUWHd2tBzNO3IUUgCsMkVswYWZq7eaV4QcF/zmp2c zTRfzYfvUYMf2uItYBrJuCug2po9KhhTAkg3R5lp+pjxXHxUjmo+Svj8acZPY47/3Pw5 O6iNoRv94hY9Ae0rzd8PpbLzKwCBuE6ipcwzgbJuhQbT8Lwv0gdgeVjdIHQ2O3IcwZdW keDQAyApcOzMH6kDF6WqmQXVovyOQysdi/TIcSRxKPXDBBjSftz4HqFC4b36WHwqVAw9 976FURNIcR4x5Rp/OZuNYtyCC5ABfdNxKpnAqXy7Nmsahw8nq/5jNPVKSGG54H8YiC4d vvhQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@nextmovesoftware.com header.s=default header.b=sDpzpdvO; 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 ay3-20020a056402202300b005221fd252c8si5601359edb.41.2023.07.26.04.41.41 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Jul 2023 04:41:41 -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=@nextmovesoftware.com header.s=default header.b=sDpzpdvO; 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 BBA85385C423 for ; Wed, 26 Jul 2023 11:41:32 +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 9A2C93858C54 for ; Wed, 26 Jul 2023 11:41:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9A2C93858C54 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=MiwxTS822+WDzlgmKAmeJkncletdlMjFTw4QAyD3lVI=; b=sDpzpdvOVsHC12xj6CFl/GGIY2 ukahNjHIUJEfaFqQDxIyB0EmH8EYRqNwVhLDPp/2TdPJqQmtaJrFs21YYACKTivwkyK8Gs23Zq1Wm daImhfJc26dRqoIz55DmE2apuwPkHhX32YIgvlzBGKbxAepCnwodL1cYqndmRd1iMYXeA2lcv50tW /6AOejlvVye+w4SmjdrGX0iE31KlweE9lrfueeDTJV0nCdp0qOYvtc5XCGP1G8MkQUzkTEHiAdmRb z/jEnT7Iw3jRteUYi5HdCphxtf/6oBPbGlcLk9RwGjQVuZBq4l/gTsrpbHT+aykzJ7PH4MucFKqnI thRg71ew==; Received: from [185.62.158.67] (port=51412 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1qOct3-0005b2-1x; Wed, 26 Jul 2023 07:41:01 -0400 From: "Roger Sayle" To: Cc: , Subject: [PATCH] PR rtl-optimization/110701: Fix SUBREG SET_DEST handling in combine. Date: Wed, 26 Jul 2023 12:40:57 +0100 Message-ID: <04e101d9bfb6$0dacecd0$2906c670$@nextmovesoftware.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Content-Language: en-gb Thread-Index: Adm/tKcD6RvBALuHRqKBV5nTtRYUgg== 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=-12.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, 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: 1772483197535289882 X-GMAIL-MSGID: 1772483197535289882 This patch is my proposed fix to PR rtl-optimization 110701, a latent bug in combine's record_dead_and_set_regs_1 exposed by recent improvements to simplify_subreg. The issue involves the handling of (normal) SUBREG SET_DESTs as in the instruction: (set (subreg:HI (reg:SI x) 0) (expr:HI y)) The semantics of this are that the bits specified by the SUBREG are set to the SET_SRC, y, and that the other bits of the SET_DEST are left/become undefined. To simplify explanation, we'll only consider lowpart SUBREGs (though in theory non-lowpart SUBREGS could be handled), and the fact that bits outside of the lowpart WORD retain their original values (treating these as undefined is a missed optimization rather than incorrect code bug, that only affects targets with less than 64-bit words). The bug is that combine simulates the behaviour of the above instruction, for calculating nonzero_bits and set_sign_bit_copies, in the function record_value_for_reg, by using the equivalent of: (set (reg:SI x) (subreg:SI (expr:HI y)) by calling gen_lowpart on the SET_SRC. Alas, the semantics of this revised instruction aren't always equivalent to the original. In the test case for PR110701, the original instruction (set (subreg:HI (reg:SI x), 0) (and:HI (subreg:HI (reg:SI y) 0) (const_int 340))) which (by definition) leaves the top bits of x undefined, is mistakenly considered to be equivalent to (set (reg:SI x) (and:SI (reg:SI y) (const_int 340))) where gen_lowpart's freedom to do anything with paradoxical SUBREG bits, has now cleared the high bits. The same bug also triggers when the SET_SRC is say (subreg:HI (reg:DI z)), where gen_lowpart transforms this into (subreg:SI (reg:DI z)) which defines bits 16-31 to be the same as bits 16-31 of z. The fix is that after calling record_value_for_reg, we need to mark the bits that should be undefined as undefined, in case gen_lowpart, which performs transforms appropriate for r-values, has changed the interpretation of the SUBREG when used as an l-value. 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? I've a version of this patch that preserves the original bits outside of the lowpart WORD that can be submitted as a follow-up, but this is the piece that addresses the wrong code regression. 2023-07-26 Roger Sayle gcc/ChangeLog PR rtl-optimization/110701 * combine.cc (record_dead_and_set_regs_1): Split comment into pieces placed before the relevant clauses. When the SET_DEST is a partial_subreg_p, mark the bits outside of the updated portion of the destination as undefined. gcc/testsuite/ChangeLog PR rtl-optimization/110701 * gcc.target/i386/pr110701.c: New test case. Thanks in advance, Roger diff --git a/gcc/combine.cc b/gcc/combine.cc index 4bf867d..c5ebb78 100644 --- a/gcc/combine.cc +++ b/gcc/combine.cc @@ -13337,27 +13337,43 @@ record_dead_and_set_regs_1 (rtx dest, const_rtx setter, void *data) if (REG_P (dest)) { - /* If we are setting the whole register, we know its value. Otherwise - show that we don't know the value. We can handle a SUBREG if it's - the low part, but we must be careful with paradoxical SUBREGs on - RISC architectures because we cannot strip e.g. an extension around - a load and record the naked load since the RTL middle-end considers - that the upper bits are defined according to LOAD_EXTEND_OP. */ + /* If we are setting the whole register, we know its value. */ if (GET_CODE (setter) == SET && dest == SET_DEST (setter)) record_value_for_reg (dest, record_dead_insn, SET_SRC (setter)); + /* We can handle a SUBREG if it's the low part, but we must be + careful with paradoxical SUBREGs on RISC architectures because + we cannot strip e.g. an extension around a load and record the + naked load since the RTL middle-end considers that the upper bits + are defined according to LOAD_EXTEND_OP. */ else if (GET_CODE (setter) == SET && GET_CODE (SET_DEST (setter)) == SUBREG && SUBREG_REG (SET_DEST (setter)) == dest && known_le (GET_MODE_PRECISION (GET_MODE (dest)), BITS_PER_WORD) && subreg_lowpart_p (SET_DEST (setter))) - record_value_for_reg (dest, record_dead_insn, - WORD_REGISTER_OPERATIONS - && word_register_operation_p (SET_SRC (setter)) - && paradoxical_subreg_p (SET_DEST (setter)) - ? SET_SRC (setter) - : gen_lowpart (GET_MODE (dest), - SET_SRC (setter))); + { + if (WORD_REGISTER_OPERATIONS + && word_register_operation_p (SET_SRC (setter)) + && paradoxical_subreg_p (SET_DEST (setter))) + record_value_for_reg (dest, record_dead_insn, SET_SRC (setter)); + else if (!partial_subreg_p (SET_DEST (setter))) + record_value_for_reg (dest, record_dead_insn, + gen_lowpart (GET_MODE (dest), + SET_SRC (setter))); + else + { + record_value_for_reg (dest, record_dead_insn, + gen_lowpart (GET_MODE (dest), + SET_SRC (setter))); + + unsigned HOST_WIDE_INT mask; + reg_stat_type *rsp = ®_stat[REGNO (dest)]; + mask = GET_MODE_MASK (GET_MODE (SET_DEST (setter))); + rsp->last_set_nonzero_bits |= ~mask; + rsp->last_set_sign_bit_copies = 1; + } + } + /* Otherwise show that we don't know the value. */ else record_value_for_reg (dest, record_dead_insn, NULL_RTX); } diff --git a/gcc/testsuite/gcc.target/i386/pr110701.c b/gcc/testsuite/gcc.target/i386/pr110701.c new file mode 100644 index 0000000..3f2cea5 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr110701.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +int a; +long b; +int *c = &a; +short d(short e, short f) { return e * f; } +void foo() { + *c = d(340, b >= 0) ^ 3; +} + +/* { dg-final { scan-assembler "andl\[ \\t]\\\$340," } } */ +/* { dg-final { scan-assembler-not "andw\[ \\t]\\\$340," } } */