Reject versioning for alignment with different masks (PR 92526)
diff mbox series

Message ID mpt8soalg7n.fsf@arm.com
State New
Headers show
Series
  • Reject versioning for alignment with different masks (PR 92526)
Related show

Commit Message

Richard Sandiford Nov. 20, 2019, 3:28 p.m. UTC
Allowing mixed vector sizes broke the assumption in the following assert,
since it's now possible for different accesses to require different
levels of alignment:

              /* FORNOW: use the same mask to test all potentially unaligned
                 references in the loop.  The vectorizer currently supports
                 a single vector size, see the reference to
                 GET_MODE_NUNITS (TYPE_MODE (vectype)) where the
                 vectorization factor is computed.  */
              gcc_assert (!LOOP_VINFO_PTR_MASK (loop_vinfo)
                          || LOOP_VINFO_PTR_MASK (loop_vinfo) == mask);

I guess we could try to over-align smaller accesses so that all
of them are consistent, or try to support multiple alignment masks,
but for now the easiest fix seems to be to turn the assert into a
bail-out check.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2019-11-20  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	PR tree-optimization/92526
	* tree-vect-data-refs.c (vect_enhance_data_refs_alignment): Reject
	versioning for alignment if the accesses do not have a consistent
	mask, rather than asserting that the masks are consistent.

gcc/testsuite/
	PR tree-optimization/92526
	* gcc.target/aarch64/pr92526.c: New test.

Comments

Richard Biener Nov. 21, 2019, 2:11 p.m. UTC | #1
On Wed, Nov 20, 2019 at 4:28 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Allowing mixed vector sizes broke the assumption in the following assert,
> since it's now possible for different accesses to require different
> levels of alignment:
>
>               /* FORNOW: use the same mask to test all potentially unaligned
>                  references in the loop.  The vectorizer currently supports
>                  a single vector size, see the reference to
>                  GET_MODE_NUNITS (TYPE_MODE (vectype)) where the
>                  vectorization factor is computed.  */
>               gcc_assert (!LOOP_VINFO_PTR_MASK (loop_vinfo)
>                           || LOOP_VINFO_PTR_MASK (loop_vinfo) == mask);
>
> I guess we could try to over-align smaller accesses so that all
> of them are consistent, or try to support multiple alignment masks,
> but for now the easiest fix seems to be to turn the assert into a
> bail-out check.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

OK.

> Richard
>
>
> 2019-11-20  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         PR tree-optimization/92526
>         * tree-vect-data-refs.c (vect_enhance_data_refs_alignment): Reject
>         versioning for alignment if the accesses do not have a consistent
>         mask, rather than asserting that the masks are consistent.
>
> gcc/testsuite/
>         PR tree-optimization/92526
>         * gcc.target/aarch64/pr92526.c: New test.
>
> Index: gcc/tree-vect-data-refs.c
> ===================================================================
> --- gcc/tree-vect-data-refs.c   2019-11-16 11:40:19.105159717 +0000
> +++ gcc/tree-vect-data-refs.c   2019-11-20 15:27:49.385346722 +0000
> @@ -2266,13 +2266,15 @@ vect_enhance_data_refs_alignment (loop_v
>                   mask must be 15 = 0xf. */
>               mask = size - 1;
>
> -              /* FORNOW: use the same mask to test all potentially unaligned
> -                 references in the loop.  The vectorizer currently supports
> -                 a single vector size, see the reference to
> -                 GET_MODE_NUNITS (TYPE_MODE (vectype)) where the
> -                 vectorization factor is computed.  */
> -              gcc_assert (!LOOP_VINFO_PTR_MASK (loop_vinfo)
> -                          || LOOP_VINFO_PTR_MASK (loop_vinfo) == mask);
> +             /* FORNOW: use the same mask to test all potentially unaligned
> +                references in the loop.  */
> +             if (LOOP_VINFO_PTR_MASK (loop_vinfo)
> +                 && LOOP_VINFO_PTR_MASK (loop_vinfo) != mask)
> +               {
> +                 do_versioning = false;
> +                 break;
> +               }
> +
>                LOOP_VINFO_PTR_MASK (loop_vinfo) = mask;
>               LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo).safe_push (stmt_info);
>              }
> Index: gcc/testsuite/gcc.target/aarch64/pr92526.c
> ===================================================================
> --- /dev/null   2019-09-17 11:41:18.176664108 +0100
> +++ gcc/testsuite/gcc.target/aarch64/pr92526.c  2019-11-20 15:27:49.385346722 +0000
> @@ -0,0 +1,9 @@
> +/* { dg-options "-O3 -mstrict-align" } */
> +
> +void
> +f (unsigned int *restrict x, unsigned int *restrict y,
> +   unsigned char *restrict z, unsigned int n)
> +{
> +  for (unsigned int i = 0; i < n % 4; ++i)
> +    x[i] = x[i] + y[i] + z[i];
> +}

Patch
diff mbox series

Index: gcc/tree-vect-data-refs.c
===================================================================
--- gcc/tree-vect-data-refs.c	2019-11-16 11:40:19.105159717 +0000
+++ gcc/tree-vect-data-refs.c	2019-11-20 15:27:49.385346722 +0000
@@ -2266,13 +2266,15 @@  vect_enhance_data_refs_alignment (loop_v
                  mask must be 15 = 0xf. */
 	      mask = size - 1;
 
-              /* FORNOW: use the same mask to test all potentially unaligned
-                 references in the loop.  The vectorizer currently supports
-                 a single vector size, see the reference to
-                 GET_MODE_NUNITS (TYPE_MODE (vectype)) where the
-                 vectorization factor is computed.  */
-              gcc_assert (!LOOP_VINFO_PTR_MASK (loop_vinfo)
-                          || LOOP_VINFO_PTR_MASK (loop_vinfo) == mask);
+	      /* FORNOW: use the same mask to test all potentially unaligned
+		 references in the loop.  */
+	      if (LOOP_VINFO_PTR_MASK (loop_vinfo)
+		  && LOOP_VINFO_PTR_MASK (loop_vinfo) != mask)
+		{
+		  do_versioning = false;
+		  break;
+		}
+
               LOOP_VINFO_PTR_MASK (loop_vinfo) = mask;
 	      LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo).safe_push (stmt_info);
             }
Index: gcc/testsuite/gcc.target/aarch64/pr92526.c
===================================================================
--- /dev/null	2019-09-17 11:41:18.176664108 +0100
+++ gcc/testsuite/gcc.target/aarch64/pr92526.c	2019-11-20 15:27:49.385346722 +0000
@@ -0,0 +1,9 @@ 
+/* { dg-options "-O3 -mstrict-align" } */
+
+void
+f (unsigned int *restrict x, unsigned int *restrict y,
+   unsigned char *restrict z, unsigned int n)
+{
+  for (unsigned int i = 0; i < n % 4; ++i)
+    x[i] = x[i] + y[i] + z[i];
+}