From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id BD59C3857354 for ; Fri, 30 Sep 2022 08:48:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BD59C3857354 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 249EC15A1; Fri, 30 Sep 2022 01:49:03 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9CCAA3F73D; Fri, 30 Sep 2022 01:48:55 -0700 (PDT) From: Richard Sandiford To: Tamar Christina Mail-Followup-To: Tamar Christina ,Richard Biener , Tamar Christina via Gcc-patches , nd , Jeff Law , richard.sandiford@arm.com Cc: Richard Biener , Tamar Christina via Gcc-patches , nd , Jeff Law Subject: Re: [PATCH 1/2]middle-end: RFC: On expansion of conditional branches, give hint if argument is a truth type to backend References: <8873DC9F-F868-458D-9AD6-90DDC5465057@suse.de> Date: Fri, 30 Sep 2022 09:48:54 +0100 In-Reply-To: (Tamar Christina's message of "Fri, 30 Sep 2022 08:38:12 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-39.4 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP 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: Tamar Christina writes: >> -----Original Message----- >> From: Richard Sandiford >> Sent: Friday, September 30, 2022 9:29 AM >> To: Tamar Christina >> Cc: Richard Biener ; Tamar Christina via Gcc-patches >> ; nd ; Jeff Law >> >> Subject: Re: [PATCH 1/2]middle-end: RFC: On expansion of conditional >> branches, give hint if argument is a truth type to backend >>=20 >> Tamar Christina writes: >> >> -----Original Message----- >> >> From: Gcc-patches > >> bounces+tamar.christina=3Darm.com@gcc.gnu.org> On Behalf Of Richard >> >> Biener via Gcc-patches >> >> Sent: Thursday, September 29, 2022 12:09 PM >> >> To: Tamar Christina via Gcc-patches >> >> Cc: Richard Sandiford ; nd >> >> Subject: Re: [PATCH 1/2]middle-end: RFC: On expansion of conditional >> >> branches, give hint if argument is a truth type to backend >> >> >> >> >> >> >> >> > Am 29.09.2022 um 12:23 schrieb Tamar Christina via Gcc-patches >> >> > > >> patches@gcc.gnu.org>: >> >> > >> >> > >> >> >> >> >> >> -----Original Message----- >> >> >> From: Richard Biener >> >> >> Sent: Thursday, September 29, 2022 10:41 AM >> >> >> To: Richard Sandiford >> >> >> Cc: Jeff Law ; Tamar Christina >> >> >> ; gcc-patches@gcc.gnu.org; nd >> >> >> >> >> Subject: Re: [PATCH 1/2]middle-end: RFC: On expansion of >> >> >> conditional branches, give hint if argument is a truth type to >> >> >> backend >> >> >> >> >> >>> On Thu, 29 Sep 2022, Richard Sandiford wrote: >> >> >>> >> >> >>> Jeff Law writes: >> >> >>>> On 9/28/22 09:04, Richard Sandiford wrote: >> >> >>>>> Tamar Christina writes: >> >> >>>>>>> Maybe the target could use (subreg:SI (reg:BI ...)) as argume= nt. >> >> Heh. >> >> >>>>>> But then I'd still need to change the expansion code. I >> >> >>>>>> suppose this could prevent the issue with changes to code on >> other targets. >> >> >>>>>> >> >> >>>>>>>>> We have undocumented addcc, negcc, etc. patterns, should >> we >> >> >>>>>>>>> have aandcc >> >> >>>>>> pattern for this indicating support for andcc + jump as >> >> >>>>>> opposedto >> >> >> cmpcc + jump? >> >> >>>>>>>> This could work yeah. I didn't know these existed. >> >> >>>>>>> Ah, so they are conditional add, not add setting CC, so andcc >> >> >>>>>>> wouldn't be appropriate. >> >> >>>>>>> So I'm not sure how we'd handle such situation - maybe >> >> >>>>>>> looking at REG_DECL and recognizing a _Bool PARM_DECL is OK? >> >> >>>>>> I have a slight suspicion that Richard Sandiford would likely >> >> >>>>>> reject this though.. >> >> >>>>> Good guess :-P We shouldn't rely on something like that for >> >> >> correctness. >> >> >>>>> >> >> >>>>> Would it help if we promoted the test-and-branch instructions >> >> >>>>> to optabs, alongside cbranch? The jump expanders could then >> >> >>>>> target it >> >> >> directly. >> >> >>>>> >> >> >>>>> IMO that'd be a reasonable thing to do if it does help. It's a >> >> >>>>> relatively common operation, especially on CISCy targets. >> >> >>>> >> >> >>>> But don't we represent these single bit tests using zero_extract >> >> >>>> as the condition of the branch? I guess if we can generate them >> >> >>>> directly rather than waiting for combine to deduce that we're >> >> >>>> dealing with a single bit test and constructing the zero_extract >> >> >>>> form would be an improvement and might help aarch at the same >> time. >> >> >>> >> >> >>> Do you mean that the promote_mode stuff should use ext(z)v rather >> >> >>> than zero_extend to promote a bool, where available? If so, I >> >> >>> agree that might help. But it sounds like it would have downsides >> too. >> >> >>> Currently a bool memory can be zero-extended on the fly using a >> >> >>> load, but if we used the zero_extract form instead, we'd have to >> >> >>> extract the bit after the load. And (as an alternative) choosing >> >> >>> different behaviour based on whether expand sees a REG or a MEM >> >> >>> sounds like it could still cause problems, since REGs could be >> >> >>> replaced by MEMs (or vice versa) later in the RTL passes. >> >> >>> >> >> >>> ISTM that the original patch was inserting an extra operation in >> >> >>> the branch expansion in order to target a specific instruction. >> >> >>> Targeting the instruction in expand seems good, but IMO we should >> >> >>> do it directly, based on knowledge of whether the instruction >> >> >>> actually >> >> exists. >> >> >> >> >> >> Yes, I think a compare-and-branch pattern is the best fit here. >> >> >> Note on GIMPLE we'd rely on the fact this is a BOOLEAN_TYPE (so >> >> >> even 8 bit precision bools only have 1 and 0 as meaningful values). >> >> >> So the 'compare-' bit in compare-and-branch would be interpreting >> >> >> a BOOLEAN_TYPE, not so much a general compare. >> >> > >> >> > Oh, I was thinking of adding a constant argument representing the >> >> > precision that is relevant for the compare in order to make this a >> >> > bit more >> >> general/future proof. >> >> > >> >> > Are you thinking I should instead just make the optab implicitly >> >> > only work for 1-bit precision comparisons? >> >> >> >> What=E2=80=99s the optab you propose (cite also the documentation par= t)? >> > >> > tbranchmode5 >> > Conditional branch instruction combined with a bit test instruction. >> Operand 0 is a comparison operator. >> > Operand 1 and Operand 2 are the first and second operands of the >> comparison, respectively. >> > Operand 3 is the number of low-order bits that are relevant for the >> comparison. >> > Operand 4 is the code_label to jump to. >>=20 >> For the TB instructions (and for other similar instructions that I've se= en on >> other architectures) it would be more useful to have a single-bit test, = with >> operand 4 specifying the bit position. Arguably it might then be better= to >> have separate eq and ne optabs, to avoid the awkward doubling of the >> operands (operand 1 contains operands 2 and 3). >>=20 >> I guess a more general way of achieving the same thing would be to make >> operand 4 in the optab above a mask rather than a bit count. But that m= ight >> be overly general, if there are no known architectures that have such an >> instruction. > > One of the reasons I wanted a range rather than a single bit is that I ca= n the use > this to generate cbz/cbnz early on as well. We already have the opportunity to do that via cbranch4. But at the moment aarch64.md always forces the separate comparison instead. (Not sure why TBH. Does it enable more ifcvt opportunities?) If we change the body of cbranch4 to: if ((GET_CODE (operands[0]) !=3D EQ && GET_CODE (operands[0]) !=3D NE) || operands[2] !=3D const0_rtx) { operands[1] =3D aarch64_gen_compare_reg (GET_CODE (operands[0]), operands[1], operands[2]); operands[2] =3D const0_rtx; } then we generate the cbz/cbnz directly. Thanks, Richard > This would mean we could use my earlier > patch that tried to drop the QI/HI promotions without needing the any_ext= end additional > pass if we wanted to. > > We'd also no longer need to rely on seeing a paradoxical subreg for a tst. > > Tamar. > >>=20 >> Thanks, >> Richard >>=20 >> > Specifically this representation would allow us to emit all our >> > different conditional branching instructions without needing to rely >> > on combine. We have some cases that happen during optimization that >> > sometimes prevent the optimal sequence from being generated. This >> would also solve that as we would expand to what we want to start with. >> > >> > Tamar. >> > >> >> >> >> > >> >> > Thanks, >> >> > Tamar >> >> > >> >> >> >> >> >> Richard.