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 24F2C3858C33; Wed, 24 Aug 2022 14:08:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 24F2C3858C33 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 27OE7N9w022010; Wed, 24 Aug 2022 09:07:23 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 27OE7NvW022009; Wed, 24 Aug 2022 09:07:23 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Wed, 24 Aug 2022 09:07:22 -0500 From: Segher Boessenkool To: Jiufu Guo Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com, linkw@gcc.gnu.org Subject: Re: [PATCH V4] rs6000: Optimize cmp on rotated 16bits constant Message-ID: <20220824140722.GZ25951@gate.crashing.org> References: <20220725132922.45470-1-guojiufu@linux.ibm.com> <20220823221854.GX25951@gate.crashing.org> <7er116gjz2.fsf@pike.rch.stglabs.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7er116gjz2.fsf@pike.rch.stglabs.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 List-Id: On Wed, Aug 24, 2022 at 03:48:49PM +0800, Jiufu Guo wrote: > Segher Boessenkool writes: > >> + "TARGET_POWERPC64 && !reload_completed && can_create_pseudo_p () > > > > reload_completed in splitters is almost always wrong. It isn't any > > better if it is in the insn condition of a define_insn_and_split :-) > > > Thanks, 'can_create_pseudo_p' would be ok for this patch. > Or just FAIL, if !can_create_pseudo_p()? You usually can split fine if you cannot create new pseudos, by reusing existing registers. FAIL will cause an ICE: the RTL instruction does match, but will fail when trying to generate machine code for it. > >> + && num_insns_constant (operands[2], DImode) > 1 > >> + && (rotate_from_leading_zeros_const (~UINTVAL (operands[2]), 49) > 0 > >> + || rotate_from_leading_zeros_const (UINTVAL (operands[2]), 48) > 0)" > > There must be a better way to describe this. > Will update this. I'm thinking to replace this with a meaning function, > maybe 'compare_rotate_immediate_p'. Thanks! > > Why is this doing a conditional branch at all? Unpredictable > > conditional branches are extremely costly. > This optimization needs to check whether the comparison code is ne/eq or > not. To get the comparison code, we need to check the parent insn of > the 'cmp' insn. This is why conditional branch patterns in used here. > > This patch should not change the information (about prediction) of the > branch insn. I'm thinking of updating the patch to keep the 'note info > REG_BR_PROB' for the branch instruction. Ah, good. Explain a bit about that? In a code comment or in the commit message, whichever works best here. Thanks! Segher