Hi Soumya, I think the reason you're having a hard time getting reviews here is because on the surface this doesn't seem like a generic improvement. i.e. it feels like a target specific optimization because of how slow divisions are and not all targets have sdiv_pow2 anyway and there's no real reason to believe sdiv is slower than sdiv_pow2.
So my suggestion for you to make progress on this is to make it an AArch64 specific optimization. You can do so by extending aarch64_instruction_selection. I normally would not recommend this but I think given the earlier review comments it makes sense to make this AArch64 only. You can rewrite to .DIV_POW2 and if needed we can always move it to generic isel later. Thanks, Tamar > -----Original Message----- > From: Soumya AR <[email protected]> > Sent: 04 August 2026 09:08 > To: Kyrylo Tkachov <[email protected]>; Andrew Pinski > <[email protected]>; Richard Biener <[email protected]> > Cc: Tamar Christina <[email protected]>; [email protected] > Subject: Re: [PING][PATCH] aarch64: Use SVE ASRD with vector division using > division operator > > Hi Kyrill, > > Thanks for the review! > > > On 13 Jul 2026, at 8:54 PM, Kyrylo Tkachov <[email protected]> wrote: > > > > Hi Soumya, > > > >> On 30 Jun 2026, at 10:24, Soumya AR <[email protected]> wrote: > >> > >> Ping. > >> > >> Also CC'ing Andrea, could you please take a look if possible? > >> > >> Thanks, > >> Soumya > >> > >> On on Tue, 16 Jun 2026 10:54:37 +0000, Soumya AR > <[email protected]> wrote: > >>> Ping. > >>> > >>> Thanks, > >>> Soumya > >>> > >>>> On 8 Jun 2026, at 11:23 PM, Soumya AR <[email protected]> > wrote: > >>>> > >>>> 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) > >>>>> + { > > > > I think it is wrong to use the sdiv_pow2_optab when unsignedp is passed in > this function, otherwise inputs like: > > { 0x80000000, 0xffffffff, 16, 7 } / 8 > > > > will give wrong results > > My bad, added the check for this. > > > >>>>> + 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; > > > > > > For TRUNC_MOD_EXPR result is the quotient, I think this new path should > only be used for !rem_flag. > > Alternatively, assign the result to quotient and continue through the > common remainder handling. > > > > I experimented with using the common handling for this. Directly assigning the > result to quotient and falling through doesn't work since the next check is > op1_is_constant (false for a CONST_VECTOR). Control therefore falls through > to > fail1:, where delete_insns_since removes the emitted ASRD. > > We could just bypass this, but that might be moot, since vector pow2 modulo > is > actually processed earlier by expand_vector_divmod. > > However, this only works for NEON cases, for SVE modulo, we get the same > ICE > mentioned here: gcc.gnu.org/bugzilla/show_bug.cgi?id=118378 > > For now, I have just added !rem_flag. > > >>>>> + } > >>>>> + } > >>>>> + } > >>>>> 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) > > > > As mentioned above, I don’t think using ASRD this is correct for unsigned > types so these tests would need to be removed. > > Done. > > Thanks, > Soumya > > > > > Thanks, > > Kyrill > > > >>>>> + > >>>>> +/* > >>>>> +** 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]>; gcc- > [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) >
