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 6FEAE3857828 for ; Tue, 15 Sep 2020 19:55:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 6FEAE3857828 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=segher@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 08FJsosg000889; Tue, 15 Sep 2020 14:54:50 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 08FJsoZR000887; Tue, 15 Sep 2020 14:54:50 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Tue, 15 Sep 2020 14:54:50 -0500 From: Segher Boessenkool To: Peter Bergner Cc: GCC Patches , Bill Schmidt , Alan Modra Subject: Re: [PATCH] rs6000: inefficient 64-bit constant generation for consecutive 1-bits Message-ID: <20200915195450.GT28786@gate.crashing.org> References: <838b2e97-dfa9-3ca0-c3c6-1767d60ddf05@linux.ibm.com> <20200915135601.GK28786@gate.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, KAM_SHORT, KAM_TK, TXREP, T_SPF_HELO_PERMERROR, T_SPF_PERMERROR autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Tue, 15 Sep 2020 19:55:52 -0000 On Tue, Sep 15, 2020 at 10:48:37AM -0500, Peter Bergner wrote: > > rs6000_is_valid_shift_mask handles this already (but it requires you to > > pass in the shift needed). rs6000_is_valid_mask will handle it. > > rs6000_is_valid_and_mask does not get a shift count parameter, so cannot > > use rldic currently. > > After talking with you off line, I changed to using rs6000_is_valid_mask. > The did mean I had to change num_insns_constant_gpr to take a mode param > so it could be passed down to rs6000_is_valid_mask. All its callers have on readily available, so that will work fine. > >> +(define_insn "rldic" > >> + [(set (match_operand:DI 0 "gpc_reg_operand" "=r") > >> + (unspec:DI [(match_operand:DI 1 "gpc_reg_operand" "r") > >> + (match_operand:DI 2 "u6bit_cint_operand" "n") > >> + (match_operand:DI 3 "u6bit_cint_operand" "n")] > >> + UNSPEC_RLDIC))] > >> + "TARGET_POWERPC64" > >> + "rldic %0,%1,%2,%3") > > ...and this is gone too. I've replaced it with a generic splitter > that matches an already existing define_insn (rotl3_mask). That define_insn always does a *single* machine instruction (just like most of our define_insns). > >> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,8,8" } } */ > >> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,24,8" } } */ > >> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,40,8" } } */ > >> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,40,48" } } */ > >> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,40,23" } } */ > > > > Please use {} quotes, and \m\M. \d can be helpful, too. > > That was how I wrote it initially, but for some reason, it wouldn't match > at all. Do I need extra \'s for my regexs when using {}? No, you don't need any here. You only need to use \[ inside double quotes because [ has a special meaning in double quotes! (command substitution.) > \d is any digit? Yeah, that would be better. Gotta find a manpage or ??? > that describes what regex patterns are allowed. "man re_syntax", and "man tcl" for the one-page tcl intro (it describes the whole language: the substitutions, the quotes, etc.) https://www.tcl.tk/man/tcl8.6/TclCmd/re_syntax.htm https://www.tcl.tk/man/tcl8.6/TclCmd/Tcl.htm > This all said, Alan's rtx_costs patch touches this same area and he talked > about removing a similar splitter, so I think I will wait for his code to > be committed and then rework this on top of his changes. Yes, good plan. Thanks! Segher