[Bug tree-optimization/60656] New: [4.8/4.9 regression] x86 vectorization produces wrong code

[Bug tree-optimization/60656] New: [4.8/4.9 regression] x86 vectorization produces wrong code

[ppluzhnikov at google dot com]

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

            Bug ID: 60656
           Summary: [4.8/4.9 regression] x86 vectorization produces wrong
                    code
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ppluzhnikov at google dot com

Current trunk (r208813) and gcc-4.8 are affected, 4.7 does not appear to be.

gcc -O0 t.c && ./a.out
500450210036

gcc -O3 t.c && ./a.out
500450200033

gcc -O2 t.c && ./a.out
500450210036

gcc -O2 t.c -ftree-vectorize && ./a.out
500450200033

// t.c
#include <stdio.h>

int
main ()
{
  int v[] = {5000, 5001, 5002, 5003};
  long s = 0;
  int i;

  for(i = 0; i < 4; ++i)
    {
      long P = v[i];
      s += P*P*P;
    }
  printf("%ld\n", s);
}

[Bug tree-optimization/60656] [4.8/4.9 regression] x86 vectorization produces wrong code

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-03-25
                 CC|                            |jakub at gcc dot gnu.org
   Target Milestone|---                         |4.8.3
     Ever confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r189006.  Looking into it.

[Bug tree-optimization/60656] [4.8/4.9 regression] x86 vectorization produces wrong code

[congh at google dot com]

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

Cong Hou <congh at google dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |congh at google dot com

--- Comment #2 from Cong Hou <congh at google dot com> ---
This bug is caused by an optimization in GCC vectorizer that is not
implemented properly. When a reduction operation is vectorized, the
order of elements in vectors directly used in reduction does not
matter. In some cases the vectorizer may generate less code based on
this fact. GCC assigns a property named "vect_used_by_reduction" to
all vectors participating in reductions. However, vectors that are
indirectly used in reduction also have this property. For example,
consider the following three statements (all operands are vectors):

a = b op1 c;
d = a op2 e;
s1 = s0 op3 d;

Here assume the last statement is a reduction one, then a,b,c,d,e all
have the property "vect_used_by_reduction". However, if op2 is
different from op3, then a's element order can affect the final
result. GCC does not check this.

[Bug tree-optimization/60656] [4.8/4.9 regression] x86 vectorization produces wrong code

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
You are right, before r189408 this was using ordered_p = false, now it is
slightly different.
In any case, perhaps we could just check if the imm use of the stmt is a
reduction PHI and only assume unordered in that case?

[Bug tree-optimization/60656] [4.8/4.9 regression] x86 vectorization produces wrong code

[congh at google dot com]

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

--- Comment #4 from Cong Hou <congh at google dot com> ---
Yes, there is a quick fix: we can check if the def with vect_used_by_reduction
is immediately used by a reduction stmt. After all, it seems that
supportable_widening_operation() is the only place that takes advantage of this
"the element order doesn't matter" feature.


diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 70fb411..7442d0c 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -7827,7 +7827,16 @@ supportable_widening_operation (enum tree_code code,
gimple stmt,
                                             stmt, vectype_out, vectype_in,
                                             code1, code2, multi_step_cvt,
                                             interm_types))
-       return true;
+        {
+          tree lhs = gimple_assign_lhs (stmt);
+          use_operand_p dummy;
+          gimple use_stmt;
+          stmt_vec_info use_stmt_info = NULL;
+          if (single_imm_use (lhs, &dummy, &use_stmt)
+              && (use_stmt_info = vinfo_for_stmt (use_stmt))
+              && STMT_VINFO_DEF_TYPE (use_stmt_info) == vect_reduction_def)
+            return true;
+        }
       c1 = VEC_WIDEN_MULT_LO_EXPR;
       c2 = VEC_WIDEN_MULT_HI_EXPR;
       break;

[Bug tree-optimization/60656] [4.8/4.9 regression] x86 vectorization produces wrong code

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Cong Hou from comment #4)
> Yes, there is a quick fix: we can check if the def with
> vect_used_by_reduction is immediately used by a reduction stmt. After all,
> it seems that supportable_widening_operation() is the only place that takes
> advantage of this "the element order doesn't matter" feature.
> 
> 
> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index 70fb411..7442d0c 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -7827,7 +7827,16 @@ supportable_widening_operation (enum tree_code code,
> gimple stmt,
>                                              stmt, vectype_out, vectype_in,
>                                              code1, code2, multi_step_cvt,
>                                              interm_types))
> -       return true;
> +        {
> +          tree lhs = gimple_assign_lhs (stmt);
> +          use_operand_p dummy;
> +          gimple use_stmt;
> +          stmt_vec_info use_stmt_info = NULL;
> +          if (single_imm_use (lhs, &dummy, &use_stmt)
> +              && (use_stmt_info = vinfo_for_stmt (use_stmt))
> +              && STMT_VINFO_DEF_TYPE (use_stmt_info) == vect_reduction_def)
> +            return true;
> +        }
>        c1 = VEC_WIDEN_MULT_LO_EXPR;
>        c2 = VEC_WIDEN_MULT_HI_EXPR;
>        break;

Looks good to me, perhaps just no need to initialize use_stmt_info to NULL.
Are you going to bootstrap/regtest this and post to gcc-patches?

[Bug tree-optimization/60656] [4.8/4.9 regression] x86 vectorization produces wrong code

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 32475
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32475&action=edit
gcc49-pr60656.patch

I've bootstrapped/regtested in the mean time this patch on x86_64-linux and
i686-linux, no regression.  As it is your patch, can you please post it?

[Bug tree-optimization/60656] [4.8/4.9 regression] x86 vectorization produces wrong code

[congh at google dot com]

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

--- Comment #7 from Cong Hou <congh at google dot com> ---
Yes, will do it. Thank you a lot!

[Bug tree-optimization/60656] [4.8/4.9 regression] x86 vectorization produces wrong code

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/60656] [4.8 regression] x86 vectorization produces wrong code

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---
            Summary|[4.8/4.9 regression] x86    |[4.8 regression] x86
                   |vectorization produces      |vectorization produces
                   |wrong code                  |wrong code

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Actually, still not fixed on the 4.8 branch, only on the trunk.

[Bug tree-optimization/60656] [4.8 regression] x86 vectorization produces wrong code

[bernd.edlinger at hotmail dot de]

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

Bernd Edlinger <bernd.edlinger at hotmail dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bernd.edlinger at hotmail dot de

--- Comment #11 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Hmm,

with gcc-4.9.0-RC-20140411 on arm-linux-gnueabihf I see the following:

ERROR: gcc.dg/vect/pr60656.c: error executing dg-final: can't read
"et_vect_widen_mult_si_to_di_pattern_saved": no such variable
UNRESOLVED: gcc.dg/vect/pr60656.c: error executing dg-final: can't read
"et_vect_widen_mult_si_to_di_pattern_saved": no such variable

what does that mean?

[Bug tree-optimization/60656] [4.8 regression] x86 vectorization produces wrong code

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed already with r209363.

[Bug tree-optimization/60656] [4.8 regression] x86 vectorization produces wrong code

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60656

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.3                       |4.8.4

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 4.8.3 is being released, adjusting target milestone.

[Bug tree-optimization/60656] [4.8 regression] x86 vectorization produces wrong code

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60656

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.4                       |4.8.5

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.4 has been released.

[Bug tree-optimization/60656] [4.8 regression] x86 vectorization produces wrong code

[bernhard.kaindl at thalesgroup dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60656

Bernhard Kaindl <bernhard.kaindl at thalesgroup dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bernhard.kaindl@thalesgroup
                   |                            |.com

--- Comment #15 from Bernhard Kaindl <bernhard.kaindl at thalesgroup dot com> ---
Confirmed:

*     Fixed in: gcc-4.9.2 (release)
* NOT Fixed in: gcc-4.8.4 (release)

It seems indeed only i386, x32 and x86_64 (-m32, -mx32 and -m64) are affected.

-fopt-info-vec-note show that e.g. powerpc & powerpc64 don't vectorize the
loop.

The mainline fix from Comment #8 apples without fuzz and fixes this wrong code
issue in gcc-4.8.4.

Apply it for 4.8.5?

[Bug tree-optimization/60656] [4.8 regression] x86 vectorization produces wrong code

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60656

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
I'll see if it is backportable easily.

[Bug tree-optimization/60656] [4.8 regression] x86 vectorization produces wrong code

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60656

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |corydoras at ridiculousfish dot co
                   |                            |m

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
*** Bug 61108 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/60656] [4.8 regression] x86 vectorization produces wrong code

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60656

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

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

--- Comment #19 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.