[Bug target/105219] New: [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[Bug target/105219] New: [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[acoplan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

            Bug ID: 105219
           Summary: [12 Regression] SVE: Wrong code with -O3
                    -msve-vector-bits=128 -mtune=thunderx
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acoplan at gcc dot gnu.org
  Target Milestone: ---

The following C program appears to be miscompiled with -O3 -march=armv8.2-a+sve
-msve-vector-bits=128 -mtune=thunderx :

int a;
char b[60];
short c[18];
short d[4][19];
long long f;
void e(int g, int h, short k[][19]) {
  for (signed i = 0; i < 3; i += 2)
    for (signed j = 1; j < h + 14; j++) {
      b[i * 14 + j] = 1;
      c[i + j] = k[2][j];
      a = g ? k[i][j] : 0;
    }
}
int main() {
  e(9, 1, d);
  for (long l = 0; l < 6; ++l)
    for (long m = 0; m < 4; ++m)
      f ^= b[l + m * 4];
  if (f)
    __builtin_abort ();
}

Of course the choice of tuning is not sensible when asking for SVE, but GCC
shouldn't produce wrong code in any case. I can't reproduce the problem with
GCC 11.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tnfchris at gcc dot gnu.org
   Last reconfirmed|                            |2022-4-11

--- Comment #1 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
Smaller reproducer getting rid of the loop nest and simplify the inner
condition.

int a;                                                                         
                                                                               
                                                                               
                               char b[60];
short d[19];
long long f;

__attribute__ ((noinline, noipa))                                              
                                                                               
                                                                               
                               void e(int g, int h, short k[19]) {
    for (signed j = 1; j < h + 14; j++) {
      int i = 0;
      b[j] = 1;
      i = 2;
      b[i * 14 + j] = 1;
      a = g ? k[j] : 0;
    }
}

__attribute__ ((optimize("O0")))
int main() {
  e(9, 1, d);
  for (long l = 0; l < 6; ++l)
    for (long m = 0; m < 4; ++m)
      f ^= b[l + m * 4];
  if (f)
    __builtin_abort ();
}

and the -mtune=thunderx doesn't seem needed. I can reproduce with just -O3
-march=armv8.2-a+sve -msve-vector-bits=128.

the write to `a` itself is significant not for the result but to block the loop
from being optimized to a memset.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[ktkachov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |12.0
                 CC|                            |ktkachov at gcc dot gnu.org
           Priority|P3                          |P1

--- Comment #2 from ktkachov at gcc dot gnu.org ---
Confirmed then.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[acoplan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #3 from Alex Coplan <acoplan at gcc dot gnu.org> ---
(In reply to Tamar Christina from comment #1)
> Smaller reproducer getting rid of the loop nest and simplify the inner
> condition.

Hmm, I can't reproduce the issue with this locally (with or without the -mtune
option). I also seem to need the -mtune option with the original reproducer.
Are you definitely setting the right VL when running the compiled program?

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #4 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Alex Coplan from comment #3)
> (In reply to Tamar Christina from comment #1)
> > Smaller reproducer getting rid of the loop nest and simplify the inner
> > condition.
> 
> Hmm, I can't reproduce the issue with this locally (with or without the
> -mtune option). I also seem to need the -mtune option with the original
> reproducer. Are you definitely setting the right VL when running the
> compiled program?

Ah, my sve-max-vq was in the wrong place... I indeed see that the thunderx is
significant now. Will try to reduce a bit again.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #5 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
OK, I think this is an alignment issue.

When using the thunderx cost model it needs to peel the loop for alignment in
order to vectorize and it looks the error is there.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
eventually you can narrow down the mis-vectorized loop with
-fdbg-cnt=vect_loop:1-1 vs 2-2 (and the 2nd loop has its epilogue vectorized
for me?  so maybe try with --param vect-epilogues-nomask=0 as well.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #7 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
I have managed to find the commit where this starts failing:

commit 61fc5e098e76c9809f35f449a70c9c8d74773d9d (HEAD)
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Feb 18 11:34:52 2022 +0100

    tree-optimization/104582 - Simplify vectorizer cost API and fixes

    This simplifies the vectorizer cost API by providing overloads
    to add_stmt_cost and record_stmt_cost suitable for scalar stmt
    and branch stmt costing which do not need information like
    a vector type or alignment.  It also fixes two mistakes where
    costs for versioning tests were recorded as vector stmt rather
    than scalar stmt.

    This is a first patch to simplify the actual fix for PR104582.

    2022-02-18  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/104582
            * tree-vectorizer.h (add_stmt_cost): New overload.
            (record_stmt_cost): Likewise.
            * tree-vect-loop.cc (vect_compute_single_scalar_iteration_cost):
            Use add_stmt_costs.
            (vect_get_known_peeling_cost): Use new overloads.
            (vect_estimate_min_profitable_iters): Likewise.  Consistently
            use scalar_stmt for costing versioning checks.
            * tree-vect-stmts.cc (record_stmt_cost): New overload.

but I think this just exposes the issue.  The weird thing is while this does
change the cost calculations during vect the final vectorized code is exactly
the same out of both version. 

and indeed there's no change at all in the dumps until cunrolli, which in one
version unrolls more than the other, but I think that's just costing
difference. So I'm disabling it to continue seeing at which pass there's
actually codegen difference between these two commits.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #8 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
looks like the code out of the vectorizer is fine, and between the two versions
it doesn't change.

The big change is in the unroller, decides to drops one of the BBs for some
reason.

It turns the loop mask phi node from

 # loop_mask_374 = PHI <next_mask_416(51), max_mask_372(53), next_mask_378(66),
next_mask_397(68)>

into

 # loop_mask_374 = PHI <next_mask_397(51), max_mask_372(53), next_mask_378(66)>

and deletes the BB 68. And indeed -fdisable-tree-cunroll fixes the problem.

It looks like BB68 in the broken version is being marked as unreachable by DFA
and removed.

Curiously with no codegen difference in output of vect, or in dce6 and pcom,
the DFA in cunroll find different number of BBs

good:

Incremental SSA update started at block: 3
Number of blocks in CFG: 88
Number of blocks to update: 37 ( 42%)
Affected blocks: 4 18 28 34 45 47 50 51 53 54 56 58 59 60 61 62 63 64 65 66 67
68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83

bad:

Incremental SSA update started at block: 3
Number of blocks in CFG: 86
Number of blocks to update: 35 ( 41%)
Affected blocks: 4 18 28 34 45 47 50 51 53 54 56 58 59 60 61 62 63 64 65 66 67
68 69 70 71 72 73 74 75 76 77 78 79 80 81

why did it lose 2 BBs?

Is there some info missing from the dump files here?

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |tnfchris at gcc dot gnu.org

--- Comment #9 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
This was a bit annoying to track down...

So BB 68 gets merged into BB 51,

BB 51 is loading 4 ints 8 - 12 out of 14.

So after it there's still 2 ints left to process.  However it looks like the
compiler thinks that there can't be any more elements left.

It turns:

  next_mask_416 = .WHILE_ULT (12, bnd.38_326, { 0, 0, 0, 0 });
  if (next_mask_416 != { 0, 0, 0, 0 })
    goto <bb 51>; [75.00%]
  else
    goto <bb 56>; [25.00%]

into

  next_mask_373 = .WHILE_ULT (12, bnd.38_326, { 0, 0, 0, 0 });
  goto <bb 56>; [100.00%]

where bb 56 is the lastb reduction bit.  So basically we're 2 scalar loop
iterations short.

This seems to indicate like Richi suggested that nb_iterations_upper_bound is
set wrong.

Perhaps it was confused with the profitability threshold which went from 14 to
12.

Mine.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #10 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
nb_iterations_upper_bound is indeed set incorrectly and tracked to this commit,

commit 7ed1cd9665d8ca0fa07b2483e604c25e704584af
Author: Andre Vieira <andre.simoesdiasvieira@arm.com>
Date:   Thu Jun 3 13:55:24 2021 +0100

    vect: Use main loop's thresholds and VF to narrow upper_bound of epilogue

    This patch uses the knowledge of the conditions to enter an epilogue loop
to
    help come up with a potentially more restricive upper bound.

    gcc/ChangeLog:

            * tree-vect-loop.c (vect_transform_loop): Use main loop's various'
            thresholds to narrow the upper bound on epilogue iterations.

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/sve/part_vect_single_iter_epilog.c: New test.

I don't quite understand when comparing the two bounds there's a -1 in the min
comparison.

And indeed:

diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
index 5c7b163f01c..19235ea79fe 100644
--- a/gcc/tree-vect-loop.cc
+++ b/gcc/tree-vect-loop.cc
@@ -9971,7 +9971,7 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple
*loop_vectorized_call)
                                        LOOP_VINFO_VECT_FACTOR (loop_vinfo),
                                        &bound))
            loop->nb_iterations_upper_bound
-             = wi::umin ((widest_int) (bound - 1),
+             = wi::umin ((widest_int)bound,
                          loop->nb_iterations_upper_bound);
       }
   }

fixes it. It looks to me that when comparing the bounds of the main loop and
epilogue you shouldn't subtract 1 again. But need to ask why this is there.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|aarch64                     |aarch64, x86_64-*-*

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
int data[128];

void __attribute((noipa))
foo (int *data, int n)
{
  for (int i = 0; i < n; ++i)
    data[i] = i;
}

int main()
{
  for (int start = 0; start < 16; ++start)
    for (int n = 1; n < 3*16; ++n)
      {
        __builtin_memset (data, 0, sizeof (data));
        foo (&data[start], n);
        for (int j = 0; j < n; ++j)
          if (data[start + j] != j)
            __builtin_abort ();
      }
  return 0;
}

for example aborts with -O3 -mtune=intel -fno-vect-cost-model on x86_64,
the cost model disabling is necessary to have the epilogue vectorized.
Without a cost model we peel for the maximum number of aligned refs
but still use the target cost to decide whether peeling is worth at all.

[Bug target/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|tnfchris at gcc dot gnu.org        |avieira at gcc dot gnu.org

--- Comment #12 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
Won't have time to continue on this this week so passing it to Andre in the
interest of getting it unblocked sooner.

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #13 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #11)
> int data[128];
> 
> void __attribute((noipa))
> foo (int *data, int n)
> {
>   for (int i = 0; i < n; ++i)
>     data[i] = i;
> }
> 
> int main()
> {
>   for (int start = 0; start < 16; ++start)
>     for (int n = 1; n < 3*16; ++n)
>       {
>         __builtin_memset (data, 0, sizeof (data));
>         foo (&data[start], n);
>         for (int j = 0; j < n; ++j)
>           if (data[start + j] != j)
>             __builtin_abort ();
>       }
>   return 0;
> }
> 
> for example aborts with -O3 -mtune=intel -fno-vect-cost-model on x86_64,
> the cost model disabling is necessary to have the epilogue vectorized.
> Without a cost model we peel for the maximum number of aligned refs
> but still use the target cost to decide whether peeling is worth at all.

This one started with r12-1181-g7ed1cd9665d8ca0f.

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #13)
> (In reply to Richard Biener from comment #11)
> > int data[128];
> > 
> > void __attribute((noipa))
> > foo (int *data, int n)
> > {
> >   for (int i = 0; i < n; ++i)
> >     data[i] = i;
> > }
> > 
> > int main()
> > {
> >   for (int start = 0; start < 16; ++start)
> >     for (int n = 1; n < 3*16; ++n)
> >       {
> >         __builtin_memset (data, 0, sizeof (data));
> >         foo (&data[start], n);
> >         for (int j = 0; j < n; ++j)
> >           if (data[start + j] != j)
> >             __builtin_abort ();
> >       }
> >   return 0;
> > }
> > 
> > for example aborts with -O3 -mtune=intel -fno-vect-cost-model on x86_64,
> > the cost model disabling is necessary to have the epilogue vectorized.
> > Without a cost model we peel for the maximum number of aligned refs
> > but still use the target cost to decide whether peeling is worth at all.
> 
> This one started with r12-1181-g7ed1cd9665d8ca0f.

So the following fixes the x86 failure for me.  Can somebody check if that
also fixes the aarch64 issue observed?

diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
index d7bc34636bd..3b63ab7b669 100644
--- a/gcc/tree-vect-loop.cc
+++ b/gcc/tree-vect-loop.cc
@@ -9977,7 +9981,7 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple
*loop_vectorized_call)
                            lowest_vf) - 1
           : wi::udiv_floor (loop->nb_iterations_upper_bound + bias_for_lowest,
                             lowest_vf) - 1);
-      if (main_vinfo)
+      if (main_vinfo && !main_vinfo->peeling_for_alignment)
        {
          unsigned int bound;
          poly_uint64 main_iters

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #15 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #14)
> diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> index d7bc34636bd..3b63ab7b669 100644
> --- a/gcc/tree-vect-loop.cc
> +++ b/gcc/tree-vect-loop.cc
> @@ -9977,7 +9981,7 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple
> *loop_vectorized_call)
>                             lowest_vf) - 1
>            : wi::udiv_floor (loop->nb_iterations_upper_bound +
> bias_for_lowest,
>                              lowest_vf) - 1);
> -      if (main_vinfo)
> +      if (main_vinfo && !main_vinfo->peeling_for_alignment)
>         {
>           unsigned int bound;
>           poly_uint64 main_iters
It might be better to add the maximum peeling amount to main_iters.
Maybe you'd prefer this anyway for GCC 12 though.

I wonder if there's a similar problem for peeling for gaps,
in cases where the epilogue doesn't need the same peeling.

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #15)
> (In reply to Richard Biener from comment #14)
> > diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> > index d7bc34636bd..3b63ab7b669 100644
> > --- a/gcc/tree-vect-loop.cc
> > +++ b/gcc/tree-vect-loop.cc
> > @@ -9977,7 +9981,7 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple
> > *loop_vectorized_call)
> >                             lowest_vf) - 1
> >            : wi::udiv_floor (loop->nb_iterations_upper_bound +
> > bias_for_lowest,
> >                              lowest_vf) - 1);
> > -      if (main_vinfo)
> > +      if (main_vinfo && !main_vinfo->peeling_for_alignment)
> >         {
> >           unsigned int bound;
> >           poly_uint64 main_iters
> It might be better to add the maximum peeling amount to main_iters.
> Maybe you'd prefer this anyway for GCC 12 though.
> 
> I wonder if there's a similar problem for peeling for gaps,
> in cases where the epilogue doesn't need the same peeling.

I don't quite understand the code in if (main_vinfo) but the point is
that for our case main_iters is zero (and so is prologue_iters if that
would exist).  I'm not sure how the code can be adjusted with that
given it computes upper bounds and uses min() for the upper bound
of the epilogue - we'd need to adjust that with a max (2*vf-2, old-upper-bound)
when there's prologue peeling and the short cut exists (I don't actually
compute that).

peeling for gaps means we run the epilogue for main VF more iterations,
but that would just mean the vectorized epilogue executes one more time
and has peeling for gaps applied as well, so the scalar epilogue runs
for epilogue VF more iterations.

I'm not sure what conditions prevent epilogue vectorization but I think
there were some at least.

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #16)
> (In reply to rsandifo@gcc.gnu.org from comment #15)
> > (In reply to Richard Biener from comment #14)
> > > diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> > > index d7bc34636bd..3b63ab7b669 100644
> > > --- a/gcc/tree-vect-loop.cc
> > > +++ b/gcc/tree-vect-loop.cc
> > > @@ -9977,7 +9981,7 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple
> > > *loop_vectorized_call)
> > >                             lowest_vf) - 1
> > >            : wi::udiv_floor (loop->nb_iterations_upper_bound +
> > > bias_for_lowest,
> > >                              lowest_vf) - 1);
> > > -      if (main_vinfo)
> > > +      if (main_vinfo && !main_vinfo->peeling_for_alignment)
> > >         {
> > >           unsigned int bound;
> > >           poly_uint64 main_iters
> > It might be better to add the maximum peeling amount to main_iters.
> > Maybe you'd prefer this anyway for GCC 12 though.
> > 
> > I wonder if there's a similar problem for peeling for gaps,
> > in cases where the epilogue doesn't need the same peeling.
> 
> I don't quite understand the code in if (main_vinfo) but the point is
> that for our case main_iters is zero (and so is prologue_iters if that
> would exist).  I'm not sure how the code can be adjusted with that
> given it computes upper bounds and uses min() for the upper bound
> of the epilogue - we'd need to adjust that with a max (2*vf-2,
> old-upper-bound)
> when there's prologue peeling and the short cut exists (I don't actually
> compute that).

That is, the code does

          if (can_div_away_from_zero_p (main_iters,
                                        LOOP_VINFO_VECT_FACTOR (loop_vinfo),
                                        &bound))
            loop->nb_iterations_upper_bound
              = wi::umin ((widest_int) (bound - 1),
                          loop->nb_iterations_upper_bound);

and so assumes that the scalar epilogue never runs for more than epilogue
VF - 1 times which is wrong.  So I simply gated this whole code.  But
you are right that peeling for gaps would need similar handling so I'll
play safe and add && !main_vinfo->peeling_for_gaps. 

> 
> peeling for gaps means we run the epilogue for main VF more iterations,
> but that would just mean the vectorized epilogue executes one more time
> and has peeling for gaps applied as well, so the scalar epilogue runs
> for epilogue VF more iterations.
> 
> I'm not sure what conditions prevent epilogue vectorization but I think
> there were some at least.

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[avieira at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #18 from avieira at gcc dot gnu.org ---
(In reply to Richard Biener from comment #16)
> (In reply to rsandifo@gcc.gnu.org from comment #15)
> > (In reply to Richard Biener from comment #14)
> > > diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> > > index d7bc34636bd..3b63ab7b669 100644
> > > --- a/gcc/tree-vect-loop.cc
> > > +++ b/gcc/tree-vect-loop.cc
> > > @@ -9977,7 +9981,7 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple
> > > *loop_vectorized_call)
> > >                             lowest_vf) - 1
> > >            : wi::udiv_floor (loop->nb_iterations_upper_bound +
> > > bias_for_lowest,
> > >                              lowest_vf) - 1);
> > > -      if (main_vinfo)
> > > +      if (main_vinfo && !main_vinfo->peeling_for_alignment)
> > >         {
> > >           unsigned int bound;
> > >           poly_uint64 main_iters
> > It might be better to add the maximum peeling amount to main_iters.
> > Maybe you'd prefer this anyway for GCC 12 though.
> > 
> > I wonder if there's a similar problem for peeling for gaps,
> > in cases where the epilogue doesn't need the same peeling.
> 
> I don't quite understand the code in if (main_vinfo) but the point is
> that for our case main_iters is zero (and so is prologue_iters if that
> would exist).  I'm not sure how the code can be adjusted with that
> given it computes upper bounds and uses min() for the upper bound
> of the epilogue - we'd need to adjust that with a max (2*vf-2,
> old-upper-bound)
> when there's prologue peeling and the short cut exists (I don't actually
> compute that).
> 
> peeling for gaps means we run the epilogue for main VF more iterations,
> but that would just mean the vectorized epilogue executes one more time
> and has peeling for gaps applied as well, so the scalar epilogue runs
> for epilogue VF more iterations.
> 
> I'm not sure what conditions prevent epilogue vectorization but I think
> there were some at least.


I think disabling this for peeling makes sense for now, but just to explain how
the code works.

The perhaps misnamed 'main_iters' represents the maximum number of iterations
left to do after the main loop, either entered or not. The maximum number of
iterations left to do after the main loop the largest of the three:
 - the main loop's VF, in case we enter the main loop there are at most VF-1
iterations left, I see I didn't add a -1 there.
 - LOOP_VINFO_COST_MODEL_THRESHOLD or LOOP_VINFO_VERSIONING_THRESHOLD in case
we don't enter the main loop because we don't have enough iterations to meet
these (but do still have enough for the epilogue).

Our problem is that this didn't take peeling into account, since skipping main
-> skipping peeling and thus really the number of iters we could be left with
after skipping main are actually main_iters + to-peel.

So I think the approach should be to add 'to_peel' to main_iters where
'to_peel' is either:
VF - 1 if PEELING_FOR_GAPS or PEELING_FOR_ALIGNMENT = -1
PEELING_FOR_ALIGNMENT otherwise.

But like I said first, disabling is probably the safest and easiest for gcc 12
and given the niche of this, I'm not even sure it's worth tightening it for gcc
13?

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:f27848a5dc4d3b16cd4112bddcb59e0916eba623

commit r12-8297-gf27848a5dc4d3b16cd4112bddcb59e0916eba623
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Apr 27 14:06:12 2022 +0200

    tree-optimization/105219 - bogus max iters for vectorized epilogue

    The following makes sure to take into account prologue peeling
    when trying to narrow down the maximum number of iterations
    computed for the vectorized epilogue.  A similar issue exists when
    peeling for gaps.

    2022-04-27  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/105219
            * tree-vect-loop.cc (vect_transform_loop): Disable
            special code narrowing the vectorized epilogue max
            iterations when peeling for alignment or gaps was in effect.

            * gcc.dg/vect/pr105219.c: New testcase.

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #20 from Richard Biener <rguenth at gcc dot gnu.org> ---
Should be fixed - please verify on the aarch64 testcase and consider adding it
to the aarch64 specific parts of the testsuite.

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #21 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/105219] [12 Regression] SVE: Wrong code with -O3 -msve-vector-bits=128 -mtune=thunderx

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105219

--- Comment #22 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andre Simoes Dias Vieira
<avieira@gcc.gnu.org>:

https://gcc.gnu.org/g:210cda732832ec589b02c722b2c73f7fa50b97ce

commit r13-47-g210cda732832ec589b02c722b2c73f7fa50b97ce
Author: Andre Vieira <andre.simoesdiasvieira@arm.com>
Date:   Fri Apr 29 16:31:08 2022 +0100

    aarch64: Add option to pr105219 testcase

    Add option that originally caused testcase to fail for aarch64.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/105219
            * gcc.dg/vect/pr105219.c: Add aarch64 target option.