I reproduced the case on the current trunk. The vectorizer widens the byte load to halfwords and then uses UDOT/SDOT directly into the .d accumulator, so it now handles this case as intended. I’ll drop this patch and look into teaching the vectorizer to synthesize such multi-step reductions.
Thanks, Naveen On Wed, Sep 9, 2026 at 3:27 PM Tamar Christina <[email protected]> wrote: > > -----Original Message----- > > From: Kyrylo Tkachov <[email protected]> > > Sent: 07 September 2026 07:47 > > To: Naveen <[email protected]> > > Cc: [email protected]; Tamar Christina <[email protected]> > > Subject: Re: [PATCH v2] aarch64: Use [SU]DOT for byte-to-word step of > > VNx2DI widening sum [SVE2] > > > > Hi Naveen, > > > > > On 21 Aug 2026, at 14:43, Naveen > > <[email protected]> wrote: > > > > > > For an 8x widening sum (VNx2DI <- VNx16QI) SVE2 had no direct pattern > > > and fell back to three [SU]ADDWB+[SU]ADDWT pairs traversing bytes -> > > > halfwords -> words -> doublewords. > > > > > > Adding reduc_widen_<su>sumvnx2divnx16qi3 replaces the first two pairs > > > with a single [SU]DOT against a vector of all-ones (bytes -> words, 4x > > > in one instruction), followed by one ADDWB+ADDWT pair for the final > > > words -> doublewords step, saving two instructions per vector > iteration. > > > The 32-bit intermediate cannot overflow: max value is 4 * 255 = 1020 > > > (unsigned) or -512..508 (signed). > > > > > > gcc/ChangeLog: > > > * config/aarch64/aarch64-sve2.md > > > (reduc_widen_<su>sumvnx2divnx16qi3): New define_expand. > > > > > > gcc/testsuite/ChangeLog: > > > * gcc.target/aarch64/sve2/widen_sum_byte_to_long.c: New test. > > > > > > Signed-off-by: Naveen <[email protected]> > > > --- > > > gcc/config/aarch64/aarch64-sve2.md | 19 ++++++++++ > > > .../aarch64/sve2/widen_sum_byte_to_long.c | 36 > > +++++++++++++++++++ > > > 2 files changed, 55 insertions(+) > > > create mode 100644 > > gcc/testsuite/gcc.target/aarch64/sve2/widen_sum_byte_to_long.c > > > > > > diff --git a/gcc/config/aarch64/aarch64-sve2.md > > b/gcc/config/aarch64/aarch64-sve2.md > > > index 3a1285a6a00..a67ed561a88 100644 > > > --- a/gcc/config/aarch64/aarch64-sve2.md > > > +++ b/gcc/config/aarch64/aarch64-sve2.md > > > @@ -2615,6 +2615,25 @@ > > > } > > > }) > > > > > > +(define_expand "reduc_widen_<su>sumvnx2divnx16qi3" > > > + [(set (match_operand:VNx2DI 0 "register_operand") > > > + (plus:VNx2DI > > > + (ANY_EXTEND:VNx2DI (match_operand:VNx16QI 1 "register_operand")) > > > + (match_operand:VNx2DI 2 "register_operand")))] > > > + "TARGET_SVE2" > > > +{ > > > + rtx ones = force_reg (VNx16QImode, CONST1_RTX (VNx16QImode)); > > > + rtx tmp = gen_reg_rtx (VNx4SImode); > > > + rtx zero = force_reg (VNx4SImode, CONST0_RTX (VNx4SImode)); > > > + /* [SU]DOT: four bytes per 32-bit lane, accumulator seeded with > zero. */ > > > + emit_insn (gen_<su>dot_prodvnx4sivnx16qi (tmp, operands[1], ones, > > zero)); > > > + /* [SU]ADDWB takes even word lanes, [SU]ADDWT takes odd word lanes; > > > + both widen to 64 bits and accumulate into the destination. */ > > > + emit_insn (gen_aarch64_sve_<su>addwbvnx2di (operands[0], > > operands[2], tmp)); > > > + emit_insn (gen_aarch64_sve_<su>addwtvnx2di (operands[0], > > operands[0], tmp)); > > > + DONE; > > > +}) > > > > So this is just a dot-product and a widening sum reduction. > > In principle these are two generic operations that the vectorizer is > aware of. > > I think the vectorizer or (vect-lowering?) should learn to synthesize > such multi- > > step reductions rather than having the backend synthesize them > explicitly. > > I do agree that this should be done in the vectorizer, however is this the > right > testcase? For this testcase I'd expect the vectorizer to use a widening > load to short > and dotprod reduction from short to long. > > And indeed https://godbolt.org/z/r8P5jPbsY it's doing the correct thing > > Also when forced to keep the input as byte it just unpacks it > https://godbolt.org/z/Y6nb11fnq > > This is of course after Kyrill's changes to have dotprod stop in the > intermediate types. > > Before this we would lower VF enough to just do a normal add > https://godbolt.org/z/j8vdWW8T8 > > So is the patch still needed? > > Thanks, > Tamar > > > CC’ing Tamar. > > Thanks, > > Kyrill > > > > > + > > > ;; > ------------------------------------------------------------------------- > > > ;; ---- [INT] Long binary arithmetic > > > ;; > ------------------------------------------------------------------------- > > > diff --git > > a/gcc/testsuite/gcc.target/aarch64/sve2/widen_sum_byte_to_long.c > > b/gcc/testsuite/gcc.target/aarch64/sve2/widen_sum_byte_to_long.c > > > new file mode 100644 > > > index 00000000000..c7bde4c7c03 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/aarch64/sve2/widen_sum_byte_to_long.c > > > @@ -0,0 +1,36 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-O3 -march=armv8-a+sve2 -mautovec-preference=sve- > > only --param vect-epilogues-nomask=0" } */ > > > + > > > +long > > > +sum_u8_l (const unsigned char *a, int n) > > > +{ > > > + long s = 0; > > > + for (int i = 0; i < n; i++) > > > + s += a[i]; > > > + return s; > > > +} > > > + > > > +long > > > +sum_i8_l (const signed char *a, int n) > > > +{ > > > + long s = 0; > > > + for (int i = 0; i < n; i++) > > > + s += a[i]; > > > + return s; > > > +} > > > + > > > +/* Byte-to-word step uses [SU]DOT, not a byte-width [SU]ADDWB. */ > > > +/* { dg-final { scan-assembler-times {\tudot\tz[0-9]+\.s, z[0-9]+\.b, > z[0- > > 9]+\.b\n} 1 } } */ > > > +/* { dg-final { scan-assembler-times {\tsdot\tz[0-9]+\.s, z[0-9]+\.b, > z[0- > > 9]+\.b\n} 1 } } */ > > > + > > > +/* Word-to-doubleword step uses [SU]ADDWB and [SU]ADDWT on a .d > > destination. */ > > > +/* { dg-final { scan-assembler-times {\tuaddwb\tz[0-9]+\.d,} 1 } } */ > > > +/* { dg-final { scan-assembler-times {\tuaddwt\tz[0-9]+\.d,} 1 } } */ > > > +/* { dg-final { scan-assembler-times {\tsaddwb\tz[0-9]+\.d,} 1 } } */ > > > +/* { dg-final { scan-assembler-times {\tsaddwt\tz[0-9]+\.d,} 1 } } */ > > > + > > > +/* No byte-to-halfword or halfword-to-word widening; DOT replaces > both. > > */ > > > +/* { dg-final { scan-assembler-not {\t[su]addwb\tz[0-9]+\.h,} } } */ > > > +/* { dg-final { scan-assembler-not {\t[su]addwt\tz[0-9]+\.h,} } } */ > > > +/* { dg-final { scan-assembler-not {\t[su]addwb\tz[0-9]+\.s,} } } */ > > > +/* { dg-final { scan-assembler-not {\t[su]addwt\tz[0-9]+\.s,} } } */ > > > -- > > > 2.34.1 > > > > >
