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 F313B3894C1E for ; Tue, 15 Nov 2022 11:33:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F313B3894C1E 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 EBBDF13D5; Tue, 15 Nov 2022 03:33:42 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D1AA93F73B; Tue, 15 Nov 2022 03:33:35 -0800 (PST) From: Richard Sandiford To: Tamar Christina Mail-Followup-To: Tamar Christina ,"gcc-patches\@gcc.gnu.org" , Richard Earnshaw , nd , Marcus Shawcroft , richard.sandiford@arm.com Cc: "gcc-patches\@gcc.gnu.org" , Richard Earnshaw , nd , Marcus Shawcroft Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab. References: Date: Tue, 15 Nov 2022 11:33:34 +0000 In-Reply-To: (Tamar Christina's message of "Tue, 15 Nov 2022 11:23:24 +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=-40.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham 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: Tuesday, November 15, 2022 11:15 AM >> To: Tamar Christina >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw >> ; nd ; Marcus Shawcroft >> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab. >>=20 >> Tamar Christina writes: >> >> -----Original Message----- >> >> From: Richard Sandiford >> >> Sent: Tuesday, November 15, 2022 10:51 AM >> >> To: Tamar Christina >> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw >> >> ; nd ; Marcus Shawcroft >> >> >> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab. >> >> >> >> Tamar Christina writes: >> >> >> -----Original Message----- >> >> >> From: Richard Sandiford >> >> >> Sent: Tuesday, November 15, 2022 10:36 AM >> >> >> To: Tamar Christina >> >> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw >> >> >> ; nd ; Marcus >> Shawcroft >> >> >> >> >> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab. >> >> >> >> >> >> Tamar Christina writes: >> >> >> > Hello, >> >> >> > >> >> >> > Ping and updated patch. >> >> >> > >> >> >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. >> >> >> > >> >> >> > Ok for master? >> >> >> > >> >> >> > Thanks, >> >> >> > Tamar >> >> >> > >> >> >> > gcc/ChangeLog: >> >> >> > >> >> >> > * config/aarch64/aarch64.md (*tb1): Rename >> to... >> >> >> > (*tb1): ... this. >> >> >> > (tbranch4): New. >> >> >> > >> >> >> > gcc/testsuite/ChangeLog: >> >> >> > >> >> >> > * gcc.target/aarch64/tbz_1.c: New test. >> >> >> > >> >> >> > --- inline copy of patch --- >> >> >> > >> >> >> > diff --git a/gcc/config/aarch64/aarch64.md >> >> >> > b/gcc/config/aarch64/aarch64.md index >> >> >> > >> >> >> >> >> >> 2bc2684b82c35a44e0a2cea6e3aaf32d939f8cdf..d7684c93fba5b717d568e1a4fd >> >> >> 71 >> >> >> > 2bde55c7c72e 100644 >> >> >> > --- a/gcc/config/aarch64/aarch64.md >> >> >> > +++ b/gcc/config/aarch64/aarch64.md >> >> >> > @@ -943,12 +943,29 @@ (define_insn "*cb1" >> >> >> > (const_int 1)))] >> >> >> > ) >> >> >> > >> >> >> > -(define_insn "*tb1" >> >> >> > +(define_expand "tbranch4" >> >> >> > [(set (pc) (if_then_else >> >> >> > - (EQL (zero_extract:DI (match_operand:GPI 0 >> "register_operand" >> >> >> "r") >> >> >> > - (const_int 1) >> >> >> > - (match_operand 1 >> >> >> > - "aarch64_simd_shift_imm_" "n")) >> >> >> > + (match_operator 0 "aarch64_comparison_operator" >> >> >> > + [(match_operand:ALLI 1 "register_operand") >> >> >> > + (match_operand:ALLI 2 >> >> >> "aarch64_simd_shift_imm_")]) >> >> >> > + (label_ref (match_operand 3 "" "")) >> >> >> > + (pc)))] >> >> >> > + "optimize > 0" >> >> >> >> >> >> Why's the pattern conditional on optimize? Seems a valid choice >> >> >> at -O0 >> >> too. >> >> >> >> >> > >> >> > Hi, >> >> > >> >> > I had explained the reason why in the original patch, just didn't >> >> > repeat it in >> >> the ping: >> >> > >> >> > Instead of emitting the instruction directly I've chosen to expand >> >> > the pattern using a zero extract and generating the existing >> >> > pattern for comparisons for two >> >> > reasons: >> >> > >> >> > 1. Allows for CSE of the actual comparison. >> >> > 2. It looks like the code in expand makes the label as unused and >> >> > removed >> >> it >> >> > if it doesn't see a separate reference to it. >> >> > >> >> > Because of this expansion though I disable the pattern at -O0 since >> >> > we >> >> have no combine in that case so we'd end up with worse code. I did >> >> try emitting the pattern directly, but as mentioned in no#2 expand >> >> would then kill the label. >> >> > >> >> > Basically I emit the pattern directly, immediately during expand >> >> > the label is >> >> marked as dead for some weird reason. >> >> >> >> Isn't #2 a bug though? It seems like something we should fix rather >> >> than work around. >> > >> > Yes it's a bug =E2=98=B9 ok if I'm going to fix that bug then do I nee= d to >> > split the optabs still? Isn't the problem atm that I need the split? >> > If I'm emitting the instruction directly then the recog pattern for it >> > can just be (eq (vec_extract x 1) 0) which is the correct semantics? >>=20 >> What rtx does the code that uses the optab pass for operand 0? > > It gets passed the full comparison: > > (eq (reg/v:SI 92 [ x ]) > (const_int 0 [0])) > > of which we only look at the operator. OK, that's what I thought. The problem is then the one I mentioned above. This rtx doesn't describe the operation that the optab is supposed to perform, so it can never be used in the instruction pattern. (This is different from something like cbranch, where operand 0 can be used directly if the target supports a very general compare-and-branch instruction.) If we want to use a single optab, the code that generates the optab should pass something like: (eq/ne (zero_extract op0 (const_int 1) op1) (const_int 0)) as operand 0, so that operand 0 specifies the real test condition. Thanks, Richard