Ping. Thanks, Soumya
> On 26 May 2026, at 2:24 PM, Soumya AR <[email protected]> wrote: > > Hi, > > Reviving this patch because it completely slipped my mind and has been sitting > in limbo ever since. > > I'll try to summarize the discussion for some context: > > This patch teaches expand_divmod to dispatch through the sdiv_pow2 optab for > vector pow2 divisions, so that > > svint32_t bar (svint32_t x) { return x / 4; } > > emits a single ASRD instead of a vector SDIV. ASRD on SVE performs signed > division by an arithmetic shift right that rounds toward zero. > > The autovectorizer already uses this optab via vect_recog_divmod_pattern; this > extends the same mechanism to the non-vectorizer path. > > There are a couple of backend approaches we could've taken: > > - make expand_divmod emit the shift sequence for vector pow2 divisions > (similar > to how it handles scalars) and and rely on combine to recognize it back into > ASRD. > > - extend the SVE SDIV pattern to accept immediates and emit ASRD for pow2. > > But those feel less clean given we have a dedicated optab (and that it would > be > neater to be consistent with the autovectorizer approach). > > Note that this also applies to Adv. SIMD vectors on SVE targets, since the > AArch64 backend exposes sdiv_pow2 for Adv. SIMD modes. > > For example, both: > > int32x4_t > foo (int32x4_t x) > { > return x / 4; > } > > and > > svint32_t > bar (svint32_t x) > { > return x / 4; > } > > currently generate DIV, but can be done using ASRD. > > The patch was bootstrapped and regtested on aarch64-linux-gnu, no regression. > OK for trunk? > > Signed-off-by: Soumya AR <[email protected]> > > gcc/ChangeLog: > > * expmed.cc (expand_divmod): Expand to sdiv_pow2 optab for vectors. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/sve/sve-asrd-2.c: New test. > --- > gcc/expmed.cc | 21 ++++++ > .../gcc.target/aarch64/sve/sve-asrd-2.c | 74 +++++++++++++++++++ > 2 files changed, 95 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/sve-asrd-2.c > > diff --git a/gcc/expmed.cc b/gcc/expmed.cc > index fe39506b6bd..ef12420689f 100644 > --- a/gcc/expmed.cc > +++ b/gcc/expmed.cc > @@ -4487,6 +4487,27 @@ expand_divmod (int rem_flag, enum tree_code code, > machine_mode mode, > { > case TRUNC_MOD_EXPR: > case TRUNC_DIV_EXPR: > + if (CONST_VECTOR_P (op1) > + && optab_handler (sdiv_pow2_optab, mode) != CODE_FOR_nothing) > + { > + rtx scalar_op1; > + if (const_vec_duplicate_p (op1, &scalar_op1) > + && CONST_INT_P (scalar_op1)) > + { > + HOST_WIDE_INT d = INTVAL (scalar_op1); > + if (d > 0 && pow2p_hwi (d)) > + { > + rtx shift_amount > + = gen_const_vec_duplicate (mode, > + GEN_INT (floor_log2 (d))); > + rtx result > + = expand_binop (mode, sdiv_pow2_optab, op0, shift_amount, > + target, unsignedp, methods); > + if (result) > + return result; > + } > + } > + } > if (op1_is_constant) > { > scalar_int_mode int_mode = as_a <scalar_int_mode> (compute_mode); > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/sve-asrd-2.c > b/gcc/testsuite/gcc.target/aarch64/sve/sve-asrd-2.c > new file mode 100644 > index 00000000000..a5163421a69 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/sve/sve-asrd-2.c > @@ -0,0 +1,74 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-O3" } */ > +/* { dg-final { check-function-bodies "**" "" "" } } */ > + > +#include <arm_sve.h> > + > +#define FOO(type) \ > +type foo_##type(type x) { \ > + return x / 8; \ > +} > + > +/* > +** foo_int32x4_t: > +** ptrue (p[0-7]).b, vl16 > +** asrd z[0-9]+\.s, \1/m, z[0-9]+\.s, #3 > +** ret > +*/ > +FOO(int32x4_t) > + > +/* > +** foo_int64x2_t: > +** ptrue (p[0-7]).b, vl16 > +** asrd z[0-9]+\.d, \1/m, z[0-9]+\.d, #3 > +** ret > +*/ > +FOO(int64x2_t) > + > +/* > +** foo_svint32_t: > +** ptrue (p[0-7]).b, all > +** asrd z[0-9]+\.s, \1/m, z[0-9]+\.s, #3 > +** ret > +*/ > +FOO(svint32_t) > + > +/* > +** foo_svint64_t: > +** ptrue (p[0-7]).b, all > +** asrd z[0-9]+\.d, \1/m, z[0-9]+\.d, #3 > +** ret > +*/ > +FOO(svint64_t) > + > +/* > +** foo_uint32x4_t: > +** ptrue (p[0-7]).b, vl16 > +** asrd z[0-9]+\.s, \1/m, z[0-9]+\.s, #3 > +** ret > +*/ > +FOO(uint32x4_t) > + > +/* > +** foo_uint64x2_t: > +** ptrue (p[0-7]).b, vl16 > +** asrd z[0-9]+\.d, \1/m, z[0-9]+\.d, #3 > +** ret > +*/ > +FOO(uint64x2_t) > + > +/* > +** foo_svuint32_t: > +** ptrue (p[0-7]).b, all > +** asrd z[0-9]+\.s, \1/m, z[0-9]+\.s, #3 > +** ret > +*/ > +FOO(svuint32_t) > + > +/* > +** foo_svuint64_t: > +** ptrue (p[0-7]).b, all > +** asrd z[0-9]+\.d, \1/m, z[0-9]+\.d, #3 > +** ret > +*/ > +FOO(svuint64_t) > -- > 2.43.0 > > >> On 22 Oct 2025, at 7:21 PM, Tamar Christina <[email protected]> wrote: >> >> External email: Use caution opening links or attachments >> >> >>> -----Original Message----- >>> From: Richard Biener <[email protected]> >>> Sent: 22 October 2025 14:30 >>> To: Tamar Christina <[email protected]> >>> Cc: Soumya AR <[email protected]>; [email protected]; Kyrylo >>> Tkachov <[email protected]> >>> Subject: RE: [PATCH] aarch64: Use SVE ASRD with vector division using >>> division operator >>> >>> On Wed, 22 Oct 2025, Tamar Christina wrote: >>> >>>>> -----Original Message----- >>>>> From: Richard Biener <[email protected]> >>>>> Sent: 22 October 2025 14:05 >>>>> To: Tamar Christina <[email protected]> >>>>> Cc: Soumya AR <[email protected]>; [email protected]; Kyrylo >>>>> Tkachov <[email protected]> >>>>> Subject: RE: [PATCH] aarch64: Use SVE ASRD with vector division using >>>>> division operator >>>>> >>>>> On Wed, 22 Oct 2025, Tamar Christina wrote: >>>>> >>>>>> Hi Soumya, >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Soumya AR <[email protected]> >>>>>>> Sent: 13 October 2025 13:42 >>>>>>> To: Tamar Christina <[email protected]> >>>>>>> Cc: [email protected]; Kyrylo Tkachov <[email protected]>; >>>>>>> Richard Biener <[email protected]> >>>>>>> Subject: Re: [PATCH] aarch64: Use SVE ASRD with vector division using >>>>>>> division operator >>>>>>> Importance: High >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On 13 Oct 2025, at 1:48 PM, Tamar Christina >>>>> <[email protected]> >>>>>>> wrote: >>>>>>>> >>>>>>>> External email: Use caution opening links or attachments >>>>>>>> >>>>>>>> >>>>>>>> Hi Soumya, >>>>>>>> >>>>>>>> The 10/13/2025 07:05, Soumya AR wrote: >>>>>>>>> The ASRD instruction on SVE performs an arithmetic shift right by an >>>>>>> immediate >>>>>>>>> for divide. This patch enables ASRD when dividing vectors using the >>> GNU >>>>> C >>>>>>>>> division operator. >>>>>>>>> >>>>>>>>> For example: >>>>>>>>> >>>>>>>>> int32x4_t >>>>>>>>> foo (int32x4_t x) >>>>>>>>> { >>>>>>>>> return x / 4; >>>>>>>>> } >>>>>>>>> >>>>>>>>> svint32_t >>>>>>>>> bar (svint32_t x) >>>>>>>>> { >>>>>>>>> return x / 4; >>>>>>>>> } >>>>>>>>> >>>>>>>>> currently generates a DIV, but can be done using ASRD. >>>>>>>>> >>>>>>>>> The patch was bootstrapped and regtested on aarch64-linux-gnu, no >>>>>>> regression. >>>>>>>>> OK for GCC16? >>>>>>>>> >>>>>>>>> Signed-off-by: Soumya AR <[email protected]> >>>>>>>>> >>>>>>>>> gcc/ChangeLog: >>>>>>>>> >>>>>>>>> * expmed.cc (expand_divmod): Expand to sdiv_pow2 optab for >>>>> vectors. >>>>>>>>> >>>>>>>>> gcc/testsuite/ChangeLog: >>>>>>>>> >>>>>>>>> * gcc.target/aarch64/sve/sve-asrd-2.c: New test. >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> Hi Tamar, >>>>>>> >>>>>>> Thanks for the quick review! >>>>>>> >>>>>>>> The test is OK, but you'll need a middle-end maintainer for the >>> expmed.cc >>>>>>> changes. >>>>>>> >>>>>>> My bad, CC'ing Richi here as well. >>>>>>> >>>>>>>> But that said... >>>>>>>> >>>>>>>> diff --git a/gcc/expmed.cc b/gcc/expmed.cc >>>>>>>> index df09cbccd08..a35aa229d44 100644 >>>>>>>> --- a/gcc/expmed.cc >>>>>>>> +++ b/gcc/expmed.cc >>>>>>>> @@ -4465,6 +4465,24 @@ expand_divmod (int rem_flag, enum >>>>> tree_code >>>>>>> code, machine_mode mode, >>>>>>>> { >>>>>>>> case TRUNC_MOD_EXPR: >>>>>>>> case TRUNC_DIV_EXPR: >>>>>>>> + if (CONST_VECTOR_P (op1) >>>>>>>> + && optab_handler (sdiv_pow2_optab, mode) != >>>>> CODE_FOR_nothing) >>>>>>>> + { >>>>>>>> + rtx scalar_op1 = unwrap_const_vec_duplicate (op1); >>>>>>>> + if (scalar_op1 != NULL_RTX && CONST_INT_P (scalar_op1)) >>>>>>>> >>>>>>>> You can simplify this using const_vec_duplicate_p. >>>>>>>> >>>>>>>> + { >>>>>>>> + HOST_WIDE_INT d = INTVAL (scalar_op1); >>>>>>>> + if (d > 0 && pow2p_hwi (d)) >>>>>>>> + { >>>>>>>> + rtx shift_amount >>>>>>>> + = gen_const_vec_duplicate (mode, >>>>>>>> + GEN_INT (floor_log2 >>>>>>>> (d))); >>>>>>>> + return expand_binop (mode, sdiv_pow2_optab, op0, >>>>>>>> + shift_amount, target, >>>>>>>> unsignedp, >>>>>>>> + methods); >>>>>>>> >>>>>>>> This expansion can fail, when e.g. the shift amount is larger than the >>>>> bitsize >>>>>>> of the element, >>>>>>>> so you should check the result of it before exiting from >>> expand_divmod. >>>>>>>> >>>>>>> >>>>>>> Makes sense, attaching an updated patch with both changes. >>>>>>> >>>>>> >>>>>> The new patch looks OK to me (it uses the old interface but that's what >>> most >>>>> of >>>>>> expmed uses). >>>>>> >>>>>> I think your cover letter needs to mention that we can't do this in the >>>>> backend because >>>>>> Adv. SIMD lacks a division instruction and that the decomposition is >>> harder >>>>> to match >>>>>> correctly in all cases. >>>>>> >>>>>> That said you still need a middle-end maintainer approval. >>>>> >>>>> So the backend advertises a vector division? Othewise I would have >>>>> expected vector lowering to lower this (if it does that at all, for >>>>> VLA vectors?!). >>>>> >>>>> So this is an alternative better expansion - but there should already >>>>> be generic code to deal with divisions by a power of two, why does >>>>> that not trigger? And what's this sdiv_pow2 thing again? >>>> >>>> Yes, expand intercepts in expand_divmod division by a power of 2 and >>> decomposes >>>> It to shifts. We could re-recognize the instruction in combine of >>>> course, but >>> we already >>>> have an optab the vectorizer would have used in this case [us]div_pow2, see >>>> vect_recog_divmod_pattern. So this is just teaching expand about the >>> special division of >>>> a power of 2 optab that the vectorizer uses for normal vector operations. >>> >>> I can't remember the difference of div_pow2 and shifts, but it feels like >>> this expand-time improvement should be done by expand_divmod then? >> >> The patch does extend expand_divmod though, but does through an existing >> optab. >> >> The issue is that signedness matters, >> this rewrite to asrd is only possible for signed values as it's an >> arithmetic shift. >> >> For unsigned values we want ushr. >> >>> 'asrd' is described as arithmetic shift right for divide by immediate >>> whatever the exact difference from 'asr' is ... the docs say sth >>> about round towards zero, but then we're using arithmetic right shifts >>> for signed division already ... >> >> SVE doesn't support division by immediates, so SDIV only takes a register. >> we could always extend the sdiv pattern to support immediates, but it >> does feel odd given we have a different optab for this. >> >> Thanks, >> Tamar >> >>> >>> Richard. >>> >>>> Thanks, >>>> Tamar >>>> >>>>> >>>>> Richard. >>>>> >>>>>> Thanks, >>>>>> Tamar >>>>>> >>>>>>> Thanks, >>>>>>> Soumya >>>>>>> >>>>>>>> Thanks, >>>>>>>> Tamar >>>>>>>> >>>>>>>> + } >>>>>>>> + } >>>>>>>> + } >>>>>>>> if (op1_is_constant) >>>>>>>> { >>>>>>>> scalar_int_mode int_mode = as_a <scalar_int_mode> >>>>>>> (compute_mode); >>>>>>> >>>>>> >>>>>> >>>>> >>>>> -- >>>>> Richard Biener <[email protected]> >>>>> SUSE Software Solutions Germany GmbH, >>>>> Frankenstrasse 146, 90461 Nuernberg, Germany; >>>>> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG >>>>> Nuernberg) >>>> >>> >>> -- >>> Richard Biener <[email protected]> >>> SUSE Software Solutions Germany GmbH, >>> Frankenstrasse 146, 90461 Nuernberg, Germany; >>> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG >>> Nuernberg) > >
