diff mbox series

Add missing alignment checks in epilogue loop vectorisation (PR 86877)

Message ID 87bm8s9z0g.fsf@arm.com
State New
Headers show
Series Add missing alignment checks in epilogue loop vectorisation (PR 86877) | expand

Commit Message

Richard Sandiford Sept. 20, 2018, 11:41 a.m. UTC
Epilogue loop vectorisation skips vect_enhance_data_refs_alignment
since it doesn't make sense to version or peel the epilogue loop
(that will already have happened for the main loop).  But this means
that it also fails to check whether the accesses are suitably aligned
for the new vector subarch.

We don't seem to carry alignment information from the (potentially
peeled or versioned) main loop to the epilogue loop, which would be
good to fix at some point.  I think we want this patch regardless,
since there's no guarantee that the alignment requirements are the
same for every subarch.

Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf
and x86_64-linux-gnu.  OK to install?

Richard


2018-09-20  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	PR tree-optimization/86877
	* tree-vect-loop.c (vect_analyze_loop_2): Call
	vect_verify_datarefs_alignment.

gcc/testsuite/
	PR tree-optimization/86877
	* gfortran.dg/vect/vect-8-epilogue.F90: New test.

Comments

Richard Biener Sept. 20, 2018, 12:47 p.m. UTC | #1
On Thu, Sep 20, 2018 at 1:42 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Epilogue loop vectorisation skips vect_enhance_data_refs_alignment
> since it doesn't make sense to version or peel the epilogue loop
> (that will already have happened for the main loop).  But this means
> that it also fails to check whether the accesses are suitably aligned
> for the new vector subarch.
>
> We don't seem to carry alignment information from the (potentially
> peeled or versioned) main loop to the epilogue loop, which would be
> good to fix at some point.  I think we want this patch regardless,
> since there's no guarantee that the alignment requirements are the
> same for every subarch.
>
> Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf
> and x86_64-linux-gnu.  OK to install?

OK.

Richard.

> Richard
>
>
> 2018-09-20  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         PR tree-optimization/86877
>         * tree-vect-loop.c (vect_analyze_loop_2): Call
>         vect_verify_datarefs_alignment.
>
> gcc/testsuite/
>         PR tree-optimization/86877
>         * gfortran.dg/vect/vect-8-epilogue.F90: New test.
>
> Index: gcc/tree-vect-loop.c
> ===================================================================
> --- gcc/tree-vect-loop.c        2018-09-20 12:39:06.341625036 +0100
> +++ gcc/tree-vect-loop.c        2018-09-20 12:39:14.541555902 +0100
> @@ -1979,20 +1979,21 @@ vect_analyze_loop_2 (loop_vec_info loop_
>    if (!ok)
>      return false;
>
> -  /* Do not invoke vect_enhance_data_refs_alignment for eplilogue
> -     vectorization.  */
> +  /* Do not invoke vect_enhance_data_refs_alignment for epilogue
> +     vectorization, since we do not want to add extra peeling or
> +     add versioning for alignment.  */
>    if (!LOOP_VINFO_EPILOGUE_P (loop_vinfo))
> -    {
>      /* This pass will decide on using loop versioning and/or loop peeling in
>         order to enhance the alignment of data references in the loop.  */
>      ok = vect_enhance_data_refs_alignment (loop_vinfo);
> -    if (!ok)
> -      {
> -       if (dump_enabled_p ())
> -         dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> -                          "bad data alignment.\n");
> -        return false;
> -      }
> +  else
> +    ok = vect_verify_datarefs_alignment (loop_vinfo);
> +  if (!ok)
> +    {
> +      if (dump_enabled_p ())
> +       dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +                        "bad data alignment.\n");
> +      return false;
>      }
>
>    if (slp)
> Index: gcc/testsuite/gfortran.dg/vect/vect-8-epilogue.F90
> ===================================================================
> --- /dev/null   2018-09-14 11:16:31.122530289 +0100
> +++ gcc/testsuite/gfortran.dg/vect/vect-8-epilogue.F90  2018-09-20 12:39:14.537555936 +0100
> @@ -0,0 +1,6 @@
> +! { dg-do compile }
> +! { dg-require-effective-target vect_double }
> +! { dg-additional-options "-finline-matmul-limit=0 --param vect-epilogues-nomask=1" }
> +! { dg-additional-options "-mstrict-align" { target { aarch64*-*-* } } }
> +
> +#include "vect-8.f90"
diff mbox series

Patch

Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c	2018-09-20 12:39:06.341625036 +0100
+++ gcc/tree-vect-loop.c	2018-09-20 12:39:14.541555902 +0100
@@ -1979,20 +1979,21 @@  vect_analyze_loop_2 (loop_vec_info loop_
   if (!ok)
     return false;
 
-  /* Do not invoke vect_enhance_data_refs_alignment for eplilogue
-     vectorization.  */
+  /* Do not invoke vect_enhance_data_refs_alignment for epilogue
+     vectorization, since we do not want to add extra peeling or
+     add versioning for alignment.  */
   if (!LOOP_VINFO_EPILOGUE_P (loop_vinfo))
-    {
     /* This pass will decide on using loop versioning and/or loop peeling in
        order to enhance the alignment of data references in the loop.  */
     ok = vect_enhance_data_refs_alignment (loop_vinfo);
-    if (!ok)
-      {
-	if (dump_enabled_p ())
-	  dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
-			   "bad data alignment.\n");
-        return false;
-      }
+  else
+    ok = vect_verify_datarefs_alignment (loop_vinfo);
+  if (!ok)
+    {
+      if (dump_enabled_p ())
+	dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
+			 "bad data alignment.\n");
+      return false;
     }
 
   if (slp)
Index: gcc/testsuite/gfortran.dg/vect/vect-8-epilogue.F90
===================================================================
--- /dev/null	2018-09-14 11:16:31.122530289 +0100
+++ gcc/testsuite/gfortran.dg/vect/vect-8-epilogue.F90	2018-09-20 12:39:14.537555936 +0100
@@ -0,0 +1,6 @@ 
+! { dg-do compile }
+! { dg-require-effective-target vect_double }
+! { dg-additional-options "-finline-matmul-limit=0 --param vect-epilogues-nomask=1" }
+! { dg-additional-options "-mstrict-align" { target { aarch64*-*-* } } }
+
+#include "vect-8.f90"