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 F0281383579B for ; Tue, 15 Nov 2022 11:14:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F0281383579B 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 EA57213D5; Tue, 15 Nov 2022 03:14:57 -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 D04E53F73B; Tue, 15 Nov 2022 03:14:50 -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:14:49 +0000 In-Reply-To: (Tamar Christina's message of "Tue, 15 Nov 2022 11:00:01 +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=-41.0 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 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. >>=20 >> 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 repe= at 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 re= moved >> 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 l= abel is >> marked as dead for some weird reason. >>=20 >> 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 need t= o 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? What rtx does the code that uses the optab pass for operand 0? Richard