Tamar Christina <tamar.christ...@arm.com> writes: >> -----Original Message----- >> From: Richard Sandiford <richard.sandif...@arm.com> >> Sent: Tuesday, November 15, 2022 10:51 AM >> To: Tamar Christina <tamar.christ...@arm.com> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw >> <richard.earns...@arm.com>; nd <n...@arm.com>; Marcus Shawcroft >> <marcus.shawcr...@arm.com> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab. >> >> Tamar Christina <tamar.christ...@arm.com> writes: >> >> -----Original Message----- >> >> From: Richard Sandiford <richard.sandif...@arm.com> >> >> Sent: Tuesday, November 15, 2022 10:36 AM >> >> To: Tamar Christina <tamar.christ...@arm.com> >> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw >> >> <richard.earns...@arm.com>; nd <n...@arm.com>; Marcus Shawcroft >> >> <marcus.shawcr...@arm.com> >> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab. >> >> >> >> Tamar Christina <tamar.christ...@arm.com> 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 (*tb<optab><mode>1): Rename to... >> >> > (*tb<optab><ALLI:mode><GPI:mode>1): ... this. >> >> > (tbranch<mode>4): 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 "*cb<optab><mode>1" >> >> > (const_int 1)))] >> >> > ) >> >> > >> >> > -(define_insn "*tb<optab><mode>1" >> >> > +(define_expand "tbranch<mode>4" >> >> > [(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_<mode>" >> >> > "n")) >> >> > + (match_operator 0 "aarch64_comparison_operator" >> >> > + [(match_operand:ALLI 1 "register_operand") >> >> > + (match_operand:ALLI 2 >> >> "aarch64_simd_shift_imm_<ALLI:mode>")]) >> >> > + (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 ☹ ok if I'm going to fix that bug then do I need 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?
What rtx does the code that uses the optab pass for operand 0? Richard
