vect: Fix access size alignment assumption [PR115192]

create_intersect_range_checks checks whether two access ranges
a and b are alias-free using something equivalent to:

  end_a <= start_b || end_b <= start_a

It has two ways of doing this: a "vanilla" way that calculates
the exact exclusive end pointers, and another way that uses the
last inclusive aligned pointers (and changes the comparisons
accordingly).  The comment for the latter is:

      /* Calculate the minimum alignment shared by all four pointers,
	 then arrange for this alignment to be subtracted from the
	 exclusive maximum values to get inclusive maximum values.
	 This "- min_align" is cumulative with a "+ access_size"
	 in the calculation of the maximum values.  In the best
	 (and common) case, the two cancel each other out, leaving
	 us with an inclusive bound based only on seg_len.  In the
	 worst case we're simply adding a smaller number than before.

The problem is that the associated code implicitly assumed that the
access size was a multiple of the pointer alignment, and so the
alignment could be carried over to the exclusive end pointer.

The testcase started failing after g:9fa5b473b5b8e289b6542
because that commit improved the alignment information for
the accesses.

Tested on aarch64-linux-gnu & x86_64-linux-gnu.  OK for trunk
and backports?

Richard


gcc/
	PR tree-optimization/115192
	* tree-data-ref.cc (create_intersect_range_checks): Take the
	alignment of the access sizes into account.

gcc/testsuite/
	PR tree-optimization/115192
	* gcc.dg/vect/pr115192.c: New test.
---
 gcc/testsuite/gcc.dg/vect/pr115192.c | 28 ++++++++++++++++++++++++++++
 gcc/tree-data-ref.cc                 |  5 ++++-
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/vect/pr115192.c

On Fri, May 24, 2024 at 2:35 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> create_intersect_range_checks checks whether two access ranges
> a and b are alias-free using something equivalent to:
>
>   end_a <= start_b || end_b <= start_a
>
> It has two ways of doing this: a "vanilla" way that calculates
> the exact exclusive end pointers, and another way that uses the
> last inclusive aligned pointers (and changes the comparisons
> accordingly).  The comment for the latter is:
>
>       /* Calculate the minimum alignment shared by all four pointers,
>          then arrange for this alignment to be subtracted from the
>          exclusive maximum values to get inclusive maximum values.
>          This "- min_align" is cumulative with a "+ access_size"
>          in the calculation of the maximum values.  In the best
>          (and common) case, the two cancel each other out, leaving
>          us with an inclusive bound based only on seg_len.  In the
>          worst case we're simply adding a smaller number than before.
>
> The problem is that the associated code implicitly assumed that the
> access size was a multiple of the pointer alignment, and so the
> alignment could be carried over to the exclusive end pointer.
>
> The testcase started failing after g:9fa5b473b5b8e289b6542
> because that commit improved the alignment information for
> the accesses.
>
> Tested on aarch64-linux-gnu & x86_64-linux-gnu.  OK for trunk
> and backports?

OK.

Thanks,
Richard.

> Richard
>
>
> gcc/
>         PR tree-optimization/115192
>         * tree-data-ref.cc (create_intersect_range_checks): Take the
>         alignment of the access sizes into account.
>
> gcc/testsuite/
>         PR tree-optimization/115192
>         * gcc.dg/vect/pr115192.c: New test.
> ---
>  gcc/testsuite/gcc.dg/vect/pr115192.c | 28 ++++++++++++++++++++++++++++
>  gcc/tree-data-ref.cc                 |  5 ++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/vect/pr115192.c
>
> diff --git a/gcc/testsuite/gcc.dg/vect/pr115192.c b/gcc/testsuite/gcc.dg/vect/pr115192.c
> new file mode 100644
> index 00000000000..923d377c1bb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr115192.c
> @@ -0,0 +1,28 @@
> +#include "tree-vect.h"
> +
> +int data[4 * 16 * 16] __attribute__((aligned(16)));
> +
> +__attribute__((noipa)) void
> +foo (__SIZE_TYPE__ n)
> +{
> +  for (__SIZE_TYPE__ i = 1; i < n; ++i)
> +    {
> +      data[i * n * 4] = data[(i - 1) * n * 4] + 1;
> +      data[i * n * 4 + 1] = data[(i - 1) * n * 4 + 1] + 2;
> +    }
> +}
> +
> +int
> +main ()
> +{
> +  check_vect ();
> +
> +  data[0] = 10;
> +  data[1] = 20;
> +
> +  foo (3);
> +
> +  if (data[24] != 12 || data[25] != 24)
> +    __builtin_abort ();
> +  return 0;
> +}
> diff --git a/gcc/tree-data-ref.cc b/gcc/tree-data-ref.cc
> index db15ddb43de..7c4049faf34 100644
> --- a/gcc/tree-data-ref.cc
> +++ b/gcc/tree-data-ref.cc
> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
>
>  */
>
> +#define INCLUDE_ALGORITHM
>  #include "config.h"
>  #include "system.h"
>  #include "coretypes.h"
> @@ -2640,7 +2641,9 @@ create_intersect_range_checks (class loop *loop, tree *cond_expr,
>          Because the maximum values are inclusive, there is an alias
>          if the maximum value of one segment is equal to the minimum
>          value of the other.  */
> -      min_align = MIN (dr_a.align, dr_b.align);
> +      min_align = std::min (dr_a.align, dr_b.align);
> +      min_align = std::min (min_align, known_alignment (dr_a.access_size));
> +      min_align = std::min (min_align, known_alignment (dr_b.access_size));
>        cmp_code = LT_EXPR;
>      }
>
> --
> 2.25.1
>