> > On 6/28/24 6:18 AM, Pengxuan Zheng wrote:
> > > This patch improves GCC’s vectorization of __builtin_popcount for
> > > aarch64 target by adding popcount patterns for vector modes besides
> > > QImode, i.e., HImode, SImode and DImode.
> > >
> > > With this patch, we now generate the following for V8HI:
> > > cnt v1.16b, v0.16b
> > > uaddlp v2.8h, v1.16b
> > >
> > > For V4HI, we generate:
> > > cnt v1.8b, v0.8b
> > > uaddlp v2.4h, v1.8b
> > >
> > > For V4SI, we generate:
> > > cnt v1.16b, v0.16b
> > > uaddlp v2.8h, v1.16b
> > > uaddlp v3.4s, v2.8h
> > >
> > > For V4SI with TARGET_DOTPROD, we generate the following instead:
> > > movi v0.4s, #0
> > > movi v1.16b, #1
> > > cnt v3.16b, v2.16b
> > > udot v0.4s, v3.16b, v1.16b
> > >
> > > For V2SI, we generate:
> > > cnt v1.8b, v.8b
> > > uaddlp v2.4h, v1.8b
> > > uaddlp v3.2s, v2.4h
> > >
> > > For V2SI with TARGET_DOTPROD, we generate the following instead:
> > > movi v0.8b, #0
> > > movi v1.8b, #1
> > > cnt v3.8b, v2.8b
> > > udot v0.2s, v3.8b, v1.8b
> > >
> > > For V2DI, we generate:
> > > cnt v1.16b, v.16b
> > > uaddlp v2.8h, v1.16b
> > > uaddlp v3.4s, v2.8h
> > > uaddlp v4.2d, v3.4s
> > >
> > > For V4SI with TARGET_DOTPROD, we generate the following instead:
> > > movi v0.4s, #0
> > > movi v1.16b, #1
> > > cnt v3.16b, v2.16b
> > > udot v0.4s, v3.16b, v1.16b
> > > uaddlp v0.2d, v0.4s
> > >
> > > PR target/113859
> > >
> > > gcc/ChangeLog:
> > >
> > > * config/aarch64/aarch64-simd.md (aarch64_<su>addlp<mode>):
> > Rename to...
> > > (@aarch64_<su>addlp<mode>): ... This.
> > > (popcount<mode>2): New define_expand.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.target/aarch64/popcnt-udot.c: New test.
> > > * gcc.target/aarch64/popcnt-vec.c: New test.
> > >
> > > Signed-off-by: Pengxuan Zheng <quic_pzh...@quicinc.com>
> > > ---
> > > gcc/config/aarch64/aarch64-simd.md | 41 ++++++++++-
> > > .../gcc.target/aarch64/popcnt-udot.c | 58 ++++++++++++++++
> > > gcc/testsuite/gcc.target/aarch64/popcnt-vec.c | 69
> +++++++++++++++++++
> > > 3 files changed, 167 insertions(+), 1 deletion(-)
> > > create mode 100644 gcc/testsuite/gcc.target/aarch64/popcnt-udot.c
> > > create mode 100644 gcc/testsuite/gcc.target/aarch64/popcnt-vec.c
> > >
> > > diff --git a/gcc/config/aarch64/aarch64-simd.md
> > > b/gcc/config/aarch64/aarch64-simd.md
> > > index 01b084d8ccb..afdf3ec7873 100644
> > > --- a/gcc/config/aarch64/aarch64-simd.md
> > > +++ b/gcc/config/aarch64/aarch64-simd.md
> > > @@ -3461,7 +3461,7 @@ (define_insn
> > "*aarch64_<su>addlv<VDQV_L:mode>_ze<GPI:mode>"
> > > [(set_attr "type" "neon_reduc_add<VDQV_L:q>")]
> > > )
> > >
> > > -(define_expand "aarch64_<su>addlp<mode>"
> > > +(define_expand "@aarch64_<su>addlp<mode>"
> > > [(set (match_operand:<VDBLW> 0 "register_operand")
> > > (plus:<VDBLW>
> > > (vec_select:<VDBLW>
> > > @@ -3517,6 +3517,45 @@ (define_insn
> > "popcount<mode>2<vczle><vczbe>"
> > > [(set_attr "type" "neon_cnt<q>")]
> > > )
> > >
> > > +(define_expand "popcount<mode>2"
> > > + [(set (match_operand:VDQHSD 0 "register_operand")
> > > + (popcount:VDQHSD (match_operand:VDQHSD 1
> > > +"register_operand")))]
> > > + "TARGET_SIMD"
> > > + {
> > > + /* Generate a byte popcount. */
> >
> > A couple of formatting nits. Two spaces before end of comment.
>
> I noticed this in other places, but didn't realize it's intentional. Glad you
> pointed this out!
>
> >
> > > + machine_mode mode = <bitsize> == 64 ? V8QImode : V16QImode;
> > > + rtx tmp = gen_reg_rtx (mode);
> > > + auto icode = optab_handler (popcount_optab, mode);
> > > + emit_insn (GEN_FCN (icode) (tmp, gen_lowpart (mode,
> > > + operands[1])));
> > > +
> > > + if (TARGET_DOTPROD
> > > + && (<VEL>mode == SImode || <VEL>mode == DImode))
> > > + {
> > > + /* For V4SI and V2SI, we can generate a UDOT with a 0
> > > + accumulator
> > and a
> > > + 1 multiplicand. For V2DI, another UAADDLP is needed. */
> >
> > Likewise.
> >
> > > + rtx ones = force_reg (mode, CONST1_RTX (mode));
> > > + auto icode = optab_handler (udot_prod_optab, mode);
> > > + mode = <bitsize> == 64 ? V2SImode : V4SImode;
> > > + rtx dest = mode == <MODE>mode ? operands[0] : gen_reg_rtx
> > (mode);
> > > + rtx zeros = force_reg (mode, CONST0_RTX (mode));
> > > + emit_insn (GEN_FCN (icode) (dest, tmp, ones, zeros));
> > > + tmp = dest;
> > > + }
> > > +
> > > + /* Use a sequence of UADDLPs to accumulate the counts. Each
> > > + step
> > doubles
> > > + the element size and halves the number of elements. */
> >
> > Likewise. Also two spaces after the dot before a new sentence.
> >
> > You could run your patch through gcc/contrib/check_GNU_style.sh to
> > check for formatting nits.
>
> Thanks for the info, Tejas. I just tried running
> gcc/contrib/check_GNU_style.sh
> on the file I changed, but it didn't seem to warn this. Maybe I am not using
> it
> correctly?
Just realized that the script should be run on the patch file instead. I've
fixed a few more tab/space warnings caught by the script. Here's the latest
version.
https://gcc.gnu.org/pipermail/gcc-patches/2024-June/655994.html
Thanks,
Pengxuan
>
> Anyway, here's the updated version. Please let me know if you notice anything
> else.
> https://gcc.gnu.org/pipermail/gcc-patches/2024-June/655991.html
>
> Thanks,
> Pengxuan
> >
> > Thanks,
> > Tejas.
> >
> > > + while (mode != <MODE>mode)
> > > + {
> > > + auto icode = code_for_aarch64_addlp (ZERO_EXTEND, GET_MODE
> > (tmp));
> > > + mode = insn_data[icode].operand[0].mode;
> > > + rtx dest = mode == <MODE>mode ? operands[0] : gen_reg_rtx
> > (mode);
> > > + emit_insn (GEN_FCN (icode) (dest, tmp));
> > > + tmp = dest;
> > > + }
> > > + DONE;
> > > + }
> > > +)
> > > +
> > > ;; 'across lanes' max and min ops.
> > >
> > > ;; Template for outputting a scalar, so we can create __builtins
> > > which can be diff --git
> > > a/gcc/testsuite/gcc.target/aarch64/popcnt-udot.c
> > > b/gcc/testsuite/gcc.target/aarch64/popcnt-udot.c
> > > new file mode 100644
> > > index 00000000000..f6a968dae95
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/aarch64/popcnt-udot.c
> > > @@ -0,0 +1,58 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2 -march=armv8.2-a+dotprod -fno-vect-cost-model
> > > +-fno-schedule-insns -fno-schedule-insns2" } */
> > > +
> > > +/*
> > > +** bar:
> > > +** movi v([0-9]+).16b, 0x1
> > > +** movi v([0-9]+).4s, 0
> > > +** ldr q([0-9]+), \[x0\]
> > > +** cnt v([0-9]+).16b, v\3.16b
> > > +** udot v\2.4s, v\4.16b, v\1.16b
> > > +** str q\2, \[x1\]
> > > +** ret
> > > +*/
> > > +void
> > > +bar (unsigned int *__restrict b, unsigned int *__restrict d) {
> > > + d[0] = __builtin_popcount (b[0]);
> > > + d[1] = __builtin_popcount (b[1]);
> > > + d[2] = __builtin_popcount (b[2]);
> > > + d[3] = __builtin_popcount (b[3]); }
> > > +
> > > +/*
> > > +** bar1:
> > > +** movi v([0-9]+).8b, 0x1
> > > +** movi v([0-9]+).2s, 0
> > > +** ldr d([0-9]+), \[x0\]
> > > +** cnt v([0-9]+).8b, v\3.8b
> > > +** udot v\2.2s, v\4.8b, v\1.8b
> > > +** str d\2, \[x1\]
> > > +** ret
> > > +*/
> > > +void
> > > +bar1 (unsigned int *__restrict b, unsigned int *__restrict d) {
> > > + d[0] = __builtin_popcount (b[0]);
> > > + d[1] = __builtin_popcount (b[1]); }
> > > +
> > > +/*
> > > +** bar2:
> > > +** movi v([0-9]+).16b, 0x1
> > > +** movi v([0-9]+).4s, 0
> > > +** ldr q([0-9]+), \[x0\]
> > > +** cnt v([0-9]+).16b, v\3.16b
> > > +** udot v\2.4s, v\4.16b, v\1.16b
> > > +** uaddlp v\2.2d, v\2.4s
> > > +** str q\2, \[x1\]
> > > +** ret
> > > +*/
> > > +void
> > > +bar2 (unsigned long long *__restrict b, unsigned long long
> > > +*__restrict d) {
> > > + d[0] = __builtin_popcountll (b[0]);
> > > + d[1] = __builtin_popcountll (b[1]); }
> > > +
> > > +/* { dg-final { check-function-bodies "**" "" "" } } */
> > > diff --git a/gcc/testsuite/gcc.target/aarch64/popcnt-vec.c
> > > b/gcc/testsuite/gcc.target/aarch64/popcnt-vec.c
> > > new file mode 100644
> > > index 00000000000..0c4926d7ca8
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/aarch64/popcnt-vec.c
> > > @@ -0,0 +1,69 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2 -fno-vect-cost-model" } */
> > > +
> > > +/* This function should produce cnt v.16b. */ void bar (unsigned
> > > +char *__restrict b, unsigned char *__restrict d) {
> > > + for (int i = 0; i < 1024; i++)
> > > + d[i] = __builtin_popcount (b[i]); }
> > > +
> > > +/* This function should produce cnt v.16b and uaddlp (Add Long
> > > +Pairwise). */ void
> > > +bar1 (unsigned short *__restrict b, unsigned short *__restrict d) {
> > > + for (int i = 0; i < 1024; i++)
> > > + d[i] = __builtin_popcount (b[i]); }
> > > +
> > > +/* This function should produce cnt v.16b and 2 uaddlp (Add Long
> > > +Pairwise). */ void
> > > +bar2 (unsigned int *__restrict b, unsigned int *__restrict d) {
> > > + for (int i = 0; i < 1024; i++)
> > > + d[i] = __builtin_popcount (b[i]); }
> > > +
> > > +/* This function should produce cnt v.16b and 3 uaddlp (Add Long
> > > +Pairwise). */ void
> > > +bar3 (unsigned long long *__restrict b, unsigned long long
> > > +*__restrict d) {
> > > + for (int i = 0; i < 1024; i++)
> > > + d[i] = __builtin_popcountll (b[i]); }
> > > +
> > > +/* SLP
> > > + This function should produce cnt v.8b and uaddlp (Add Long
> > > +Pairwise). */ void
> > > +bar4 (unsigned short *__restrict b, unsigned short *__restrict d) {
> > > + d[0] = __builtin_popcount (b[0]);
> > > + d[1] = __builtin_popcount (b[1]);
> > > + d[2] = __builtin_popcount (b[2]);
> > > + d[3] = __builtin_popcount (b[3]); }
> > > +
> > > +/* SLP
> > > + This function should produce cnt v.8b and 2 uaddlp (Add Long
> > > +Pairwise). */ void
> > > +bar5 (unsigned int *__restrict b, unsigned int *__restrict d) {
> > > + d[0] = __builtin_popcount (b[0]);
> > > + d[1] = __builtin_popcount (b[1]); }
> > > +
> > > +/* SLP
> > > + This function should produce cnt v.16b and 3 uaddlp (Add Long
> > > +Pairwise). */ void
> > > +bar6 (unsigned long long *__restrict b, unsigned long long
> > > +*__restrict d) {
> > > + d[0] = __builtin_popcountll (b[0]);
> > > + d[1] = __builtin_popcountll (b[1]); }
> > > +
> > > +/* { dg-final { scan-assembler-not {\tbl\tpopcount} } } */
> > > +/* { dg-final { scan-assembler-times {cnt\t} 7 } } */
> > > +/* { dg-final { scan-assembler-times {uaddlp\t} 12 } } */
> > > +/* { dg-final { scan-assembler-times {ldr\tq} 5 } } */
> > > +/* { dg-final { scan-assembler-times {ldr\td} 2 } } */
