> -----Original Message----- > From: Richard Sandiford <richard.sandif...@arm.com> > Sent: Friday, September 23, 2022 5:43 AM > To: Tamar Christina <tamar.christ...@arm.com> > Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; Richard Earnshaw > <richard.earns...@arm.com>; Marcus Shawcroft > <marcus.shawcr...@arm.com>; Kyrylo Tkachov <kyrylo.tkac...@arm.com> > Subject: Re: [PATCH 2/2]AArch64 Extend tbz pattern to allow SI to SI > extensions. > > Tamar Christina <tamar.christ...@arm.com> writes: > > Hi All, > > > > This adds additional recognition of & 1 into the tbz/tbnz pattern. > > > > Concretely with the mid-end changes this changes > > > > void g1(bool x) > > { > > if (__builtin_expect (x, 0)) > > h (); > > } > > > > from > > > > tst w0, 255 > > bne .L7 > > > > to > > > > tbnz w0, #0, .L5 > > > > This pattern occurs ~120,000 times in SPECCPU 20127, basically on > > every boolean comparison. > > > > 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): Renamed this > ... > > (*tb<optab><GPI2:mode><GPI:mode>1): ... To this. > > * config/aarch64/iterators.md (GPI2): 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 > > > 6aa1fb4be003f2027d63ac69fd314c2bbc876258..3faa03f453c94665d9d82225f18 > 0 > > d8afdcd0b5fe 100644 > > --- a/gcc/config/aarch64/aarch64.md > > +++ b/gcc/config/aarch64/aarch64.md > > @@ -943,31 +943,33 @@ (define_insn "*cb<optab><mode>1" > > (const_int 1)))] > > ) > > > > -(define_insn "*tb<optab><mode>1" > > +(define_insn "*tb<optab><GPI2:mode><GPI:mode>1" > > [(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")) > > + (EQL (zero_extract:GPI2 > > + (match_operand:GPI 0 "register_operand" "r") > > + (const_int 1) > > + (match_operand 1 > "aarch64_simd_shift_imm_<GPI:mode>" "n")) > > (const_int 0)) > > (label_ref (match_operand 2 "" "")) > > (pc))) > > (clobber (reg:CC CC_REGNUM))] > > - "!aarch64_track_speculation" > > + "!aarch64_track_speculation > > + && known_ge (GET_MODE_SIZE (<GPI2:MODE>mode), > > + GET_MODE_SIZE (<GPI:MODE>mode))" > > Is this check necessary? The extraction evaluates to 0 or 1, so it shouldn't > matter whether it is interpreted as DI or SI. >
Ah yes, fair point. I will remove the check, Thanks, Tamar. > OK without the check if you agree. > > Thanks, > Richard > > > { > > if (get_attr_length (insn) == 8) > > { > > if (get_attr_far_branch (insn) == 1) > > return aarch64_gen_far_branch (operands, 2, "Ltb", > > - "<inv_tb>\\t%<w>0, %1, "); > > + "<inv_tb>\\t%<GPI:w>0, %1, "); > > else > > { > > operands[1] = GEN_INT (HOST_WIDE_INT_1U << UINTVAL > (operands[1])); > > - return "tst\t%<w>0, %1\;<bcond>\t%l2"; > > + return "tst\t%<GPI:w>0, %1\;<bcond>\t%l2"; > > } > > } > > else > > - return "<tbz>\t%<w>0, %1, %l2"; > > + return "<tbz>\t%<GPI:w>0, %1, %l2"; > > } > > [(set_attr "type" "branch") > > (set (attr "length") > > diff --git a/gcc/config/aarch64/iterators.md > > b/gcc/config/aarch64/iterators.md index > > > 89ca66fd291b60a28979785706ecc5345ea86744..f6b2e7a83c63cab73947b6bd6 > 1b4 > > 99b4b57d14ac 100644 > > --- a/gcc/config/aarch64/iterators.md > > +++ b/gcc/config/aarch64/iterators.md > > @@ -28,6 +28,8 @@ (define_mode_iterator CCFP_CCFPE [CCFP CCFPE]) > > > > ;; Iterator for General Purpose Integer registers (32- and 64-bit > > modes) (define_mode_iterator GPI [SI DI]) > > +;; Copy of the above iterator > > +(define_mode_iterator GPI2 [SI DI]) > > > > ;; Iterator for HI, SI, DI, some instructions can only work on these modes. > > (define_mode_iterator GPI_I16 [(HI "AARCH64_ISA_F16") SI DI]) diff > > --git a/gcc/testsuite/gcc.target/aarch64/tbz_1.c > > b/gcc/testsuite/gcc.target/aarch64/tbz_1.c > > new file mode 100644 > > index > > > 0000000000000000000000000000000000000000..6a75eb4e7aedbfa3ae329358c > 6ee > > 4d675704a074 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/aarch64/tbz_1.c > > @@ -0,0 +1,32 @@ > > +/* { dg-do compile } */ > > +/* { dg-additional-options "-O2 -std=c99 -fno-unwind-tables > > +-fno-asynchronous-unwind-tables" } */ > > +/* { dg-final { check-function-bodies "**" "" "" { target { le } } } > > +} */ > > + > > +#include <stdbool.h>// Type your code here, or load an example. > > + > > +void h(void); > > + > > +/* > > +** g1: > > +** tbnz w[0-9], #?0, .L([0-9]+) > > +** ret > > +** ... > > +*/ > > +void g1(bool x) > > +{ > > + if (__builtin_expect (x, 0)) > > + h (); > > +} > > + > > +/* > > +** g2: > > +** tbz w[0-9]+, #?0, .L([0-9]+) > > +** b h > > +** ... > > +*/ > > +void g2(bool x) > > +{ > > + if (__builtin_expect (x, 1)) > > + h (); > > +} > > +