Re: [PATCH] Fixing PR60000: A bug in the vectorizer.

[Cong Hou <congh at google dot com>]

On Fri, Jan 31, 2014 at 5:06 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, Jan 31, 2014 at 09:41:59AM +0100, Richard Biener wrote:
> > Is that because si and pattern_def_si point to the same stmts?  Then
> > I'd prefer to do
> >
> >   if (is_store)
> >    {
> >      ...
> >      pattern_def_seq = NULL;
> >    }
> >  else if (!transform_pattern_stmt && gsi_end_p (pattern_def_si))
> >    {
> >      pattern_def_seq = NULL;
> >      gsi_next (&si);
> >    }
>
> Yeah, I think stores can only appear at the end of patterns, so IMHO it should be
> safe to just clear pattern_def_seq always in that case.  Right now the code
> has continue; separately for STMT_VINFO_GROUPED_ACCESS (stmt_info) and
> for !STMT_VINFO_GROUPED_ACCESS (stmt_info) stores, but I guess you should
> just move them at the end of if (is_store) and clear pattern_def_seq there
> before the continue.  Add gcc_assert (!transform_pattern_stmt); too?


I agree. I have updated the patch accordingly. Bootstrapped and tested
on x86_64. OK for the trunk?

thanks,
Cong




diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 95a324c..cabcaf8 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2014-01-30  Cong Hou  <congh@google.com>
+
+       PR tree-optimization/60000
+       * tree-vect-loop.c (vect_transform_loop): Set pattern_def_seq to NULL
+       if the vectorized statement is a store. A store statement can only
+       appear at the end of pattern statements.
+
 2014-01-27  Jakub Jelinek  <jakub@redhat.com>

        PR bootstrap/59934
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index fa61d5c..f2ce70f 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-01-30  Cong Hou  <congh@google.com>
+
+       PR tree-optimization/60000
+       * g++.dg/vect/pr60000.cc: New test.
+
 2014-01-27  Christian Bruel  <christian.bruel@st.com>

        * gcc.target/sh/torture/strncmp.c: New tests.
diff --git a/gcc/testsuite/g++.dg/vect/pr60000.cc
b/gcc/testsuite/g++.dg/vect/pr60000.cc
new file mode 100644
index 0000000..fe39d6a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/vect/pr60000.cc
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fno-tree-vrp" } */
+
+void foo (bool* a, int* b)
+{
+  for (int i = 0; i < 1000; ++i)
+    {
+      a[i] = i % 2;
+      b[i] = i % 3;
+    }
+}
+
+/* { dg-final { cleanup-tree-dump "vect" } } */
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 69c8d21..0e162cb 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -6053,7 +6053,6 @@ vect_transform_loop (loop_vec_info loop_vinfo)
                     the chain.  */
                  gsi_next (&si);
                  vect_remove_stores (GROUP_FIRST_ELEMENT (stmt_info));
-                 continue;
                }
              else
                {
@@ -6063,11 +6062,13 @@ vect_transform_loop (loop_vec_info loop_vinfo)
                  unlink_stmt_vdef (store);
                  gsi_remove (&si, true);
                  release_defs (store);
-                 continue;
                }
-           }

-         if (!transform_pattern_stmt && gsi_end_p (pattern_def_si))
+             /* Stores can only appear at the end of pattern statements.  */
+             gcc_assert (!transform_pattern_stmt);
+             pattern_def_seq = NULL;
+           }
+         else if (!transform_pattern_stmt && gsi_end_p (pattern_def_si))
            {
              pattern_def_seq = NULL;
              gsi_next (&si);





>
>
>         Jakub