From patchwork Wed Oct 25 09:21:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Roger Sayle X-Patchwork-Id: 157953 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce89:0:b0:403:3b70:6f57 with SMTP id p9csp2470107vqx; Wed, 25 Oct 2023 02:22:25 -0700 (PDT) X-Google-Smtp-Source: AGHT+IESrUAnI+zElO9S0h0MHFk16wZetxcT2J4UBzr+KgQ3pGwM4fovkK+YTc1JSQZpZY0Ld79F X-Received: by 2002:a05:622a:2c9:b0:419:8f42:8c45 with SMTP id a9-20020a05622a02c900b004198f428c45mr18077329qtx.8.1698225745152; Wed, 25 Oct 2023 02:22:25 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1698225745; cv=pass; d=google.com; s=arc-20160816; b=JS3ALoSt/2WkXl2L4JNmFXq0Roivnhm+XjmMIRahe0ln/aoPrJt0ICaWxyI+thv9p6 OBLKvSY23gsfYW1x5Q0DVetxxWAM3Mnek6de3mUZB4PLDlRyFukN6tyPj0dBgvCOsyoM /3sg4x/yg2MKvNBDKpwTTwEd7TB/h3d2mxKMQlMi3RaXULdNYD6wLycm1OpIZc2xExnR 6RKos7MeVLhvn2ZDe9HoMMH73L9zZtUZ59AWxtha/ntxxHDaDtNueBe9zdtNWKORMdA7 AwbPSWSuYEzBpjJpv9mLd8LsoybFQzv4KH3h5N4w5ppkDDIXZeQOArs4pgqy8P1ld6X2 5O2Q== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-language:thread-index :mime-version:message-id:date:subject:in-reply-to:references:cc:to :from:dkim-signature:arc-filter:dmarc-filter:delivered-to; bh=JZRHK9IkCYk8p2O8xGSBIg3GH0o/ByoJxqtkOwkcw3I=; fh=BpdtwJwV+AnSTxjVA4GBxdimgIVmW+8llDZzCKjLBf8=; b=ktvH28HO+PELnpytjn7cxuEudWdr0UCFq10g2BRZ68QA5uQsmdR8bS/BMNhlV8Pir9 940C6X27OKWx6gNjq/3PysxezLptIT4GVS3pW0ZdS6YweT6RKk6GMQFwRZiPbuyLFH0B RJnRC6GljSPBP5gv6xSGuEtWcCclUGbovkfz4f7N0vWr7OGKY2TTtv8B9D6IDWjjm1s6 xiisorpZgGwSwcnT4P/ABQIzcsMU3+dcN2ICb/RZ6iy9hmel3OR3Kuy4MYoXu5BT/gf1 8PvSj+OYn0SHuhUcIsLuQxvhcjVl6ok25JR9CbW2N2fK6wniUtn/qbBLF9/9qC/N/RcF g+pA== ARC-Authentication-Results: i=2; mx.google.com; dkim=fail header.i=@nextmovesoftware.com header.s=default header.b=X3A84BjI; arc=pass (i=1); 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 y16-20020a05622a165000b00403c3fb27d8si8296230qtj.678.2023.10.25.02.22.25 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Oct 2023 02:22:25 -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=fail header.i=@nextmovesoftware.com header.s=default header.b=X3A84BjI; arc=pass (i=1); 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 20243385702F for ; Wed, 25 Oct 2023 09:22:23 +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 62AA53858017 for ; Wed, 25 Oct 2023 09:21:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 62AA53858017 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 62AA53858017 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=162.254.253.69 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698225717; cv=none; b=B7VpD0l1fm8TIJHHVdW1VFUzQSV0jr1RHZcy23enBrQaM96UjorjhU1gCAUGAOtYNEomqa/2h8OEnp+1upfOQAtRUYATYhunoItjXN9yXlAY1jP7FCkZ3l3Jb4/1VuZMZVcIPZ3psUtc3U+1Z+zGypZxlp8rY9N3F6mElACIcA0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698225717; c=relaxed/simple; bh=kqX0DGlXPrz4DiExxkjn/TgIXsabzzu/jvewj9eZM74=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=rh08R637EJzv7nLdXRTSlgQ5pGrWelRlmLNx0K37Mj+baCtKAwnYKVt54WuuZ03IEUYZ0iPZBlu5mgITSF5+MI0tnX+9ccwExfQz67OGCWXL2VTkw2iz+4vkD0tBJN0TDtkHyqrrDn9ne1HoQpwhNQ80hshVWS+rsiq7hA+HMF8= ARC-Authentication-Results: i=1; server2.sourceware.org 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:In-Reply-To:References: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:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=JZRHK9IkCYk8p2O8xGSBIg3GH0o/ByoJxqtkOwkcw3I=; b=X3A84BjIqF08YN3QhOqBOGqpex bM8nZxK0Y3Y3k03aVyxwvxg3EgdIipqIpygKgsF5su+R8XxDSPpyNeqDH8CuUMDxw2rX0WClLtuuT SlwcMxCJ5GwJrAGIQYMkRQh5eZpbw0rFrF/VJgfAS3NrayXxP7SGn3DZJfs6PGN3EnN+jjIhKqKgu aQxraYrAwUDRxO5ejmnyQg1wtvBTVRGPbf/VL3NsbwiJ8yTioVjkRxQuyAsmdGQ9TtDROEvgAF4FZ MbWjvNo0iC2K6YZCS217bfmE1RZBXCEIasXinAcjGu6ETqcbOcoFYjV7Iu9OaBr3Z85IyjxaymqYH /6G3luGw==; Received: from host86-160-20-38.range86-160.btcentralplus.com ([86.160.20.38]:51206 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96.2) (envelope-from ) id 1qva5J-0005CD-0p; Wed, 25 Oct 2023 05:21:53 -0400 From: "Roger Sayle" To: "'Jeff Law'" Cc: , "'Richard Biener'" References: <01f701d9feeb$ce8654e0$6b92fea0$@nextmovesoftware.com> <48dd875c-b65f-451c-b878-3b1c1215227d@gmail.com> In-Reply-To: <48dd875c-b65f-451c-b878-3b1c1215227d@gmail.com> Subject: [PATCH v2] PR 91865: Avoid ZERO_EXTEND of ZERO_EXTEND in make_compound_operation. Date: Wed, 25 Oct 2023 10:21:51 +0100 Message-ID: <001501da0724$b12ffc20$138ff460$@nextmovesoftware.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AdoHIk0xgmJdnuhmTgKed/le7ohtqA== 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=-12.1 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 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+ouuuleilei=gmail.com@gcc.gnu.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1779770795500474707 X-GMAIL-MSGID: 1780718758692973530 Hi Jeff, Many thanks for the review/approval of my fix for PR rtl-optimization/91865. Based on your and Richard Biener's feedback, I’d like to propose a revision calling simplify_unary_operation instead of simplify_const_unary_operation (i.e. Richi's recommendation). I was originally concerned that this might potentially result in unbounded recursion, and testing for ZERO_EXTEND was safer but "uglier", but testing hasn't shown any issues. If we do see issues in the future, it's easy to fall back to the previous version of 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? 2023-10-25 Roger Sayle Richard Biener gcc/ChangeLog PR rtl-optimization/91865 * combine.cc (make_compound_operation): Avoid creating a ZERO_EXTEND of a ZERO_EXTEND. gcc/testsuite/ChangeLog PR rtl-optimization/91865 * gcc.target/msp430/pr91865.c: New test case. Thanks again, Roger --- > -----Original Message----- > From: Jeff Law > Sent: 19 October 2023 16:20 > > On 10/14/23 16:14, Roger Sayle wrote: > > > > This patch is my proposed solution to PR rtl-optimization/91865. > > Normally RTX simplification canonicalizes a ZERO_EXTEND of a > > ZERO_EXTEND to a single ZERO_EXTEND, but as shown in this PR it is > > possible for combine's make_compound_operation to unintentionally > > generate a non-canonical ZERO_EXTEND of a ZERO_EXTEND, which is > > unlikely to be matched by the backend. > > > > For the new test case: > > > > const int table[2] = {1, 2}; > > int foo (char i) { return table[i]; } > > > > compiling with -O2 -mlarge on msp430 we currently see: > > > > Trying 2 -> 7: > > 2: r25:HI=zero_extend(R12:QI) > > REG_DEAD R12:QI > > 7: r28:PSI=sign_extend(r25:HI)#0 > > REG_DEAD r25:HI > > Failed to match this instruction: > > (set (reg:PSI 28 [ iD.1772 ]) > > (zero_extend:PSI (zero_extend:HI (reg:QI 12 R12 [ iD.1772 ])))) > > > > which results in the following code: > > > > foo: AND #0xff, R12 > > RLAM.A #4, R12 { RRAM.A #4, R12 > > RLAM.A #1, R12 > > MOVX.W table(R12), R12 > > RETA > > > > With this patch, we now see: > > > > Trying 2 -> 7: > > 2: r25:HI=zero_extend(R12:QI) > > REG_DEAD R12:QI > > 7: r28:PSI=sign_extend(r25:HI)#0 > > REG_DEAD r25:HI > > Successfully matched this instruction: > > (set (reg:PSI 28 [ iD.1772 ]) > > (zero_extend:PSI (reg:QI 12 R12 [ iD.1772 ]))) allowing > > combination of insns 2 and 7 original costs 4 + 8 = 12 replacement > > cost 8 > > > > foo: MOV.B R12, R12 > > RLAM.A #1, R12 > > MOVX.W table(R12), R12 > > RETA > > > > > > 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? > > > > 2023-10-14 Roger Sayle > > > > gcc/ChangeLog > > PR rtl-optimization/91865 > > * combine.cc (make_compound_operation): Avoid creating a > > ZERO_EXTEND of a ZERO_EXTEND. > Final question. Is there a reasonable expectation that we could get a > similar situation with sign extensions? If so we probably ought to try > and handle both. > > OK with the obvious change to handle nested sign extensions if you think it's > useful to do so. And OK as-is if you don't think handling nested sign extensions is > useful. > > jeff diff --git a/gcc/combine.cc b/gcc/combine.cc index 360aa2f25e6..b1b16ac7bb2 100644 --- a/gcc/combine.cc +++ b/gcc/combine.cc @@ -8449,8 +8449,8 @@ make_compound_operation (rtx x, enum rtx_code in_code) if (code == ZERO_EXTEND) { new_rtx = make_compound_operation (XEXP (x, 0), next_code); - tem = simplify_const_unary_operation (ZERO_EXTEND, GET_MODE (x), - new_rtx, GET_MODE (XEXP (x, 0))); + tem = simplify_unary_operation (ZERO_EXTEND, GET_MODE (x), + new_rtx, GET_MODE (XEXP (x, 0))); if (tem) return tem; SUBST (XEXP (x, 0), new_rtx); diff --git a/gcc/testsuite/gcc.target/msp430/pr91865.c b/gcc/testsuite/gcc.target/msp430/pr91865.c new file mode 100644 index 00000000000..8cc21c8b9e8 --- /dev/null +++ b/gcc/testsuite/gcc.target/msp430/pr91865.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mlarge" } */ + +const int table[2] = {1, 2}; +int foo (char i) { return table[i]; } + +/* { dg-final { scan-assembler-not "AND" } } */ +/* { dg-final { scan-assembler-not "RRAM" } } */