[Bug tree-optimization/45902] New: CPU2006 benchmark sphinx3 fails with vectorization

[Bug tree-optimization/45902] New: CPU2006 benchmark sphinx3 fails with vectorization

[pthaugen at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

           Summary: CPU2006 benchmark sphinx3 fails with vectorization
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: pthaugen@gcc.gnu.org
                CC: irar@il.ibm.com
              Host: powerpc64-linux
            Target: powerpc64-linux
             Build: powerpc64-linux

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[pthaugen at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

Pat Haugen <pthaugen at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bergner at gcc dot gnu.org,
                   |                            |meissner at gcc dot gnu.org

--- Comment #1 from Pat Haugen <pthaugen at gcc dot gnu.org> 2010-10-05 21:24:01 UTC ---
Hmmm, apparently bug got filed before entering all the text.  Anyway, here goes
again...


482.sphinx3 miscompares when built with a recent mainline and the options -O3
-mcpu=power7. The error message seen is:

*** Miscompare of an4.log

Recompiling bencmark source file lm.c with -mno-vsx -mno-altivec or
-fno-tree-vectorize results in a successful execution.

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl.tools at gmail dot com

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> 2010-10-06 00:37:39 UTC ---
Please check if revision 164914 is OK.

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[pthaugen at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

--- Comment #3 from Pat Haugen <pthaugen at gcc dot gnu.org> 2010-10-06 02:29:35 UTC ---
Just checked current trunk, rev 165011, still fails.  Started failing a while
ago actually, have it narrowed down to 159910(ok) - 159980(fail). Unfortunately
159920-159970 result in GCC build errors for me.

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[irar at il dot ibm.com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

Ira Rosen <irar at il dot ibm.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2010.10.07 13:07:52
         AssignedTo|unassigned at gcc dot       |irar at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #4 from Ira Rosen <irar at il dot ibm.com> 2010-10-07 13:07:52 UTC ---
It is caused by combining different type conversions into one vector stmt
(lm.c:928/1147):

D.8017_68 = (s3lmwid_t) D.8016_67;
D.8025_86 = (uint16) D.8024_85;

The types are compatible, but not the same. In revision 157833 all the type
checks were changed from simple comparison to calls to types_compatible_p ().
The following patch adds an exact type comparison for LHS of type conversions:

Index: tree-vect-slp.c
===================================================================
--- tree-vect-slp.c     (revision 164987)
+++ tree-vect-slp.c     (working copy)
@@ -335,6 +335,7 @@ vect_build_slp_tree (loop_vec_info loop_
   bool permutation = false;
   unsigned int load_place;
   gimple first_load, prev_first_load = NULL;
+  tree first_stmt_lhs = NULL_TREE;

   /* For every stmt in NODE find its def stmt/s.  */
   FOR_EACH_VEC_ELT (gimple, stmts, i, stmt)
@@ -371,6 +372,9 @@ vect_build_slp_tree (loop_vec_info loop_
          return false;
        }

+      if (!first_stmt_lhs)
+        first_stmt_lhs = lhs;
+
       scalar_type = vect_get_smallest_scalar_type (stmt, &dummy, &dummy);
       vectype = get_vectype_for_scalar_type (scalar_type);
       if (!vectype)
@@ -473,6 +477,19 @@ vect_build_slp_tree (loop_vec_info loop_
              return false;
            }

+          if (CONVERT_EXPR_CODE_P (rhs_code)
+              && TREE_TYPE (lhs) != TREE_TYPE (first_stmt_lhs))
+            {
+              if (vect_print_dump_info (REPORT_SLP))
+                {
+                  fprintf (vect_dump, "Build SLP failed: different type "
+                           "conversion in stmt ");
+                  print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
+                }
+
+              return false;
+            }
+
          if (need_same_oprnds
              && !operand_equal_p (first_op1, gimple_assign_rhs2 (stmt), 0))
            {

I'll test the patch on Sunday.

Ira

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[pthaugen at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

Pat Haugen <pthaugen at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |UNCONFIRMED
     Ever Confirmed|1                           |0

--- Comment #5 from Pat Haugen <pthaugen at gcc dot gnu.org> 2010-10-07 17:56:38 UTC ---
Not sure it matters since you already have a patch, but it appears the fix for
PR 44284 introduced the problem. If I revert that patch sphinx3 passes.

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[pthaugen at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

Pat Haugen <pthaugen at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|2010-10-07 13:07:52         |2010.10.07 18:41:31
     Ever Confirmed|0                           |1

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-10-08 09:44:04 UTC ---
+          if (CONVERT_EXPR_CODE_P (rhs_code)
+              && TREE_TYPE (lhs) != TREE_TYPE (first_stmt_lhs))

checking types_compatible_p (TREE_TYPE (lhs), TREE_TYPE (first_stmt_lhs))
should work.  If for some reason it doesn't please use at least
TYPE_MAIN_VARIANT (TREE_TYPE (...

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[irar at il dot ibm.com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

--- Comment #7 from Ira Rosen <irar at il dot ibm.com> 2010-10-10 08:58:32 UTC ---
I think I made a mistake. It doesn't make sense. I'll take another look.

Thanks,
Ira

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[irar at il dot ibm.com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

--- Comment #8 from Ira Rosen <irar at il dot ibm.com> 2010-10-10 12:47:47 UTC ---
The problem is in creation of constant operands for SLP, and is caused by my
patch r155882, which takes the type for the constant from the constant itself.
This was needed for pointers, but is bad for other cases, like shift of shorts
that is treated as int here, causing wrong code generation.

I am going to test this patch now:

Index: tree-vect-slp.c
===================================================================
--- tree-vect-slp.c     (revision 164987)
+++ tree-vect-slp.c     (working copy)
@@ -1894,13 +1894,20 @@ vect_get_constant_vectors (slp_tree slp_
     }

   if (CONSTANT_CLASS_P (op))
-    constant_p = true;
+    {
+      constant_p = true;
+      if (POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt))))
+        vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
+      else
+        vector_type = STMT_VINFO_VECTYPE (stmt_vinfo);
+    }
   else
-    constant_p = false;
+    {
+      constant_p = false;
+      vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
+    }

-  vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
   gcc_assert (vector_type);
-
   nunits = TYPE_VECTOR_SUBPARTS (vector_type);

   /* NUMBER_OF_COPIES is the number of times we need to use the same values in

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-10-10 13:26:39 UTC ---
(In reply to comment #8)
> The problem is in creation of constant operands for SLP, and is caused by my
> patch r155882, which takes the type for the constant from the constant itself.
> This was needed for pointers, but is bad for other cases, like shift of shorts
> that is treated as int here, causing wrong code generation.
> 
> I am going to test this patch now:
> 
> Index: tree-vect-slp.c
> ===================================================================
> --- tree-vect-slp.c     (revision 164987)
> +++ tree-vect-slp.c     (working copy)
> @@ -1894,13 +1894,20 @@ vect_get_constant_vectors (slp_tree slp_
>      }
> 
>    if (CONSTANT_CLASS_P (op))
> -    constant_p = true;
> +    {
> +      constant_p = true;
> +      if (POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt))))
> +        vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
> +      else
> +        vector_type = STMT_VINFO_VECTYPE (stmt_vinfo);
> +    }
>    else
> -    constant_p = false;
> +    {
> +      constant_p = false;
> +      vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
> +    }
> 
> -  vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
>    gcc_assert (vector_type);
> -
>    nunits = TYPE_VECTOR_SUBPARTS (vector_type);
> 
>    /* NUMBER_OF_COPIES is the number of times we need to use the same values in

I certainly would prefer unconditional use of STMT_VINFO_VECTYPE (stmt_vinfo).
Why should that not work?

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[irar at il dot ibm.com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

--- Comment #10 from Ira Rosen <irar at il dot ibm.com> 2010-10-10 15:06:37 UTC ---
It doesn't work for pointers (pr42709):

+int
+main1 (int *a, int *b, int *c, int *d, int dummy)
+{
+  int i;
+
+  for (i = 0; i < N/2; i+=4)
+    {
+      res[i] = a + 16;
+      res[i+1] = b + 16;
+      res[i+2] = c + 16;
+      res[i+3] = d + 16;
+      if (dummy == 32)
+        abort ();
+    }
+}

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[irar at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

--- Comment #11 from irar at gcc dot gnu.org 2010-10-11 12:18:57 UTC ---
Author: irar
Date: Mon Oct 11 12:18:47 2010
New Revision: 165302

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=165302
Log:

    PR tree-optimization/45902
    * tree-vect-slp.c (vect_get_constant_vectors): Use statement's
    vector type for constants, unless it's a pointer.


Added:
    trunk/gcc/testsuite/gcc.dg/vect/pr45902.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vect-slp.c

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[irar at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

--- Comment #12 from irar at gcc dot gnu.org 2010-11-02 12:13:01 UTC ---
Author: irar
Date: Tue Nov  2 12:12:56 2010
New Revision: 166175

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166175
Log:

    PR tree-optimization/45902
    * tree-vectorizer.h (enum stmt_vec_info_type): Add new value for
    shift.
    (vect_get_slp_defs): Add arguments.
    * tree-vect-stmts.c (cost_for_stmt): Handle shifts.
    (vect_get_vec_def_for_operand): Use operand's
    type to determine number of units in the created vector.
    (vect_get_vec_defs): Pass scalar operands to vect_get_slp_defs.
    (vectorizable_shift): New function.
    (vectorizable_operation): Move code that handles shifts to
    vectorizable_shift.
    (vectorizable_type_demotion): Pass scalar operands to
    vect_get_slp_defs.
    (vectorizable_type_promotion, vectorizable_store): Likewise.
    (vect_analyze_stmt): Call vectorizable_shift.
    (vect_transform_stmt): Likewise.
    * tree-vect-slp.c (vect_get_constant_vectors): Add new argument.
    Use it as the operand to create vectors except for stores.
    (vect_get_slp_defs): Add new arguments. Pass them to
    vect_get_constant_vectors.


Added:
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/vect/pr45902.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/tree-vect-slp.c
    branches/gcc-4_5-branch/gcc/tree-vect-stmts.c
    branches/gcc-4_5-branch/gcc/tree-vectorizer.h

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[irar at il dot ibm.com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

Ira Rosen <irar at il dot ibm.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #13 from Ira Rosen <irar at il dot ibm.com> 2010-11-02 12:20:41 UTC ---
Fixed.

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[spop at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

--- Comment #14 from Sebastian Pop <spop at gcc dot gnu.org> 2010-11-09 19:09:01 UTC ---
Author: spop
Date: Tue Nov  9 19:08:57 2010
New Revision: 166506

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166506
Log:
Fix PR45314: backport revision 163123.

2010-11-09  Sebastian Pop  <sebastian.pop@amd.com>

    PR tree-optimization/45902
    Backport of revision 163132.
    * graphite-sese-to-poly.c (rewrite_phi_out_of_ssa): Always insert out
    of SSA copies on edges except for loop->latch.

Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/graphite-sese-to-poly.c

[Bug tree-optimization/45902] CPU2006 benchmark sphinx3 fails with vectorization

[spop at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45902

Sebastian Pop <spop at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |spop at gcc dot gnu.org

--- Comment #15 from Sebastian Pop <spop at gcc dot gnu.org> 2010-11-09 19:17:49 UTC ---
Sorry for the noise, the PR number was wrong for this commit.

(In reply to comment #14)
> Author: spop
> Date: Tue Nov  9 19:08:57 2010
> New Revision: 166506
> 
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166506
> Log:
> Fix PR45314: backport revision 163123.
> 
> 2010-11-09  Sebastian Pop  <sebastian.pop@amd.com>
> 
>     PR tree-optimization/45902
>     Backport of revision 163132.
>     * graphite-sese-to-poly.c (rewrite_phi_out_of_ssa): Always insert out
>     of SSA copies on edges except for loop->latch.
> 
> Modified:
>     branches/gcc-4_5-branch/gcc/ChangeLog
>     branches/gcc-4_5-branch/gcc/graphite-sese-to-poly.c