[Bug tree-optimization/50162] New: Wrong vectorization

[Bug tree-optimization/50162] New: Wrong vectorization

[ubizjak at gmail dot com]

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

             Bug #: 50162
           Summary: Wrong vectorization
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: ubizjak@gmail.com
            Target: x86


Following testcase results in wrong vectorization:

--cut here--
double a[256];
int b[256];
unsigned short c[256];

extern long lrint (double);

void bar(void)
{
  int i;

  for (i=0; i<256; ++i)
    {
      b[i] = lrint (a[i]);
      c[i] += c[i];
    }
}
--cut here--

gcc -O2 -ffast-math -ftree-vectorize -m32 -msse2 -mfpmath=sse

.L2:
    cvtpd2dq    a+16(,%eax,4), %xmm0
    cvtpd2dq    a(,%eax,4), %xmm1
    cvtpd2dq    a+32(,%eax,4), %xmm2
    punpcklqdq    %xmm0, %xmm1
    punpcklqdq    %xmm2, %xmm0
    movdqa    %xmm0, b+16(%eax,%eax)
    movdqa    c(%eax), %xmm0
    paddw    %xmm0, %xmm0
    movdqa    %xmm1, b(%eax,%eax)
    movdqa    %xmm0, c(%eax)
    addl    $16, %eax
    cmpl    $512, %eax
    jne    .L2

One cvtpd2dq is missing.

4.6.1. compiles to:

.L2:
    cvtpd2dq    a+16(,%eax,4), %xmm0
    cvtpd2dq    a(,%eax,4), %xmm1
    cvtpd2dq    a+48(,%eax,4), %xmm2
    punpcklqdq    %xmm0, %xmm1
    cvtpd2dq    a+32(,%eax,4), %xmm0
    punpcklqdq    %xmm2, %xmm0
    movdqa    %xmm0, b+16(%eax,%eax)
    movdqa    c(%eax), %xmm0
    paddw    %xmm0, %xmm0
    movdqa    %xmm1, b(%eax,%eax)
    movdqa    %xmm0, c(%eax)
    addl    $16, %eax
    cmpl    $512, %eax
    jne    .L2

The problem is already in .optimized tree dump:

  vect_var_.13_22 = MEM[symbol: a, index: ivtmp.41_17, step: 4, offset: 0B];
  vect_var_.14_24 = MEM[symbol: a, index: ivtmp.41_17, step: 4, offset: 16B];
  vect_var_.15_26 = MEM[symbol: a, index: ivtmp.41_17, step: 4, offset: 32B];
  vect_var_.17_29 = __builtin_ia32_vec_pack_sfix (vect_var_.13_22,
vect_var_.14_24);
  vect_var_.17_30 = __builtin_ia32_vec_pack_sfix (vect_var_.14_24,
vect_var_.15_26);
  MEM[symbol: b, index: ivtmp.41_17, step: 2, offset: 0B] = vect_var_.17_29;
  MEM[symbol: b, index: ivtmp.41_17, step: 2, offset: 16B] = vect_var_.17_30;
  vect_var_.26_38 = MEM[symbol: c, index: ivtmp.41_17, offset: 0B];
  vect_var_.27_39 = vect_var_.26_38 + vect_var_.26_38;
  MEM[symbol: c, index: ivtmp.41_17, offset: 0B] = vect_var_.27_39;

Please note, there is no access to offset 48B.

Gcc 4.6.1 generates:

  vect_var_.39 = __builtin_ia32_vec_pack_sfix (MEM[symbol: a, index: ivtmp.69,
step: 4], MEM[symbol: a, index: ivtmp.69, step: 4, offset: 16]);
  vect_var_.77 = __builtin_ia32_vec_pack_sfix (MEM[symbol: a, index: ivtmp.69,
step: 4, offset: 32], MEM[symbol: a, index: ivtmp.69, step: 4, offset: 48]);
  MEM[symbol: b, index: ivtmp.69, step: 2] = vect_var_.39;
  MEM[symbol: b, index: ivtmp.69, step: 2, offset: 16] = vect_var_.77;
  vect_var_.52 = MEM[symbol: c, index: ivtmp.69];
  MEM[symbol: c, index: ivtmp.69] = [plus_expr] vect_var_.52 + vect_var_.52;

[Bug tree-optimization/50162] [4.7 Regression] Wrong vectorization

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
   Target Milestone|---                         |4.7.0
            Summary|Wrong vectorization         |[4.7 Regression] Wrong
                   |                            |vectorization

--- Comment #1 from Uros Bizjak <ubizjak at gmail dot com> 2011-08-23 08:02:41 UTC ---
A regression from 4.6., and a very nasty one.

[Bug tree-optimization/50162] [4.7 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-08-23
     Ever Confirmed|0                           |1

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-23 08:40:43 UTC ---
Confirmed.

The vectorizer inserts the load:

  vect_var_.13_22 = MEM[(double[256] *)vect_pa.9_20];
  vect_pa.9_23 = vect_pa.9_20 + 16;
  vect_var_.14_24 = MEM[(double[256] *)vect_pa.9_23];
  vect_pa.9_25 = vect_pa.9_23 + 16;
  vect_var_.15_26 = MEM[(double[256] *)vect_pa.9_25];
  vect_pa.9_27 = vect_pa.9_25 + 16;
  vect_var_.16_28 = MEM[(double[256] *)vect_pa.9_27];

but nothing uses it so it gets DCEd quickly.

  vect_var_.17_29 = __builtin_ia32_vec_pack_sfix (vect_var_.13_22,
vect_var_.14_24);
  vect_var_.17_30 = __builtin_ia32_vec_pack_sfix (vect_var_.14_24,
vect_var_.15_26);

you can see _24 is used twice - I suppose instead _28 should appear here
somehow.

[Bug tree-optimization/50162] [4.7 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |irar at gcc dot gnu.org
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-23 08:52:26 UTC ---
Fix:

Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c       (revision 177983)
+++ gcc/tree-vect-stmts.c       (working copy)
@@ -1697,7 +1697,7 @@ vectorizable_call (gimple stmt, gimple_s
                }
              else
                {
-                 vec_oprnd1 = gimple_call_arg (new_stmt, 2*i);
+                 vec_oprnd1 = gimple_call_arg (new_stmt, 2*i + 1);
                  vec_oprnd0
                    = vect_get_vec_def_for_stmt_copy (dt[i], vec_oprnd1);
                  vec_oprnd1


Ira, does that make sense?  I suppose we replace foo (a, b, c) with
foo_vec_pack (a1, a2, b1, b2, c1, c2), etc.  And we want the second
argument from each pair from the last stmt?  (I wonder if all this even
works for nargs != 1).

[Bug tree-optimization/50162] [4.5/4.6/4.7 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.7 Regression] Wrong      |[4.5/4.6/4.7 Regression]
                   |vectorization               |Wrong vectorization
      Known to fail|                            |4.5.3, 4.6.1, 4.7.0

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-23 08:54:52 UTC ---
Btw, same code is on the 4.6 branch - I wonder why the bug doesn't happen
there?  It does.  And on the 4.5 branch.

[Bug tree-optimization/50162] [4.5/4.6/4.7 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.4.6

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-23 08:57:37 UTC ---
4.4 works though.  Which has

          for (i = 0; i < nargs; i++)
            {
              op = gimple_call_arg (stmt, i);
              if (j == 0)
                {
                  vec_oprnd0
                    = vect_get_vec_def_for_operand (op, stmt, NULL);
                  vec_oprnd1
                    = vect_get_vec_def_for_stmt_copy (dt[nargs], vec_oprnd0);
                }
              else
                {
                  vec_oprnd0
                    = vect_get_vec_def_for_stmt_copy (dt[nargs], vec_oprnd1);
                  vec_oprnd1
                    = vect_get_vec_def_for_stmt_copy (dt[nargs], vec_oprnd0);
                }

instead.  Which surely doesn't work for nargs != 1 (?)

[Bug tree-optimization/50162] [4.5/4.6 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.7.0
   Target Milestone|4.7.0                       |4.5.4
            Summary|[4.5/4.6/4.7 Regression]    |[4.5/4.6 Regression] Wrong
                   |Wrong vectorization         |vectorization
      Known to fail|4.7.0                       |

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-23 10:21:16 UTC ---
Fixed on trunk sofar.

[Bug tree-optimization/50162] [4.5/4.6/4.7 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-23 10:20:31 UTC ---
Author: rguenth
Date: Tue Aug 23 10:20:26 2011
New Revision: 177984

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=177984
Log:
2011-08-23  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/50162
    * tree-vect-stmts.c (vectorizable_call): Fix argument lookup.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-vect-stmts.c

[Bug tree-optimization/50162] [4.5/4.6 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-08 09:23:12 UTC ---
Author: rguenth
Date: Thu Sep  8 09:23:08 2011
New Revision: 178684

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178684
Log:
2011-09-08  Richard Guenther  <rguenther@suse.de>

    Backport from mainline
    2011-08-23  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/50162
    * tree-vect-stmts.c (vectorizable_call): Fix argument lookup.

Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/tree-vect-stmts.c

[Bug tree-optimization/50162] [4.5 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.6.2
            Summary|[4.5/4.6 Regression] Wrong  |[4.5 Regression] Wrong
                   |vectorization               |vectorization

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-08 09:23:49 UTC ---
And 4.6.2.

[Bug tree-optimization/50162] [4.5 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-03 13:19:35 UTC ---
Author: rguenth
Date: Tue Jan  3 13:19:28 2012
New Revision: 182843

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182843
Log:
2012-01-03  Richard Guenther  <rguenther@suse.de>

    Backport from mainline
    2011-08-23  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/50162
    * tree-vect-stmts.c (vectorizable_call): Fix argument lookup.

Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/tree-vect-stmts.c

[Bug tree-optimization/50162] [4.5 Regression] Wrong vectorization

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

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

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-03 13:19:56 UTC ---
Fixed.