[vect] PR92317: fix skip_epilogue creation for epilogues

Message ID 0da5fc53-8858-18ad-1fc4-b3aeb3c46f12@arm.com
State New
Headers show
Series
  • [vect] PR92317: fix skip_epilogue creation for epilogues
Related show

Commit Message

Andre Vieira (lists) Nov. 6, 2019, 10:57 a.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-06  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-06  Andre Vieira  <andre.simoesdiasvieira@arm.com>

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

Comments

Andre Vieira (lists) Nov. 6, 2019, 11:16 a.m. | #1
Sorry for the double post, ignore please.

On 06/11/2019 10:57, 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?

> 

> Cheers,

> Andre

> 

> gcc/ChangeLog:

> 

> 2019-11-06  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-06  Andre Vieira  <andre.simoesdiasvieira@arm.com>

> 

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

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;