Message ID | c94c55ff-38dd-d15b-05f6-6bba18aff5b0@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 DC37B3858D35 for <patchwork@sourceware.org>; Thu, 20 Jan 2022 07:36:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DC37B3858D35 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1642664194; bh=lprlAN9S29pcpR7uHdx+vaJxrpvEFw5wQcTL0vt/9kU=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=EOMf0emNWqurDfFi/a6KgyMtBKBcPZ0K6LoS/heGsidXjmNAWx+tyzsGCPpYwPMS0 MXCDzU3TIn1zIK1SPuTqFkskQweIB/+F8DZs1ZTS9sznjb8MntDANZqVIhWws9NcTj +rFGyO4PXlL1eyoN/1iZSd4d//fKwNhNVDP/i6O8= 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 EBB353858D35 for <gcc-patches@gcc.gnu.org>; Thu, 20 Jan 2022 07:36:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EBB353858D35 Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 20K5RkY7017215; Thu, 20 Jan 2022 07:35:59 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 3dq1m3j1y3-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Jan 2022 07:35:59 +0000 Received: from m0098393.ppops.net (m0098393.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 20K7NDBY022421; Thu, 20 Jan 2022 07:35:59 GMT Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0a-001b2d01.pphosted.com with ESMTP id 3dq1m3j1xr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Jan 2022 07:35:58 +0000 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 20K7W7sb027936; Thu, 20 Jan 2022 07:35:56 GMT Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by ppma03ams.nl.ibm.com with ESMTP id 3dknwa6gmp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Jan 2022 07:35:56 +0000 Received: from d06av21.portsmouth.uk.ibm.com (d06av21.portsmouth.uk.ibm.com [9.149.105.232]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 20K7Zsfg37290274 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 20 Jan 2022 07:35:54 GMT Received: from d06av21.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 105D052059; Thu, 20 Jan 2022 07:35:54 +0000 (GMT) Received: from [9.197.236.137] (unknown [9.197.236.137]) by d06av21.portsmouth.uk.ibm.com (Postfix) with ESMTP id 8C18A52050; Thu, 20 Jan 2022 07:35:51 +0000 (GMT) Message-ID: <c94c55ff-38dd-d15b-05f6-6bba18aff5b0@linux.ibm.com> Date: Thu, 20 Jan 2022 15:35:48 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Content-Language: en-US To: gcc-patches <gcc-patches@gcc.gnu.org> Subject: [PATCH v2, 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-ORIG-GUID: ZPnJ77jvDDQJHEpxpLiwbdVzf2HYcPaZ X-Proofpoint-GUID: EbLKWBHaUfTl3pHshmraYERpvHNqQveb X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2022-01-20_02,2022-01-19_01,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 mlxscore=0 priorityscore=1501 suspectscore=0 spamscore=0 lowpriorityscore=0 mlxlogscore=999 adultscore=0 bulkscore=0 malwarescore=0 phishscore=0 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000 definitions=main-2201200038 X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Bill Schmidt <wschmidt@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 |
[v2,rs6000] Add a combine pattern for CA minus one [PR95737]
|
|
Commit Message
HAO CHEN GUI
Jan. 20, 2022, 7:35 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-01-20 Haochen Gui <guihaoc@linux.ibm.com> gcc/ * config/rs6000/rs6000.md (extenddi_ca_minus_one): Define. gcc/testsuite/ * gcc.target/powerpc/pr95737.c: New. patch.diff
Comments
On Thu, Jan 20, 2022 at 2:36 AM HAO CHEN GUI <guihaoc@linux.ibm.com> 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-01-20 Haochen Gui <guihaoc@linux.ibm.com> > > gcc/ > * config/rs6000/rs6000.md (extenddi_ca_minus_one): Define. > > gcc/testsuite/ > * gcc.target/powerpc/pr95737.c: New. > > > patch.diff > diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md > index 6ecb0bd6142..1d8b212962f 100644 > --- a/gcc/config/rs6000/rs6000.md > +++ b/gcc/config/rs6000/rs6000.md > @@ -2358,6 +2358,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..94320f23423 > --- /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 -mdejagnu-cpu=power8" } */ Why does the testcase force power8? This testcase is not specific to Power8 or later. > +/* { dg-final { scan-assembler-not {\mextsw\M} } } */ > + > + > +unsigned long long negativeLessThan (unsigned long long a, unsigned long long b) > +{ > + return -(a < b); > +} If you're only testing for lp64, the testcase could use "long" instead of "long long". This is okay with those changes. Thanks, David
Hi! On Thu, Jan 20, 2022 at 01:46:48PM -0500, David Edelsohn wrote: > On Thu, Jan 20, 2022 at 2:36 AM HAO CHEN GUI <guihaoc@linux.ibm.com> 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. > > > > Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. > > Is this okay for trunk? Any recommendations? Thanks a lot. There are ten gazillion similar things we could make extra backend patterns for, and we still would not cover a majority of cases. If instead we got some generic way to handle this we could cover many more cases, for much less effort. We need both widening modes from SI to DI, amd narrowing modes from DI to SI. Both are useful in certain cases; it is not like using wider modes is always better, in some cases narrower modes is better (in cases where we can let the generated code then generate whatever bits in the high half of the word, for example; a typical example is addition in an unsigned int). > > --- /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 -mdejagnu-cpu=power8" } */ > > Why does the testcase force power8? This testcase is not specific to > Power8 or later. Yes, and we should generate the same code on older machines. > > +/* { dg-final { scan-assembler-not {\mextsw\M} } } */ > > + > > + > > +unsigned long long negativeLessThan (unsigned long long a, unsigned long long b) > > +{ > > + return -(a < b); > > +} > > If you're only testing for lp64, the testcase could use "long" instead > of "long long". The testcase really needs "powerpc64", if that would mean "test if -mpowerpc64 is (implicitly) used". But that is not what it currently means (it is something akin to "powerpc64_hw", instead). So we test lp64, which is set if and only if -m64 was used. It is reasonable coverage, no one cares much for -m32 -mpowerpc64 . Segher
Thanks so much for your advice. Please see my comments. On 21/1/2022 上午 5:42, Segher Boessenkool wrote: > Hi! > > On Thu, Jan 20, 2022 at 01:46:48PM -0500, David Edelsohn wrote: >> On Thu, Jan 20, 2022 at 2:36 AM HAO CHEN GUI <guihaoc@linux.ibm.com> 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. >>> >>> Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. >>> Is this okay for trunk? Any recommendations? Thanks a lot. > > There are ten gazillion similar things we could make extra backend > patterns for, and we still would not cover a majority of cases. > > If instead we got some generic way to handle this we could cover many > more cases, for much less effort. Could we add an additional pass to exam the finally generated instructions and its used registers to decide which extension is unnecessary? > > We need both widening modes from SI to DI, amd narrowing modes from DI > to SI. Both are useful in certain cases; it is not like using wider > modes is always better, in some cases narrower modes is better (in cases > where we can let the generated code then generate whatever bits in the > high half of the word, for example; a typical example is addition in an > unsigned int). Just for this case, converting CA from DI to SI is supported in simplify_rtx. The original comparison result is in DI mode. But it's truncated to SI mode as C standard requires. Trying 8 -> 11: 8: {r127:DI=ca:DI-0x1;clobber ca:DI;} REG_DEAD ca:DI REG_UNUSED ca:DI 11: r128:SI=r127:DI#0 REG_DEAD r127:DI Successfully matched this instruction: (set (reg:SI 128) (plus:SI (reg:SI 98 ca) (const_int -1 [0xffffffffffffffff]))) allowing combination of insns 8 and 11 original costs 4 + 4 = 8 replacement cost 4 deferring deletion of insn with uid = 8. modifying insn i3 11: {r128:SI=ca:SI-0x1;clobber ca:SI;} REG_UNUSED ca:SI deferring rescan insn with uid = 11. The C standard type promotion requirement and 64-bit return value are the root cause of such problem, I think. > >>> --- /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 -mdejagnu-cpu=power8" } */ >> >> Why does the testcase force power8? This testcase is not specific to >> Power8 or later. > > Yes, and we should generate the same code on older machines. > >>> +/* { dg-final { scan-assembler-not {\mextsw\M} } } */ >>> + >>> + >>> +unsigned long long negativeLessThan (unsigned long long a, unsigned long long b) >>> +{ >>> + return -(a < b); >>> +} >> >> If you're only testing for lp64, the testcase could use "long" instead >> of "long long". > > The testcase really needs "powerpc64", if that would mean "test if > -mpowerpc64 is (implicitly) used". But that is not what it currently > means (it is something akin to "powerpc64_hw", instead). > > So we test lp64, which is set if and only if -m64 was used. It is > reasonable coverage, no one cares much for -m32 -mpowerpc64 . > > > Segher
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 6ecb0bd6142..1d8b212962f 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -2358,6 +2358,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..94320f23423 --- /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 -mdejagnu-cpu=power8" } */ +/* { dg-final { scan-assembler-not {\mextsw\M} } } */ + + +unsigned long long negativeLessThan (unsigned long long a, unsigned long long b) +{ + return -(a < b); +}