[vect] PR92317: fix skip_epilogue creation for epilogues

Message ID 0bdc7221-6cb7-078f-e0a7-485bd5dbc57c@arm.com
State Superseded
Headers show
Series
  • [vect] PR92317: fix skip_epilogue creation for epilogues
Related show

Commit Message

Andre Vieira (lists) Nov. 5, 2019, 12:06 p.m.
Hi,

When investigating PR92317 I noticed that when we create the skip 
epilogue condition, see ('if (skip_epilog)' in 'vect_do_peeling'), we 
only copy phi-nodes that are not constants in 
'slpeel_update_phi_nodes_for_guard2'.  This can cause problems later 
when we create the scalar epilogue for this epilogue, since if the 
'scalar_loop' is not the same as 'loop' 
'slpeel_tree_duplicate_loop_to_edge_cfg' will expect both to have 
identical single_exit bb's and use that to copy the current_def 
meta_data of phi-nodes.

This makes sure that is true even if these phi-nodes are constants, 
fixing PR92317.  I copied the failing testcase and added the options 
that originally made it fail.

Is this OK for trunk?

Cheers,
Andre

gcc/ChangeLog:

2019-11-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>

        * tree-vect-loop-manip.c (slpeel_update_phi_nodes_for_guard2): Also
        update phi's with constant phi arguments.


gcc/testsuite/ChangeLog:
2019-11-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>

        * gcc/testsuite/g++.dg/opt/pr92317.C

Comments

Richard Biener Nov. 6, 2019, 8 a.m. | #1
On Tue, 5 Nov 2019, Andre Vieira (lists) wrote:

> Hi,

> 

> When investigating PR92317 I noticed that when we create the skip epilogue

> condition, see ('if (skip_epilog)' in 'vect_do_peeling'), we only copy

> phi-nodes that are not constants in 'slpeel_update_phi_nodes_for_guard2'.

> This can cause problems later when we create the scalar epilogue for this

> epilogue, since if the 'scalar_loop' is not the same as 'loop'

> 'slpeel_tree_duplicate_loop_to_edge_cfg' will expect both to have identical

> single_exit bb's and use that to copy the current_def meta_data of phi-nodes.

> 

> This makes sure that is true even if these phi-nodes are constants, fixing

> PR92317.  I copied the failing testcase and added the options that originally

> made it fail.

> 

> Is this OK for trunk?


OK.

Thanks,
Richard.

Patch

diff --git a/gcc/testsuite/g++.dg/opt/pr92317.C b/gcc/testsuite/g++.dg/opt/pr92317.C
new file mode 100644
index 0000000000000000000000000000000000000000..2bb9729fc961a998db9a88045ee04a81e12b07a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/pr92317.C
@@ -0,0 +1,51 @@ 
+// Copied from pr87967.C
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2 -ftree-vectorize -fno-tree-pre --param vect-epilogues-nomask=1" }
+
+void h();
+template <typename b> struct k { using d = b; };
+template <typename b, template <typename> class> using e = k<b>;
+template <typename b, template <typename> class f>
+using g = typename e<b, f>::d;
+struct l {
+  template <typename i> using ab = typename i::j;
+};
+struct n : l {
+  using j = g<char *, ab>;
+};
+class o {
+public:
+  long r();
+};
+char m;
+char s() {
+  if (m)
+    return '0';
+  return 'A';
+}
+class t {
+public:
+  typedef char *ad;
+  ad m_fn2();
+};
+void fn3() {
+  char *a;
+  t b;
+  bool p = false;
+  while (*a) {
+    h();
+    o c;
+    if (*a)
+      a++;
+    if (c.r()) {
+      n::j q;
+      for (t::ad d = b.m_fn2(), e; d != e; d++) {
+        char f = *q;
+        *d = f + s();
+      }
+      p = true;
+    }
+  }
+  if (p)
+    throw;
+}
diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
index 1fbcaf2676f3a099cccc5d4f63e18f4a689abeff..54f3ccf3ec373b5621e7778e6e80bab853a57687 100644
--- a/gcc/tree-vect-loop-manip.c
+++ b/gcc/tree-vect-loop-manip.c
@@ -2291,12 +2291,14 @@  slpeel_update_phi_nodes_for_guard2 (class loop *loop, class loop *epilog,
     {
       gphi *update_phi = gsi.phi ();
       tree old_arg = PHI_ARG_DEF (update_phi, 0);
-      /* This loop-closed-phi actually doesn't represent a use out of the
-	 loop - the phi arg is a constant.  */
-      if (TREE_CODE (old_arg) != SSA_NAME)
-	continue;
 
-      tree merge_arg = get_current_def (old_arg);
+      tree merge_arg = NULL_TREE;
+
+      /* If the old argument is a SSA_NAME use its current_def.  */
+      if (TREE_CODE (old_arg) == SSA_NAME)
+	merge_arg = get_current_def (old_arg);
+      /* If it's a constant or doesn't have a current_def, just use the old
+	 argument.  */
       if (!merge_arg)
 	merge_arg = old_arg;