diff mbox series

[vect] : Fix scalar iteration bounds when doing alignment peeling with partial vectors [PR127677]

Message ID patch-20948-rb-rev-1-tamar@arm.com
State New
Headers
Series [vect] : Fix scalar iteration bounds when doing alignment peeling with partial vectors [PR127677] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed

Commit Message

Tamar Christina Oct. 1, 2026, 8:37 a.m. UTC 
  static unsigned int list[32] __attribute__((aligned(64)));

__attribute__((noinline, noipa))
static int
contains (const unsigned int *p, unsigned int count, unsigned int start,
          unsigned int key)
{
  for (unsigned int i = start; i < count; ++i)
    if (p[i] == key)
      return 1;
  return 0;
}

int
main (void)
{
  return contains (list, 2, 1, 1);
}

when compiled on RISC-V with

-O3 -march=rv64gcv_zvl256b -mabi=lp64d -mrvv-vector-bits=zvl -mrvv-max-lmul=m2 -g0

shows

  bnd.9_47 = niters.5_27 - prolog_loop_niters.7_31;
  _63 = (unsigned long) bnd.9_47;

...


and so if niters is lower than prolog iters we get an underflow.
i.e.

  niters        = count - start = 2 - 1 = 1
  prolog_niters = 7

so when it calculates the remainder

  remaining = niters - prolog_niters;

this underflows:

   1 - 7 = 4294967290

Normally we don't get here for AArch64 and x86_64 because of the different
paths they take in vectorization.

For SVE we perform the loop fully masked, including trailing elements.
for Adv. SIMD and x86_64 we insert a peeling check.

  _46 = niters - 1 = 0;
  _48 = MAX(peel + 3, 5);   // at least 5

  if (0 < _48)
      goto bb26;

which skips the alignment peeling prologue.  Prior to
8af2e8e49d6e5d33c01c2beaead4933bc286974c we would have generated an epilog
for the early break and alignment peeling.

The change removed the requirement for an epilog for early break loops which
don't have side-effects, but still required alignment peeling.

This exposed that the checks in vect_do_peeling generate the epilog and so
the iteration check only when

  poly_uint64 bound_epilog = 0;
  if (!LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)
      && LOOP_VINFO_PEELING_FOR_NITER (loop_vinfo))
    bound_epilog += vf - 1;
  if (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo))
    bound_epilog += 1;

so if LOOP_VINFO_USING_PARTUAL_VECTORS_P then no epilog or check is generated.
This means that vect_gen_prolog_loop_niters needs to in itself handle this
assumption.

This is easy to do by generating a MIN (niters, prolog_niters) check.
If the guard is generated this will be optimized out, but if not,
it makes the prolog peeling loop handle the "scalar" iterations itself without
needing a second scalar loop.

Bootstrapped Regtested on aarch64-none-linux-gnu,
arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
-m32, -m64 and no issues.

Any comments?

Thanks,
Tamar

gcc/ChangeLog:

	PR tree-optimization/127677
	* tree-vect-loop-manip.cc (vect_gen_prolog_loop_niters): Generate MIN
	expression on niters.

gcc/testsuite/ChangeLog:

	PR tree-optimization/127677
	* gcc.dg/vect/vect-early-break_146-pr127677.c: New test.

---


--

Comments

Richard Biener Oct. 1, 2026, 9:43 a.m. UTC | #1 
On Thu, 1 Oct 2026, Tamar Christina wrote:

> static unsigned int list[32] __attribute__((aligned(64)));
> 
> __attribute__((noinline, noipa))
> static int
> contains (const unsigned int *p, unsigned int count, unsigned int start,
>           unsigned int key)
> {
>   for (unsigned int i = start; i < count; ++i)
>     if (p[i] == key)
>       return 1;
>   return 0;
> }
> 
> int
> main (void)
> {
>   return contains (list, 2, 1, 1);
> }
> 
> when compiled on RISC-V with
> 
> -O3 -march=rv64gcv_zvl256b -mabi=lp64d -mrvv-vector-bits=zvl -mrvv-max-lmul=m2 -g0
> 
> shows
> 
>   bnd.9_47 = niters.5_27 - prolog_loop_niters.7_31;
>   _63 = (unsigned long) bnd.9_47;
> 
> ...
> 
> 
> and so if niters is lower than prolog iters we get an underflow.
> i.e.
> 
>   niters        = count - start = 2 - 1 = 1
>   prolog_niters = 7
> 
> so when it calculates the remainder
> 
>   remaining = niters - prolog_niters;
> 
> this underflows:
> 
>    1 - 7 = 4294967290
> 
> Normally we don't get here for AArch64 and x86_64 because of the different
> paths they take in vectorization.
> 
> For SVE we perform the loop fully masked, including trailing elements.
> for Adv. SIMD and x86_64 we insert a peeling check.
> 
>   _46 = niters - 1 = 0;
>   _48 = MAX(peel + 3, 5);   // at least 5
> 
>   if (0 < _48)
>       goto bb26;
> 
> which skips the alignment peeling prologue.  Prior to
> 8af2e8e49d6e5d33c01c2beaead4933bc286974c we would have generated an epilog
> for the early break and alignment peeling.
> 
> The change removed the requirement for an epilog for early break loops which
> don't have side-effects, but still required alignment peeling.
> 
> This exposed that the checks in vect_do_peeling generate the epilog and so
> the iteration check only when
> 
>   poly_uint64 bound_epilog = 0;
>   if (!LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)
>       && LOOP_VINFO_PEELING_FOR_NITER (loop_vinfo))
>     bound_epilog += vf - 1;
>   if (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo))
>     bound_epilog += 1;
> 
> so if LOOP_VINFO_USING_PARTUAL_VECTORS_P then no epilog or check is generated.
> This means that vect_gen_prolog_loop_niters needs to in itself handle this
> assumption.
> 
> This is easy to do by generating a MIN (niters, prolog_niters) check.
> If the guard is generated this will be optimized out, but if not,
> it makes the prolog peeling loop handle the "scalar" iterations itself without
> needing a second scalar loop.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu,
> arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
> -m32, -m64 and no issues.
> 
> Any comments?

So when not using partial vectors this is guaranteed by a niter check
which would skip the prologue and main loop if there's not enough
iterations to cover at least one full vector iteration?  With
partial vectors we assume it's always profitable to enter the loop
(IIRC), right?  I wonder if it makes more sense to put this check
into the caller at the correct place ('remaining' isn't visible here)?
The comment would also need to be updated to reflect this.

Richard.

> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/127677
> 	* tree-vect-loop-manip.cc (vect_gen_prolog_loop_niters): Generate MIN
> 	expression on niters.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/127677
> 	* gcc.dg/vect/vect-early-break_146-pr127677.c: New test.
> 
> ---
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c b/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f3bd8665c7fe39cff5e2be88521650fae9bf4c90
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
> @@ -0,0 +1,45 @@
> +/* { dg-add-options vect_early_break } */
> +/* { dg-require-effective-target vect_early_break_hw } */
> +/* { dg-require-effective-target vect_partial_vectors } */
> +/* { dg-require-effective-target vect_int } */
> +
> +/* { dg-additional-options "-O3" } */
> +
> +#include "tree-vect.h"
> +
> +struct data
> +{
> +  unsigned int list[16];
> +  unsigned int sentinel;
> +};
> +
> +static struct data data __attribute__ ((aligned (64))) =
> +{
> +  { 0 },
> +  1
> +};
> +
> +__attribute__ ((noinline, noipa))
> +static int
> +contains (const unsigned int *p, unsigned int count, unsigned int start,
> +          unsigned int key)
> +{
> +  for (unsigned int i = start; i < count; ++i)
> +    if (p[i] == key)
> +      return 1;
> +
> +  return 0;
> +}
> +
> +int
> +main (void)
> +{
> +  check_vect ();
> +
> +  if (contains (data.list, 2, 1, 1) != 0)
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */
> diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> index 9d22d87705baaa60b96d973486bf38398c9ac3f2..37b1b83703718a7d0bc091ec6f755485855141af 100644
> --- a/gcc/tree-vect-loop-manip.cc
> +++ b/gcc/tree-vect-loop-manip.cc
> @@ -2832,6 +2832,30 @@ vect_gen_prolog_loop_niters (loop_vec_info loop_vinfo,
>        *bound = align_in_elems;
>      }
>  
> +  /* Peeling for alignment if the amount to peel is larger than scalar niters
> +     needs to cap the scalar niters to the MIN of niters and peeling.  Otherwise
> +     when we are calculating the number of iterations the peeled loop performs
> +     we may cause an overflow.  i.e.
> +
> +     niters        = count - start = 2 - 1 = 1
> +     prolog_niters = 7
> +
> +     we then calculate the iteration count with
> +
> +     remaining = niters - prolog_niters;
> +
> +     which would overflow:
> +
> +     remaining = 1 - 7 = 4294967290 and then produce an incorrect value.  */
> +  if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)
> +      && !LOOP_VINFO_NITERS_UNCOUNTED_P (loop_vinfo))
> +    {
> +      tree niters = LOOP_VINFO_NITERS (loop_vinfo);
> +      if (!types_compatible_p (niters_type, TREE_TYPE (niters)))
> +       niters = fold_convert (niters_type, niters);
> +      iters = fold_build2 (MIN_EXPR, niters_type, iters, niters);
> +    }
> +
>    if (dump_enabled_p ())
>      dump_printf_loc (MSG_NOTE, vect_location,
>  		     "niters for prolog loop: %T\n", iters);
> 
> 
>
Tamar Christina Oct. 2, 2026, 7:28 a.m. UTC | #2 
> -----Original Message-----
> From: Richard Biener <rguenther@suse.de>
> Sent: 01 October 2026 11:43
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; rdapp.gcc@gmail.com
> Subject: Re: [patch][vect]: Fix scalar iteration bounds when doing alignment
> peeling with partial vectors [PR127677]
> 
> On Thu, 1 Oct 2026, Tamar Christina wrote:
> 
> > static unsigned int list[32] __attribute__((aligned(64)));
> >
> > __attribute__((noinline, noipa))
> > static int
> > contains (const unsigned int *p, unsigned int count, unsigned int start,
> >           unsigned int key)
> > {
> >   for (unsigned int i = start; i < count; ++i)
> >     if (p[i] == key)
> >       return 1;
> >   return 0;
> > }
> >
> > int
> > main (void)
> > {
> >   return contains (list, 2, 1, 1);
> > }
> >
> > when compiled on RISC-V with
> >
> > -O3 -march=rv64gcv_zvl256b -mabi=lp64d -mrvv-vector-bits=zvl -mrvv-
> max-lmul=m2 -g0
> >
> > shows
> >
> >   bnd.9_47 = niters.5_27 - prolog_loop_niters.7_31;
> >   _63 = (unsigned long) bnd.9_47;
> >
> > ...
> >
> >
> > and so if niters is lower than prolog iters we get an underflow.
> > i.e.
> >
> >   niters        = count - start = 2 - 1 = 1
> >   prolog_niters = 7
> >
> > so when it calculates the remainder
> >
> >   remaining = niters - prolog_niters;
> >
> > this underflows:
> >
> >    1 - 7 = 4294967290
> >
> > Normally we don't get here for AArch64 and x86_64 because of the different
> > paths they take in vectorization.
> >
> > For SVE we perform the loop fully masked, including trailing elements.
> > for Adv. SIMD and x86_64 we insert a peeling check.
> >
> >   _46 = niters - 1 = 0;
> >   _48 = MAX(peel + 3, 5);   // at least 5
> >
> >   if (0 < _48)
> >       goto bb26;
> >
> > which skips the alignment peeling prologue.  Prior to
> > 8af2e8e49d6e5d33c01c2beaead4933bc286974c we would have generated
> an epilog
> > for the early break and alignment peeling.
> >
> > The change removed the requirement for an epilog for early break loops
> which
> > don't have side-effects, but still required alignment peeling.
> >
> > This exposed that the checks in vect_do_peeling generate the epilog and so
> > the iteration check only when
> >
> >   poly_uint64 bound_epilog = 0;
> >   if (!LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)
> >       && LOOP_VINFO_PEELING_FOR_NITER (loop_vinfo))
> >     bound_epilog += vf - 1;
> >   if (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo))
> >     bound_epilog += 1;
> >
> > so if LOOP_VINFO_USING_PARTUAL_VECTORS_P then no epilog or check is
> generated.
> > This means that vect_gen_prolog_loop_niters needs to in itself handle this
> > assumption.
> >
> > This is easy to do by generating a MIN (niters, prolog_niters) check.
> > If the guard is generated this will be optimized out, but if not,
> > it makes the prolog peeling loop handle the "scalar" iterations itself without
> > needing a second scalar loop.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
> > -m32, -m64 and no issues.
> >
> > Any comments?
> 
> So when not using partial vectors this is guaranteed by a niter check
> which would skip the prologue and main loop if there's not enough
> iterations to cover at least one full vector iteration?

Indeed.

> With
> partial vectors we assume it's always profitable to enter the loop
> (IIRC), right?

Yeah and its normally expected that the partial vector be able to handle
any remaining operations.  You then have two approaches to handle the
peeling, using masks, or using scalar iters.

SVE uses masks of course, and RISC-V seems to use scalar peeling.

> I wonder if it makes more sense to put this check
> into the caller at the correct place ('remaining' isn't visible here)?

I originally did think of putting it in the caller, in the

      if (!uncounted_p)
	{
	  /* Update niters for vector loop.  */

That generates the adjustment.  But in this case the value out of
vect_gen_prolog_loop_niters, because essentially we know it's
invalid to peel more iterations than we have niters.

So it seemed odd to me to allow vect_gen_prolog_loop_niters to
return a known, invalid value.

But we then use this value in e.g. setting loop control, or moving the
DRs in

      vect_update_inits_of_drs (loop_vinfo, niters_prolog, PLUS_EXPR);

and those DRs are now invalid.  But at least returning the capped number
can make sense, for if e..g p was misaligned by 7 elements (in this example)
since doing the single step does get you aligned.

So putting it in the callee seemed more correct, but indeed the comment
needs updating.

Do you still want me to pull it to the caller?

Thanks,
Tamar

> The comment would also need to be updated to reflect this.
> 

Yeah,

> Richard.
> 
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > 	PR tree-optimization/127677
> > 	* tree-vect-loop-manip.cc (vect_gen_prolog_loop_niters): Generate
> MIN
> > 	expression on niters.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 	PR tree-optimization/127677
> > 	* gcc.dg/vect/vect-early-break_146-pr127677.c: New test.
> >
> > ---
> > diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
> b/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..f3bd8665c7fe39cff5e2
> be88521650fae9bf4c90
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
> > @@ -0,0 +1,45 @@
> > +/* { dg-add-options vect_early_break } */
> > +/* { dg-require-effective-target vect_early_break_hw } */
> > +/* { dg-require-effective-target vect_partial_vectors } */
> > +/* { dg-require-effective-target vect_int } */
> > +
> > +/* { dg-additional-options "-O3" } */
> > +
> > +#include "tree-vect.h"
> > +
> > +struct data
> > +{
> > +  unsigned int list[16];
> > +  unsigned int sentinel;
> > +};
> > +
> > +static struct data data __attribute__ ((aligned (64))) =
> > +{
> > +  { 0 },
> > +  1
> > +};
> > +
> > +__attribute__ ((noinline, noipa))
> > +static int
> > +contains (const unsigned int *p, unsigned int count, unsigned int start,
> > +          unsigned int key)
> > +{
> > +  for (unsigned int i = start; i < count; ++i)
> > +    if (p[i] == key)
> > +      return 1;
> > +
> > +  return 0;
> > +}
> > +
> > +int
> > +main (void)
> > +{
> > +  check_vect ();
> > +
> > +  if (contains (data.list, 2, 1, 1) != 0)
> > +    __builtin_abort ();
> > +
> > +  return 0;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */
> > diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> > index
> 9d22d87705baaa60b96d973486bf38398c9ac3f2..37b1b83703718a7d0bc
> 091ec6f755485855141af 100644
> > --- a/gcc/tree-vect-loop-manip.cc
> > +++ b/gcc/tree-vect-loop-manip.cc
> > @@ -2832,6 +2832,30 @@ vect_gen_prolog_loop_niters (loop_vec_info
> loop_vinfo,
> >        *bound = align_in_elems;
> >      }
> >
> > +  /* Peeling for alignment if the amount to peel is larger than scalar niters
> > +     needs to cap the scalar niters to the MIN of niters and peeling.
> Otherwise
> > +     when we are calculating the number of iterations the peeled loop
> performs
> > +     we may cause an overflow.  i.e.
> > +
> > +     niters        = count - start = 2 - 1 = 1
> > +     prolog_niters = 7
> > +
> > +     we then calculate the iteration count with
> > +
> > +     remaining = niters - prolog_niters;
> > +
> > +     which would overflow:
> > +
> > +     remaining = 1 - 7 = 4294967290 and then produce an incorrect value.  */
> > +  if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)
> > +      && !LOOP_VINFO_NITERS_UNCOUNTED_P (loop_vinfo))
> > +    {
> > +      tree niters = LOOP_VINFO_NITERS (loop_vinfo);
> > +      if (!types_compatible_p (niters_type, TREE_TYPE (niters)))
> > +       niters = fold_convert (niters_type, niters);
> > +      iters = fold_build2 (MIN_EXPR, niters_type, iters, niters);
> > +    }
> > +
> >    if (dump_enabled_p ())
> >      dump_printf_loc (MSG_NOTE, vect_location,
> >  		     "niters for prolog loop: %T\n", iters);
> >
> >
> >
> 
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)
Richard Biener Oct. 2, 2026, 10:42 a.m. UTC | #3 
On Fri, 2 Oct 2026, Tamar Christina wrote:

> > -----Original Message-----
> > From: Richard Biener <rguenther@suse.de>
> > Sent: 01 October 2026 11:43
> > To: Tamar Christina <Tamar.Christina@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; rdapp.gcc@gmail.com
> > Subject: Re: [patch][vect]: Fix scalar iteration bounds when doing alignment
> > peeling with partial vectors [PR127677]
> > 
> > On Thu, 1 Oct 2026, Tamar Christina wrote:
> > 
> > > static unsigned int list[32] __attribute__((aligned(64)));
> > >
> > > __attribute__((noinline, noipa))
> > > static int
> > > contains (const unsigned int *p, unsigned int count, unsigned int start,
> > >           unsigned int key)
> > > {
> > >   for (unsigned int i = start; i < count; ++i)
> > >     if (p[i] == key)
> > >       return 1;
> > >   return 0;
> > > }
> > >
> > > int
> > > main (void)
> > > {
> > >   return contains (list, 2, 1, 1);
> > > }
> > >
> > > when compiled on RISC-V with
> > >
> > > -O3 -march=rv64gcv_zvl256b -mabi=lp64d -mrvv-vector-bits=zvl -mrvv-
> > max-lmul=m2 -g0
> > >
> > > shows
> > >
> > >   bnd.9_47 = niters.5_27 - prolog_loop_niters.7_31;
> > >   _63 = (unsigned long) bnd.9_47;
> > >
> > > ...
> > >
> > >
> > > and so if niters is lower than prolog iters we get an underflow.
> > > i.e.
> > >
> > >   niters        = count - start = 2 - 1 = 1
> > >   prolog_niters = 7
> > >
> > > so when it calculates the remainder
> > >
> > >   remaining = niters - prolog_niters;
> > >
> > > this underflows:
> > >
> > >    1 - 7 = 4294967290
> > >
> > > Normally we don't get here for AArch64 and x86_64 because of the different
> > > paths they take in vectorization.
> > >
> > > For SVE we perform the loop fully masked, including trailing elements.
> > > for Adv. SIMD and x86_64 we insert a peeling check.
> > >
> > >   _46 = niters - 1 = 0;
> > >   _48 = MAX(peel + 3, 5);   // at least 5
> > >
> > >   if (0 < _48)
> > >       goto bb26;
> > >
> > > which skips the alignment peeling prologue.  Prior to
> > > 8af2e8e49d6e5d33c01c2beaead4933bc286974c we would have generated
> > an epilog
> > > for the early break and alignment peeling.
> > >
> > > The change removed the requirement for an epilog for early break loops
> > which
> > > don't have side-effects, but still required alignment peeling.
> > >
> > > This exposed that the checks in vect_do_peeling generate the epilog and so
> > > the iteration check only when
> > >
> > >   poly_uint64 bound_epilog = 0;
> > >   if (!LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)
> > >       && LOOP_VINFO_PEELING_FOR_NITER (loop_vinfo))
> > >     bound_epilog += vf - 1;
> > >   if (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo))
> > >     bound_epilog += 1;
> > >
> > > so if LOOP_VINFO_USING_PARTUAL_VECTORS_P then no epilog or check is
> > generated.
> > > This means that vect_gen_prolog_loop_niters needs to in itself handle this
> > > assumption.
> > >
> > > This is easy to do by generating a MIN (niters, prolog_niters) check.
> > > If the guard is generated this will be optimized out, but if not,
> > > it makes the prolog peeling loop handle the "scalar" iterations itself without
> > > needing a second scalar loop.
> > >
> > > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > > arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
> > > -m32, -m64 and no issues.
> > >
> > > Any comments?
> > 
> > So when not using partial vectors this is guaranteed by a niter check
> > which would skip the prologue and main loop if there's not enough
> > iterations to cover at least one full vector iteration?
> 
> Indeed.
> 
> > With
> > partial vectors we assume it's always profitable to enter the loop
> > (IIRC), right?
> 
> Yeah and its normally expected that the partial vector be able to handle
> any remaining operations.  You then have two approaches to handle the
> peeling, using masks, or using scalar iters.
> 
> SVE uses masks of course, and RISC-V seems to use scalar peeling.
> 
> > I wonder if it makes more sense to put this check
> > into the caller at the correct place ('remaining' isn't visible here)?
> 
> I originally did think of putting it in the caller, in the
> 
>       if (!uncounted_p)
> 	{
> 	  /* Update niters for vector loop.  */
> 
> That generates the adjustment.  But in this case the value out of
> vect_gen_prolog_loop_niters, because essentially we know it's
> invalid to peel more iterations than we have niters.
> 
> So it seemed odd to me to allow vect_gen_prolog_loop_niters to
> return a known, invalid value.
> 
> But we then use this value in e.g. setting loop control, or moving the
> DRs in
> 
>       vect_update_inits_of_drs (loop_vinfo, niters_prolog, PLUS_EXPR);

Yeah, but that's only for non-mask peeling?

> and those DRs are now invalid.  But at least returning the capped number
> can make sense, for if e..g p was misaligned by 7 elements (in this example)
> since doing the single step does get you aligned.
> 
> So putting it in the callee seemed more correct, but indeed the comment
> needs updating.
>
> Do you still want me to pull it to the caller?

I'm fine with the explanation but please update the comment.

Thanks,
Richard.
 
> Thanks,
> Tamar
> 
> > The comment would also need to be updated to reflect this.
> > 
> 
> Yeah,
> 
> > Richard.
> > 
> > > Thanks,
> > > Tamar
> > >
> > > gcc/ChangeLog:
> > >
> > > 	PR tree-optimization/127677
> > > 	* tree-vect-loop-manip.cc (vect_gen_prolog_loop_niters): Generate
> > MIN
> > > 	expression on niters.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > 	PR tree-optimization/127677
> > > 	* gcc.dg/vect/vect-early-break_146-pr127677.c: New test.
> > >
> > > ---
> > > diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
> > b/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
> > > new file mode 100644
> > > index
> > 0000000000000000000000000000000000000000..f3bd8665c7fe39cff5e2
> > be88521650fae9bf4c90
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
> > > @@ -0,0 +1,45 @@
> > > +/* { dg-add-options vect_early_break } */
> > > +/* { dg-require-effective-target vect_early_break_hw } */
> > > +/* { dg-require-effective-target vect_partial_vectors } */
> > > +/* { dg-require-effective-target vect_int } */
> > > +
> > > +/* { dg-additional-options "-O3" } */
> > > +
> > > +#include "tree-vect.h"
> > > +
> > > +struct data
> > > +{
> > > +  unsigned int list[16];
> > > +  unsigned int sentinel;
> > > +};
> > > +
> > > +static struct data data __attribute__ ((aligned (64))) =
> > > +{
> > > +  { 0 },
> > > +  1
> > > +};
> > > +
> > > +__attribute__ ((noinline, noipa))
> > > +static int
> > > +contains (const unsigned int *p, unsigned int count, unsigned int start,
> > > +          unsigned int key)
> > > +{
> > > +  for (unsigned int i = start; i < count; ++i)
> > > +    if (p[i] == key)
> > > +      return 1;
> > > +
> > > +  return 0;
> > > +}
> > > +
> > > +int
> > > +main (void)
> > > +{
> > > +  check_vect ();
> > > +
> > > +  if (contains (data.list, 2, 1, 1) != 0)
> > > +    __builtin_abort ();
> > > +
> > > +  return 0;
> > > +}
> > > +
> > > +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */
> > > diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> > > index
> > 9d22d87705baaa60b96d973486bf38398c9ac3f2..37b1b83703718a7d0bc
> > 091ec6f755485855141af 100644
> > > --- a/gcc/tree-vect-loop-manip.cc
> > > +++ b/gcc/tree-vect-loop-manip.cc
> > > @@ -2832,6 +2832,30 @@ vect_gen_prolog_loop_niters (loop_vec_info
> > loop_vinfo,
> > >        *bound = align_in_elems;
> > >      }
> > >
> > > +  /* Peeling for alignment if the amount to peel is larger than scalar niters
> > > +     needs to cap the scalar niters to the MIN of niters and peeling.
> > Otherwise
> > > +     when we are calculating the number of iterations the peeled loop
> > performs
> > > +     we may cause an overflow.  i.e.
> > > +
> > > +     niters        = count - start = 2 - 1 = 1
> > > +     prolog_niters = 7
> > > +
> > > +     we then calculate the iteration count with
> > > +
> > > +     remaining = niters - prolog_niters;
> > > +
> > > +     which would overflow:
> > > +
> > > +     remaining = 1 - 7 = 4294967290 and then produce an incorrect value.  */
> > > +  if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)
> > > +      && !LOOP_VINFO_NITERS_UNCOUNTED_P (loop_vinfo))
> > > +    {
> > > +      tree niters = LOOP_VINFO_NITERS (loop_vinfo);
> > > +      if (!types_compatible_p (niters_type, TREE_TYPE (niters)))
> > > +       niters = fold_convert (niters_type, niters);
> > > +      iters = fold_build2 (MIN_EXPR, niters_type, iters, niters);
> > > +    }
> > > +
> > >    if (dump_enabled_p ())
> > >      dump_printf_loc (MSG_NOTE, vect_location,
> > >  		     "niters for prolog loop: %T\n", iters);
> > >
> > >
> > >
> > 
> > --
> > Richard Biener <rguenther@suse.de>
> > SUSE Software Solutions Germany GmbH,
> > Frankenstrasse 146, 90461 Nuernberg, Germany;
> > GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)
>
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c b/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
new file mode 100644
index 0000000000000000000000000000000000000000..f3bd8665c7fe39cff5e2be88521650fae9bf4c90
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_146-pr127677.c
@@ -0,0 +1,45 @@ 
+/* { dg-add-options vect_early_break } */
+/* { dg-require-effective-target vect_early_break_hw } */
+/* { dg-require-effective-target vect_partial_vectors } */
+/* { dg-require-effective-target vect_int } */
+
+/* { dg-additional-options "-O3" } */
+
+#include "tree-vect.h"
+
+struct data
+{
+  unsigned int list[16];
+  unsigned int sentinel;
+};
+
+static struct data data __attribute__ ((aligned (64))) =
+{
+  { 0 },
+  1
+};
+
+__attribute__ ((noinline, noipa))
+static int
+contains (const unsigned int *p, unsigned int count, unsigned int start,
+          unsigned int key)
+{
+  for (unsigned int i = start; i < count; ++i)
+    if (p[i] == key)
+      return 1;
+
+  return 0;
+}
+
+int
+main (void)
+{
+  check_vect ();
+
+  if (contains (data.list, 2, 1, 1) != 0)
+    __builtin_abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */
diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
index 9d22d87705baaa60b96d973486bf38398c9ac3f2..37b1b83703718a7d0bc091ec6f755485855141af 100644
--- a/gcc/tree-vect-loop-manip.cc
+++ b/gcc/tree-vect-loop-manip.cc
@@ -2832,6 +2832,30 @@  vect_gen_prolog_loop_niters (loop_vec_info loop_vinfo,
       *bound = align_in_elems;
     }
 
+  /* Peeling for alignment if the amount to peel is larger than scalar niters
+     needs to cap the scalar niters to the MIN of niters and peeling.  Otherwise
+     when we are calculating the number of iterations the peeled loop performs
+     we may cause an overflow.  i.e.
+
+     niters        = count - start = 2 - 1 = 1
+     prolog_niters = 7
+
+     we then calculate the iteration count with
+
+     remaining = niters - prolog_niters;
+
+     which would overflow:
+
+     remaining = 1 - 7 = 4294967290 and then produce an incorrect value.  */
+  if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)
+      && !LOOP_VINFO_NITERS_UNCOUNTED_P (loop_vinfo))
+    {
+      tree niters = LOOP_VINFO_NITERS (loop_vinfo);
+      if (!types_compatible_p (niters_type, TREE_TYPE (niters)))
+       niters = fold_convert (niters_type, niters);
+      iters = fold_build2 (MIN_EXPR, niters_type, iters, niters);
+    }
+
   if (dump_enabled_p ())
     dump_printf_loc (MSG_NOTE, vect_location,
 		     "niters for prolog loop: %T\n", iters);