From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0016_01DA072D.12F54E80" 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 List-Id: This is a multipart message in MIME format. ------=_NextPart_000_0016_01DA072D.12F54E80 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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=E2=80=99d 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=3Dunix{-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 >=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] =3D {1, 2}; > > int foo (char i) { return table[i]; } > > > > compiling with -O2 -mlarge on msp430 we currently see: > > > > Trying 2 -> 7: > > 2: r25:HI=3Dzero_extend(R12:QI) > > REG_DEAD R12:QI > > 7: r28:PSI=3Dsign_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=3Dzero_extend(R12:QI) > > REG_DEAD R12:QI > > 7: r28:PSI=3Dsign_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 =3D 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=3Dunix{-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. >=20 > 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. >=20 > jeff ------=_NextPart_000_0016_01DA072D.12F54E80 Content-Type: text/plain; name="patchmc2.txt" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="patchmc2.txt" diff --git a/gcc/combine.cc b/gcc/combine.cc=0A= index 360aa2f25e6..b1b16ac7bb2 100644=0A= --- a/gcc/combine.cc=0A= +++ b/gcc/combine.cc=0A= @@ -8449,8 +8449,8 @@ make_compound_operation (rtx x, enum rtx_code = in_code)=0A= if (code =3D=3D ZERO_EXTEND)=0A= {=0A= new_rtx =3D make_compound_operation (XEXP (x, 0), next_code);=0A= - tem =3D simplify_const_unary_operation (ZERO_EXTEND, GET_MODE (x),=0A= - new_rtx, GET_MODE (XEXP (x, 0)));=0A= + tem =3D simplify_unary_operation (ZERO_EXTEND, GET_MODE (x),=0A= + new_rtx, GET_MODE (XEXP (x, 0)));=0A= if (tem)=0A= return tem;=0A= SUBST (XEXP (x, 0), new_rtx);=0A= diff --git a/gcc/testsuite/gcc.target/msp430/pr91865.c = b/gcc/testsuite/gcc.target/msp430/pr91865.c=0A= new file mode 100644=0A= index 00000000000..8cc21c8b9e8=0A= --- /dev/null=0A= +++ b/gcc/testsuite/gcc.target/msp430/pr91865.c=0A= @@ -0,0 +1,8 @@=0A= +/* { dg-do compile } */=0A= +/* { dg-options "-O2 -mlarge" } */=0A= +=0A= +const int table[2] =3D {1, 2};=0A= +int foo (char i) { return table[i]; }=0A= +=0A= +/* { dg-final { scan-assembler-not "AND" } } */=0A= +/* { dg-final { scan-assembler-not "RRAM" } } */=0A= ------=_NextPart_000_0016_01DA072D.12F54E80--