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 5CBFE3858C55; Wed, 10 Aug 2022 16:44:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5CBFE3858C55 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 27AGh7kw013030; Wed, 10 Aug 2022 11:43:07 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 27AGh6rw013029; Wed, 10 Aug 2022 11:43:06 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Wed, 10 Aug 2022 11:43:06 -0500 From: Segher Boessenkool To: Jiufu Guo Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com, linkw@gcc.gnu.org Subject: Re: [PATCH] rs6000: Enable generate const through pli+pli+rldimi Message-ID: <20220810164306.GY25951@gate.crashing.org> References: <20220810071123.165157-1-guojiufu@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220810071123.165157-1-guojiufu@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, 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: Wed, 10 Aug 2022 16:44:09 -0000 Hi! On Wed, Aug 10, 2022 at 03:11:23PM +0800, Jiufu Guo wrote: > As mentioned in PR106550, since pli could support 34bits immediate, we could > use less instructions(3insn would be ok) to build 64bits constant with pli. > > For example, for constant 0x020805006106003, we could generate it with: > asm code1: > pli 9,101736451 (0x6106003) > sldi 9,9,32 > paddi 9,9, 2130000 (0x0208050) > > or asm code2: > pli 10, 2130000 > pli 9, 101736451 > rldimi 9, 10, 32, 0 > > If there is only one register can be used, then the asm code1 is ok. Otherwise > asm code2 may be better. It is significantly better yes. That code with sldi is perhaps what we have to do after reload, but all those three insns are sequential, expensive. > This patch re-enable the constant building(splitter) before RA by updating the > constrains from int_reg_operand_not_pseudo to gpc_reg_operand. And then, we > could use two different pseduo for two pli(s), and asm code2 can be generated. > This patch also could generate asm code1 if hard register is allocated for the > constant. > + else if (TARGET_PREFIXED) > + { > + /* pli 9,high32 + pli 10,low32 + rldimi 9,10,32,0. */ > + if (can_create_pseudo_p ()) > + { > + temp = gen_reg_rtx (DImode); > + rtx temp1 = gen_reg_rtx (DImode); > + emit_move_insn (copy_rtx (temp), GEN_INT ((ud4 << 16) | ud3)); > + emit_move_insn (copy_rtx (temp1), GEN_INT ((ud2 << 16) | ud1)); > + > + rtx one = gen_rtx_AND (DImode, temp1, GEN_INT (0xffffffff)); Why do you meed to mask the value here? That is a nop, no? > + rtx two = gen_rtx_ASHIFT (DImode, temp, GEN_INT (32)); > + emit_move_insn (dest, gen_rtx_IOR (DImode, one, two)); But you can call gen_rotldi3_insert_3 explicitly, a better idea if this code can run late (so we cannot rely on other optimisations to clean things up). > --- a/gcc/config/rs6000/rs6000.md > +++ b/gcc/config/rs6000/rs6000.md > @@ -9659,7 +9659,7 @@ (define_split > ;; When non-easy constants can go in the TOC, this should use > ;; easy_fp_constant predicate. > (define_split > - [(set (match_operand:DI 0 "int_reg_operand_not_pseudo") > + [(set (match_operand:DI 0 "gpc_reg_operand") > (match_operand:DI 1 "const_int_operand"))] > "TARGET_POWERPC64 && num_insns_constant (operands[1], DImode) > 1" > [(set (match_dup 0) (match_dup 2)) This is a huge change. Do you have some indication that it helps / hurts / is neutral? Some reasoning why it is a good idea? I am not against it, but some more rationale would be good :-) Btw, this splitter uses operands[2] and [3] in the replacement, and neither of those exists. The replacement never is used of course. Instead, rs6000_emit_set_const is called always. It would be less misleading if the replacement text was just "(pc)" or such. Segher