[vect] PR92429: do not fold when updating epilogue statements

Message ID 03a1edb5-f1d2-9285-70d3-ea008a765bd0@arm.com
State New
Headers show
Series
  • [vect] PR92429: do not fold when updating epilogue statements
Related show

Commit Message

Andre Vieira (lists) Jan. 10, 2020, 1:51 p.m.
Hi,

This patch addresses the problem reported in PR92429.  When creating an 
epilogue for vectorization we have to replace the SSA_NAMEs in the 
PATTERN_DEF_SEQs and RELATED_STMTs of the epilogue's loop_vec_infos. 
When doing this we were using simplify_replace_tree which always folds 
the replacement.  This may lead to a different tree-node than the one 
which was analyzed in vect_loop_analyze.  In turn the new tree-node may 
require a different vectorization than the one we had prepared for which 
caused the ICE in question.

This patch adds a parameter to the simplify_replace_tree function such 
that we can optionally disable folding as this patch does in 
update_epilogue_loop_vinfo.

Bootstrapped and regression tested on x86_64.

Is this OK for trunk?

gcc/ChangeLog:
2020-01-10  Andre Vieira  <andre.simoesdiasvieira@arm.com>

         PR tree-optimization/92429
         * tree-ssa-loop-niter.h (simplify_replace_tree): Add parameter.
         * tree-ssa-loop-niter.c (simplify_replace_tree): Add parameter
         to control folding.
         * tree-vect-loop.c (update_epilogue_vinfo): Do not fold when
         replacing tree.

gcc/testsuite/ChangeLog:
2020-01-10  Andre Vieira  <andre.simoesdiasvieira@arm.com>

         PR tree-optimization/92429
         * gcc.dg/vect/pr92429.c

Comments

Richard Biener Jan. 13, 2020, 9:32 a.m. | #1
On Fri, 10 Jan 2020, Andre Vieira (lists) wrote:

> Hi,

> 

> This patch addresses the problem reported in PR92429.  When creating an

> epilogue for vectorization we have to replace the SSA_NAMEs in the

> PATTERN_DEF_SEQs and RELATED_STMTs of the epilogue's loop_vec_infos. When

> doing this we were using simplify_replace_tree which always folds the

> replacement.  This may lead to a different tree-node than the one which was

> analyzed in vect_loop_analyze.  In turn the new tree-node may require a

> different vectorization than the one we had prepared for which caused the ICE

> in question.

> 

> This patch adds a parameter to the simplify_replace_tree function such that we

> can optionally disable folding as this patch does in

> update_epilogue_loop_vinfo.

> 

> Bootstrapped and regression tested on x86_64.

> 

> Is this OK for trunk?


OK.

Thanks,
Richard.

> gcc/ChangeLog:

> 2020-01-10  Andre Vieira  <andre.simoesdiasvieira@arm.com>

> 

>         PR tree-optimization/92429

>         * tree-ssa-loop-niter.h (simplify_replace_tree): Add parameter.

>         * tree-ssa-loop-niter.c (simplify_replace_tree): Add parameter

>         to control folding.

>         * tree-vect-loop.c (update_epilogue_vinfo): Do not fold when

>         replacing tree.

> 

> gcc/testsuite/ChangeLog:

> 2020-01-10  Andre Vieira  <andre.simoesdiasvieira@arm.com>

> 

>         PR tree-optimization/92429

>         * gcc.dg/vect/pr92429.c

> 

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Patch

diff --git a/gcc/testsuite/gcc.dg/vect/pr92429.c b/gcc/testsuite/gcc.dg/vect/pr92429.c
new file mode 100644
index 0000000000000000000000000000000000000000..7885cd6e4c2639f553a68edcbd70203d1b7b13bb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr92429.c
@@ -0,0 +1,14 @@ 
+/* PR92429 */
+/* { dg-do compile } */
+/* { dg-additional-options "-O1 -fno-tree-fre" } */
+/* { dg-additional-options "-mavx2" { target { x86_64-*-* i?86-*-* } } } */
+
+void
+as (int *gl, int k1)
+{
+  while (k1 < 1)
+    {
+      gl[k1] = gl[k1] * gl[k1] / 2;
+      ++k1;
+    }
+}
diff --git a/gcc/tree-ssa-loop-niter.h b/gcc/tree-ssa-loop-niter.h
index 621e2c2e28dfbeda279687dcae78e9c4fbe7f559..eb8d1579479cf6df66f9486c7ce6f9ac20a02e2a 100644
--- a/gcc/tree-ssa-loop-niter.h
+++ b/gcc/tree-ssa-loop-niter.h
@@ -58,7 +58,7 @@  extern void free_numbers_of_iterations_estimates (class loop *);
 extern void free_numbers_of_iterations_estimates (function *);
 extern tree simplify_replace_tree (tree, tree,
 				   tree, tree (*)(tree, void *) = NULL,
-				   void * = NULL);
+				   void * = NULL, bool do_fold = true);
 extern void substitute_in_loop_info (class loop *, tree, tree);
 
 #endif /* GCC_TREE_SSA_LOOP_NITER_H */
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index 4d5e04945118880d0c0cfb00d9c5841cc58e46f5..6e6df0bfdb898de948c917640ff488b19cf7911f 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -1934,7 +1934,8 @@  number_of_iterations_cond (class loop *loop,
 
 tree
 simplify_replace_tree (tree expr, tree old, tree new_tree,
-		       tree (*valueize) (tree, void*), void *context)
+		       tree (*valueize) (tree, void*), void *context,
+		       bool do_fold)
 {
   unsigned i, n;
   tree ret = NULL_TREE, e, se;
@@ -1966,7 +1967,7 @@  simplify_replace_tree (tree expr, tree old, tree new_tree,
   for (i = 0; i < n; i++)
     {
       e = TREE_OPERAND (expr, i);
-      se = simplify_replace_tree (e, old, new_tree, valueize, context);
+      se = simplify_replace_tree (e, old, new_tree, valueize, context, do_fold);
       if (e == se)
 	continue;
 
@@ -1976,7 +1977,7 @@  simplify_replace_tree (tree expr, tree old, tree new_tree,
       TREE_OPERAND (ret, i) = se;
     }
 
-  return (ret ? fold (ret) : expr);
+  return (ret ? (do_fold ? fold (ret) : ret) : expr);
 }
 
 /* Expand definitions of ssa names in EXPR as long as they are simple
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index cf9b7bcb9c41d678b326b41a4167f1e9c00cc5a4..f30e104e9958c0ebaf170004747ff436b1c1d855 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -8440,8 +8440,13 @@  update_epilogue_loop_vinfo (class loop *epilogue, tree advance,
 	    gimple_set_op (stmt, j, *new_op);
 	  else
 	    {
+	      /* PR92429: The last argument of simplify_replace_tree disables
+		 folding when replacing arguments.  This is required as
+		 otherwise you might end up with different statements than the
+		 ones analyzed in vect_loop_analyze, leading to different
+		 vectorization.  */
 	      op = simplify_replace_tree (op, NULL_TREE, NULL_TREE,
-				     &find_in_mapping, &mapping);
+					  &find_in_mapping, &mapping, false);
 	      gimple_set_op (stmt, j, op);
 	    }
 	}