aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakub Jelinek <jakub@redhat.com>2024-04-05 14:56:14 +0200
committerJakub Jelinek <jakub@redhat.com>2024-06-11 12:35:35 +0200
commitf8a327930b82e89ae1466cfacb9e8ac9f5c44e77 (patch)
tree88268caa5be204574a3167d9286d3560c4ffa7cf
parentf33e8ee4cb44e7a6326a894a9c153557238bde03 (diff)
downloadgcc-f8a327930b82e89ae1466cfacb9e8ac9f5c44e77.zip
gcc-f8a327930b82e89ae1466cfacb9e8ac9f5c44e77.tar.gz
gcc-f8a327930b82e89ae1466cfacb9e8ac9f5c44e77.tar.bz2
vect: Don't clear base_misaligned in update_epilogue_loop_vinfo [PR114566]
The following testcase is miscompiled, because in the vectorized epilogue the vectorizer assumes it can use aligned loads/stores (if the base decl gets alignment increased), but it actually doesn't increase that. This is because r10-4203-g97c1460367 added the hunk following patch removes. The explanation feels reasonable, but actually it is not true as the testcase proves. The thing is, we vectorize the main loop with 64-byte vectors and the corresponding data refs have base_alignment 16 (the a array has DECL_ALIGN 128) and offset_alignment 32. Now, because of the offset_alignment 32 rather than 64, we need to use unaligned loads/stores in the main loop (and ditto in the first load/store in vectorized epilogue). But the second load/store in the vectorized epilogue uses only 32-byte vectors and because it is a multiple of offset_alignment, it checks if we could increase alignment of the a VAR_DECL, the function returns true, sets base_misaligned = true and says the access is then aligned. But when update_epilogue_loop_vinfo clears base_misaligned with the assumption that the var had to have the alignment increased already, the update of DECL_ALIGN doesn't happen anymore. Now, I'd think this base_alignment = false was needed before r10-4030-gd2db7f7901 change was committed where it incorrectly overwrote DECL_ALIGN even if it was already larger, rather than just always increasing it. But with that change in, it doesn't make sense to me anymore. Note, the testcase is latent on the trunk, but reproduces on the 13 branch. 2024-04-05 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/114566 * tree-vect-loop.cc (update_epilogue_loop_vinfo): Don't clear base_misaligned. * gcc.target/i386/avx512f-pr114566.c: New test. (cherry picked from commit a844095e17c1a5aada1364c6f6eaade87ead463c)
-rw-r--r--gcc/testsuite/gcc.target/i386/avx512f-pr114566.c34
-rw-r--r--gcc/tree-vect-loop.cc8
2 files changed, 35 insertions, 7 deletions
diff --git a/gcc/testsuite/gcc.target/i386/avx512f-pr114566.c b/gcc/testsuite/gcc.target/i386/avx512f-pr114566.c
new file mode 100644
index 0000000..abfab1b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/avx512f-pr114566.c
@@ -0,0 +1,34 @@
+/* PR tree-optimization/114566 */
+/* { dg-do run } */
+/* { dg-options "-O3 -mavx512f" } */
+/* { dg-additional-options "-fstack-protector-strong" { target fstack_protector } } */
+/* { dg-require-effective-target avx512f } */
+
+#define AVX512F
+#include "avx512f-helper.h"
+
+__attribute__((noipa)) int
+foo (float x, float y)
+{
+ float a[8][56];
+ __builtin_memset (a, 0, sizeof (a));
+
+ for (int j = 0; j < 8; j++)
+ for (int k = 0; k < 56; k++)
+ {
+ float b = k * y;
+ if (b < 0.)
+ b = 0.;
+ if (b > 0.)
+ b = 0.;
+ a[j][k] += b;
+ }
+
+ return __builtin_log (x);
+}
+
+void
+TEST (void)
+{
+ foo (86.25f, 0.625f);
+}
diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
index fd0e5a7..1abc43f 100644
--- a/gcc/tree-vect-loop.cc
+++ b/gcc/tree-vect-loop.cc
@@ -9499,9 +9499,7 @@ find_in_mapping (tree t, void *context)
corresponding dr_vec_info need to be reconnected to the EPILOGUE's
stmt_vec_infos, their statements need to point to their corresponding copy,
if they are gather loads or scatter stores then their reference needs to be
- updated to point to its corresponding copy and finally we set
- 'base_misaligned' to false as we have already peeled for alignment in the
- prologue of the main loop. */
+ updated to point to its corresponding copy. */
static void
update_epilogue_loop_vinfo (class loop *epilogue, tree advance)
@@ -9642,10 +9640,6 @@ update_epilogue_loop_vinfo (class loop *epilogue, tree advance)
}
DR_STMT (dr) = STMT_VINFO_STMT (stmt_vinfo);
stmt_vinfo->dr_aux.stmt = stmt_vinfo;
- /* The vector size of the epilogue is smaller than that of the main loop
- so the alignment is either the same or lower. This means the dr will
- thus by definition be aligned. */
- STMT_VINFO_DR_INFO (stmt_vinfo)->base_misaligned = false;
}
epilogue_vinfo->shared->datarefs_copy.release ();