Message ID | 243c7199-e6a5-f04b-568c-64024590695f@linux.ibm.com |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> 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 45767385736C for <patchwork@sourceware.org>; Fri, 13 May 2022 01:08:37 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 45767385736C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1652404117; bh=tBYVEEyeG5+w7NH1PpLpmRDMojVPDYY9jI+/KyjPYLs=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=eyIIgAduHWuowmMuyIt0eZldfpWTVnZqRWbjbYRsDAd/sd9ffEqyA9gWaSmS4z5wz WIy2VJ2l9VopKuj6tSwjsO6Kcmy3qoNNynxb+Vff9M7kVWaJJmlSVIEY1p9HOLqPO3 l39L2vcuD5dV5sFyWIDHrKna7AtqyD8J3AwCkFCk= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 213383858427 for <gcc-patches@gcc.gnu.org>; Fri, 13 May 2022 01:08:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 213383858427 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 24D0fvRD001942; Fri, 13 May 2022 01:08:06 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3g1d0w0bxk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 13 May 2022 01:08:05 +0000 Received: from m0098410.ppops.net (m0098410.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 24D185C8019099; Fri, 13 May 2022 01:08:05 GMT Received: from ppma06ams.nl.ibm.com (66.31.33a9.ip4.static.sl-reverse.com [169.51.49.102]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3g1d0w0bx5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 13 May 2022 01:08:05 +0000 Received: from pps.filterd (ppma06ams.nl.ibm.com [127.0.0.1]) by ppma06ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 24D13XTi032700; Fri, 13 May 2022 01:08:03 GMT Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by ppma06ams.nl.ibm.com with ESMTP id 3fyrkk3nxu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 13 May 2022 01:08:02 +0000 Received: from b06wcsmtp001.portsmouth.uk.ibm.com (b06wcsmtp001.portsmouth.uk.ibm.com [9.149.105.160]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 24D17x9049349062 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 13 May 2022 01:07:59 GMT Received: from b06wcsmtp001.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7EFD5A405B; Fri, 13 May 2022 01:07:59 +0000 (GMT) Received: from b06wcsmtp001.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 868F8A4054; Fri, 13 May 2022 01:07:57 +0000 (GMT) Received: from [9.200.32.112] (unknown [9.200.32.112]) by b06wcsmtp001.portsmouth.uk.ibm.com (Postfix) with ESMTP; Fri, 13 May 2022 01:07:57 +0000 (GMT) Message-ID: <243c7199-e6a5-f04b-568c-64024590695f@linux.ibm.com> Date: Fri, 13 May 2022 09:07:54 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Content-Language: en-US To: gcc-patches <gcc-patches@gcc.gnu.org> Subject: [PATCH v4, rs6000] Add a combine pattern for CA minus one [PR95737] Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: GxtyPYRJsFk-CoyaTuP5VLm0oKbfHWz6 X-Proofpoint-ORIG-GUID: R449ojrPoKioFqlBMIxqSXsPhiQCaH2v X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.858,Hydra:6.0.486,FMLib:17.11.64.514 definitions=2022-05-12_19,2022-05-12_01,2022-02-23_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 malwarescore=0 adultscore=0 spamscore=0 suspectscore=0 bulkscore=0 phishscore=0 mlxscore=0 priorityscore=1501 clxscore=1015 mlxlogscore=999 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000 definitions=main-2205130005 X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_MSPIKE_H2, 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: HAO CHEN GUI via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: HAO CHEN GUI <guihaoc@linux.ibm.com> Cc: Peter Bergner <bergner@linux.ibm.com>, David <dje.gcc@gmail.com>, Segher Boessenkool <segher@kernel.crashing.org> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
[v4,rs6000] Add a combine pattern for CA minus one [PR95737]
|
|
Commit Message
HAO CHEN GUI
May 13, 2022, 1:07 a.m. UTC
Hi, This patch adds a combine pattern for "CA minus one". As CA only has two values (0 or 1), we could convert following pattern (sign_extend:DI (plus:SI (reg:SI 98 ca) (const_int -1 [0xffffffffffffffff])))) to (plus:DI (reg:DI 98 ca) (const_int -1 [0xffffffffffffffff]))) With this patch, one unnecessary sign extend is eliminated. Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. Is this okay for trunk? Any recommendations? Thanks a lot. ChangeLog 2022-05-13 Haochen Gui <guihaoc@linux.ibm.com> gcc/ PR target/95737 * config/rs6000/rs6000.md (extenddi_ca_minus_one): Define. gcc/testsuite/ PR target/95737 * gcc.target/powerpc/pr95737.c: New. patch.diff
Comments
Hi Haochen, on 2022/5/13 09:07, HAO CHEN GUI wrote: > Hi, > This patch adds a combine pattern for "CA minus one". As CA only has two > values (0 or 1), we could convert following pattern > (sign_extend:DI (plus:SI (reg:SI 98 ca) > (const_int -1 [0xffffffffffffffff])))) > to > (plus:DI (reg:DI 98 ca) > (const_int -1 [0xffffffffffffffff]))) > With this patch, one unnecessary sign extend is eliminated. > > Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. > Is this okay for trunk? Any recommendations? Thanks a lot. > > ChangeLog > 2022-05-13 Haochen Gui <guihaoc@linux.ibm.com> > > gcc/ > PR target/95737 > * config/rs6000/rs6000.md (extenddi_ca_minus_one): Define. > Nit: (*extenddi_ca_minus_one): New define_insn_and_split. > gcc/testsuite/ > PR target/95737 > * gcc.target/powerpc/pr95737.c: New. > > patch.diff > diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md > index 64049a6e521..483a93956f8 100644 > --- a/gcc/config/rs6000/rs6000.md > +++ b/gcc/config/rs6000/rs6000.md > @@ -2353,6 +2353,19 @@ (define_insn "subf<mode>3_carry_in_xx" > "subfe %0,%0,%0" > [(set_attr "type" "add")]) > > +(define_insn_and_split "*extenddi_ca_minus_one" > + [(set (match_operand:DI 0 "gpc_reg_operand" "=r") > + (sign_extend:DI (plus:SI (reg:SI CA_REGNO) > + (const_int -1))))] > + "" > + "#" > + "" > + [(parallel [(set (match_dup 0) > + (plus:DI (reg:DI CA_REGNO) > + (const_int -1))) > + (clobber (reg:DI CA_REGNO))])] > + "" > +) > > (define_insn "@neg<mode>2" > [(set (match_operand:GPR 0 "gpc_reg_operand" "=r") > diff --git a/gcc/testsuite/gcc.target/powerpc/pr95737.c b/gcc/testsuite/gcc.target/powerpc/pr95737.c > new file mode 100644 > index 00000000000..d4d6a4198cf > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr95737.c > @@ -0,0 +1,10 @@ > +/* PR target/95737 */ > +/* { dg-do compile { target lp64 } } */> +/* { dg-options "-O2 -mno-isel" } */ Nit: It seems good to put one comment for the reason why we need this special -mno-isel, like something: Power9 leverages isel for this case, force -mno-isel to keep the test point valid on Power9 and later." > +/* { dg-final { scan-assembler-not {\mextsw\M} } } */ > + > + > +unsigned long negativeLessThan (unsigned long a, unsigned long b) > +{ > + return -(a < b); > +} OK with those nits fixed, but please wait for a few days, just in case Segher/David have more comments. BR, Kewen
Hi! On Fri, May 13, 2022 at 09:07:54AM +0800, HAO CHEN GUI wrote: > This patch adds a combine pattern for "CA minus one". As CA only has two > values (0 or 1), we could convert following pattern > (sign_extend:DI (plus:SI (reg:SI 98 ca) > (const_int -1 [0xffffffffffffffff])))) > to > (plus:DI (reg:DI 98 ca) > (const_int -1 [0xffffffffffffffff]))) > With this patch, one unnecessary sign extend is eliminated. > +(define_insn_and_split "*extenddi_ca_minus_one" > + [(set (match_operand:DI 0 "gpc_reg_operand" "=r") > + (sign_extend:DI (plus:SI (reg:SI CA_REGNO) > + (const_int -1))))] > + "" > + "#" > + "" > + [(parallel [(set (match_dup 0) > + (plus:DI (reg:DI CA_REGNO) > + (const_int -1))) > + (clobber (reg:DI CA_REGNO))])] > + "" > +) This is the subf<mode>3_carry_in_xx pattern but with an extend, so a better name (well, more like existing names :-) ) would be *subfsi3_carry_in_xx_64. You already put it right after the more basic pattern, which would have been my next suggestion :-) It needs TARGET_POWERPC64 in the insn condition. Without it, DImode does exist, but it stands for two registers then. Very unlikely to ever match the RTL of course, but it's much cleaner to not risk it even. It would be better to teach simplify-rtx how to do this, but it will have problems understanding that CA can only be 0 or 1. Okay. > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr95737.c > @@ -0,0 +1,10 @@ > +/* PR target/95737 */ > +/* { dg-do compile { target lp64 } } */ This testcase will work fine on 32 bit. Of course there is no extsw insn generated then no matter what, but it simplifies the testcase, and it gives a bit more test coverage anyway. In general, don't restrict testcases to only be tested for some systems, unless there is a reason for that (and if that reason isn't obvious, make a short note in the testcase itself: /* { dg-do compile { target lp64 } } */ /* This requires lp64 because of XYZ. */ or similar). So please add that TARGET_POWERPC64 and remove the lp64 from the testcase. Oh and the pattern name. Looks perfect to me then. Thanks, Segher
Hi! On Fri, May 13, 2022 at 10:40:22AM +0800, Kewen.Lin wrote: > on 2022/5/13 09:07, HAO CHEN GUI wrote: > > * config/rs6000/rs6000.md (extenddi_ca_minus_one): Define. > > Nit: (*extenddi_ca_minus_one): New define_insn_and_split. Or just "New." even :-) It's boring, yes, but boring is good. It helps to be more verbose sometimes, certainly, but it isn't required here. Changelog entries are free-form in any case, after the colons that is :-) > > +/* { dg-options "-O2 -mno-isel" } */ > > Nit: It seems good to put one comment for the reason why we need this > special -mno-isel, like something: Power9 leverages isel for this case, > force -mno-isel to keep the test point valid on Power9 and later." But in fewer words if you can :-) Even "/* for p9 and later */" would be very helpful already. It's not necessary to explain everything, but giving some leads to whoever gets to investigate this testcase next is useful :-) Segher
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 64049a6e521..483a93956f8 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -2353,6 +2353,19 @@ (define_insn "subf<mode>3_carry_in_xx" "subfe %0,%0,%0" [(set_attr "type" "add")]) +(define_insn_and_split "*extenddi_ca_minus_one" + [(set (match_operand:DI 0 "gpc_reg_operand" "=r") + (sign_extend:DI (plus:SI (reg:SI CA_REGNO) + (const_int -1))))] + "" + "#" + "" + [(parallel [(set (match_dup 0) + (plus:DI (reg:DI CA_REGNO) + (const_int -1))) + (clobber (reg:DI CA_REGNO))])] + "" +) (define_insn "@neg<mode>2" [(set (match_operand:GPR 0 "gpc_reg_operand" "=r") diff --git a/gcc/testsuite/gcc.target/powerpc/pr95737.c b/gcc/testsuite/gcc.target/powerpc/pr95737.c new file mode 100644 index 00000000000..d4d6a4198cf --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr95737.c @@ -0,0 +1,10 @@ +/* PR target/95737 */ +/* { dg-do compile { target lp64 } } */ +/* { dg-options "-O2 -mno-isel" } */ +/* { dg-final { scan-assembler-not {\mextsw\M} } } */ + + +unsigned long negativeLessThan (unsigned long a, unsigned long b) +{ + return -(a < b); +}