[Bug tree-optimization/49771] New: [4.7 Regression] wrong code with -ftree-vectorize

[Bug tree-optimization/49771] New: [4.7 Regression] wrong code with -ftree-vectorize

[zsojka at seznam dot cz]

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

           Summary: [4.7 Regression] wrong code with -ftree-vectorize
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: zsojka@seznam.cz
              Host: x86_64-pc-linux-gnu
            Target: x86_64-pc-linux-gnu


Created attachment 24783
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24783
reduced testcase (from gcc.dg/graphite/block-0.c)

Output:
$ gcc -O -ftree-vectorize testcase.c
$ ./a.out 
Aborted
$ gcc -O3 testcase.c
$ ./a.out 
Aborted

Tested revisions:
r176370 - fail
4.6 r176280 - OK

[Bug tree-optimization/49771] [4.7 Regression] wrong code with -ftree-vectorize

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.07.17 22:26:59
                 CC|                            |jakub at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-07-17 22:26:59 UTC ---
Caused by http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176229

[Bug tree-optimization/49771] [4.7 Regression] wrong code with -ftree-vectorize

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org
   Target Milestone|---                         |4.7.0

[Bug tree-optimization/49771] [4.7 Regression] wrong code with -ftree-vectorize

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-07-18 14:12:49 UTC ---
That commit looks bogus, the change it made is:
   # ivtmp.37_45 = PHI <ivtmp.37_43(6), ivtmp.37_46(2)>
   vect_pa.7_44 = (vector(4) int *) ivtmp.37_45;
-  D.2731_1 = vect_pa.7_44 < &a;
-  D.2733_14 = vect_pa.7_44 > &a[1000];
+  D.2731_1 = vect_pa.7_44 <= &a;
+  D.2733_14 = vect_pa.7_44 >= &a[1000];
   D.2734_23 = D.2733_14 | D.2731_1;
   if (D.2734_23 != 0)

vect_pa.7_44 is the pointer that goes from &a[0] to &a[999].  While in this
case it is fine to change the second > to >=, changing the first one is of
course wrong, because in the first iteration it will overlap, yet this test
will say that they do not.
What is weird is that segment_length_a in vect_create_cond_for_alias_checks
is 0 instead of 4 (sizeof (int)) I'd expect, while segment_length_b is the
expected 4000.  Apparently DR_STEP in the first case is 0.  I wonder whether
the change of the comparison should only be done if one segment length is empty
(and thus clearly doesn't include all bytes of the read/store), or something
similar.

[Bug tree-optimization/49771] [4.7 Regression] wrong code with -ftree-vectorize

[irar at il dot ibm.com]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |irar at il dot ibm.com

--- Comment #3 from Ira Rosen <irar at il dot ibm.com> 2011-07-18 15:55:49 UTC ---
(In reply to comment #2)
> That commit looks bogus, the change it made is:
>    # ivtmp.37_45 = PHI <ivtmp.37_43(6), ivtmp.37_46(2)>
>    vect_pa.7_44 = (vector(4) int *) ivtmp.37_45;
> -  D.2731_1 = vect_pa.7_44 < &a;
> -  D.2733_14 = vect_pa.7_44 > &a[1000];
> +  D.2731_1 = vect_pa.7_44 <= &a;
> +  D.2733_14 = vect_pa.7_44 >= &a[1000];
>    D.2734_23 = D.2733_14 | D.2731_1;
>    if (D.2734_23 != 0)
> 
> vect_pa.7_44 is the pointer that goes from &a[0] to &a[999].  While in this
> case it is fine to change the second > to >=, changing the first one is of
> course wrong, because in the first iteration it will overlap, yet this test
> will say that they do not.
> What is weird is that segment_length_a in vect_create_cond_for_alias_checks
> is 0 instead of 4 (sizeof (int)) I'd expect, while segment_length_b is the
> expected 4000.  Apparently DR_STEP in the first case is 0. 

The step of a[i] is indeed 0 in the j loop.

> I wonder whether
> the change of the comparison should only be done if one segment length is empty
> (and thus clearly doesn't include all bytes of the read/store), or something
> similar.

We can create a segment of length sizeof(type) for the zero step case:

Index: tree-vect-loop-manip.c
===================================================================
--- tree-vect-loop-manip.c      (revision 176389)
+++ tree-vect-loop-manip.c      (working copy)
@@ -2356,9 +2356,14 @@ static tree
 vect_vfa_segment_size (struct data_reference *dr, tree length_factor)
 {
   tree segment_length;
-  segment_length = size_binop (MULT_EXPR,
-                              fold_convert (sizetype, DR_STEP (dr)),
-                              fold_convert (sizetype, length_factor));
+
+  if (!compare_tree_int (DR_STEP (dr), 0))
+    segment_length = TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (dr)));
+  else
+    segment_length = size_binop (MULT_EXPR,
+                                fold_convert (sizetype, DR_STEP (dr)),
+                                fold_convert (sizetype, length_factor));
+
   if (vect_supportable_dr_alignment (dr, false)
         == dr_explicit_realign_optimized)
     {


(tested only with the vectorizer testsuite).

Ira

[Bug tree-optimization/49771] [4.7 Regression] wrong code with -ftree-vectorize

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-07-18 18:45:56 UTC ---
Yeah, that indeed fixes this testcase, and in addition keeps
static int a[2000];

int
foo (void)
{
  int j;
  int i;
  for (i = 0; i < 1000; i++)
    for (j = 0; j < 1000; j++)
      a[j] = a[1000 + i] + 1;
  return a[0];
}

doing the non-aliased loop (which I assume was the original intent of Richard's
patch).
It would be nice if GCC could figure this out at compile time in this case
though, it should know that on the #c0 testcase one of the accesses is 4 bytes
from &a[0] to &a[999] and the other can already be seen to be &a[0] through
&a[1000], so should be able to say that they always do alias.  In the testcase
from this comment that they never alias.

[Bug tree-optimization/49771] [4.7 Regression] wrong code with -ftree-vectorize

[irar at il dot ibm.com]

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

--- Comment #5 from Ira Rosen <irar at il dot ibm.com> 2011-07-18 20:01:24 UTC ---
Right, the dependence analysis fails here:

(compute_affine_dependence
  (stmt_a =
D.2713_5 = a[i_20];
)
  (stmt_b =
a[j_19] = D.2714_6;
)
(subscript_dependence_tester
(analyze_overlapping_iterations
  (chrec_a = i_20)
  (chrec_b = {0, +, 1}_2)
  (overlap_iterations_a = not known
)
  (overlap_iterations_b = not known
)
)
(dependence classified: scev_not_known)
)
)

I think it would be better to open a separate missed-optimization pr for that.

Meanwhile, I tested the patch on powerpc64-suse-linux. I'll commit it tomorrow
if there are no objections.

Ira

[Bug tree-optimization/49771] [4.7 Regression] wrong code with -ftree-vectorize

[irar at gcc dot gnu.org]

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

--- Comment #6 from irar at gcc dot gnu.org 2011-07-19 06:25:11 UTC ---
Author: irar
Date: Tue Jul 19 06:25:07 2011
New Revision: 176434

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

        PR tree-optimization/49771
        * tree-vect-loop-manip.c (vect_vfa_segment_size): In case of
        zero step, set segment length to the size of the data-ref's type.


Added:
    trunk/gcc/testsuite/gcc.dg/vect/pr49771.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vect-loop-manip.c

[Bug tree-optimization/49771] [4.7 Regression] wrong code with -ftree-vectorize

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-07-19 07:20:13 UTC ---
Fixed, the dependency analysis issue tracked now in PR49782.