From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id F1CDE395B474 for ; Fri, 13 May 2022 13:22:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F1CDE395B474 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 24DDL100004381; Fri, 13 May 2022 08:21:01 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 24DDL0TM004380; Fri, 13 May 2022 08:21:00 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Fri, 13 May 2022 08:21:00 -0500 From: Segher Boessenkool To: HAO CHEN GUI Cc: gcc-patches , David , "Kewen.Lin" , Peter Bergner Subject: Re: [PATCH v4, rs6000] Add a combine pattern for CA minus one [PR95737] Message-ID: <20220513132100.GV25951@gate.crashing.org> References: <243c7199-e6a5-f04b-568c-64024590695f@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <243c7199-e6a5-f04b-568c-64024590695f@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no 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: , X-List-Received-Date: Fri, 13 May 2022 13:22:03 -0000 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 subf3_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