[Bug c/52406] New: likely wrong code bug

[Bug c/52406] New: likely wrong code bug

[regehr at cs dot utah.edu]

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

             Bug #: 52406
           Summary: likely wrong code bug
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: regehr@cs.utah.edu


regehr@john-home:~/z/reduce/1$ current-gcc -O2 small.c ; ./a.out 
1
regehr@john-home:~/z/reduce/1$ current-gcc -O3 small.c ; ./a.out 
0
regehr@john-home:~/z/reduce/1$ current-gcc -v
Using built-in specs.
COLLECT_GCC=current-gcc
COLLECT_LTO_WRAPPER=/home/regehr/z/compiler-install/gcc-r184581-install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../configure --enable-lto
--prefix=/home/regehr/z/compiler-install/gcc-r184581-install
--program-prefix=r184581- --enable-languages=c,c++
Thread model: posix
gcc version 4.7.0 20120226 (experimental) (GCC) 
regehr@john-home:~/z/reduce/1$ cat small.c
int printf (const char *, ...);

struct
{
  int f1;
} a[2];

int *b, *const k = &a[1].f1;

static int **c = &b;

int e, f, d;

int main ()
{
  int **l = &b;
  *l = k;
  for ( ; d <= 0; d++ )
    {
      int *j = &e;
      **c = 1;
      *l = k;
      *k ^= 0;
      f = **l;
      *j = f;
    }
  printf ( "%d\n", e );
  return 0;
}

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-02-27
                 CC|                            |jakub at gcc dot gnu.org
          Component|c                           |tree-optimization
   Target Milestone|---                         |4.7.0
            Summary|likely wrong code bug       |[4.7 Regression] likely
                   |                            |wrong code bug
     Ever Confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-27 22:27:43 UTC ---
Started with http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178028

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-28 09:54:01 UTC ---
Strangely,
/* PR tree-optimization/52406 */

extern void abort (void);
struct { int f1; } a[2];

int *b, *const k = &a[1].f1;
static int **c = &b;

int e, f, d;

int
main ()
{
  int **l = &b;
  *l = k;
  for (; d <= 0; d++)
    {
      int *j = &e;
      **c = 1;
      *l = k;
      *k ^= 0;
      f = **l;
      *j = f;
    }
  if (e != 1)
    abort ();
  return 0;
}

fails, but with
--- pr52406.c 2012-02-28 10:47:45.663204390 +0100
+++ pr52406.c 2012-02-28 10:47:56.695143490 +0100
@@ -1,9 +1,9 @@
 /* PR tree-optimization/52406 */

 extern void abort (void);
-struct { int f1; } a[2];
+int a[2];

-int *b, *const k = &a[1].f1;
+int *b, *const k = &a[1];
 static int **c = &b;

 int e, f, d;

it works (IL starts to differ during pcom), beyond the &a[1] vs. &a[1].f1
changes.  So even if there isn't a wrong code, there would be at least
missed-optimization.

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-28 10:14:57 UTC ---
Indeed, it is pcom that breaks it.

Before pcom we have:
  MEM[(int *)&a + 4B] = 1;
  D.1723_6 = a[1].f1;
but pcom doesn't consider the first store to be possibly affecting the load:
  a_I_f1_lsm0.14_26 = a[1].f1;

<bb 5>:
  # d_lsm0.12_12 = PHI <d_lsm0.12_18(6), d.13_5(4)>
  MEM[(int *)&a + 4B] = 1;
  D.1723_6 = a_I_f1_lsm0.14_26;

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-28 10:42:47 UTC ---
Mine.

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-28 11:06:16 UTC ---
We have

Creating dr for MEM[(int *)&a + 4B]
        base_address: &a
        offset from base address: 0
        constant offset from base address: 4
        step: 0
        aligned to: 128
        base_object: MEM[(int *)&a + 4B]
Creating dr for a[1].f1
        base_address: &a
        offset from base address: 0
        constant offset from base address: 4
        step: 0
        aligned to: 128
        base_object: a[0].f1
        Access function 0: 1

thus we have an access function for a[1].f1 but no access function for
MEM[(int *)&a + 4B].  That should cause it to conflict.

(compute_affine_dependence
  (stmt_a =
MEM[(int *)&a + 4B] = 1;
)
  (stmt_b =
D.1724_6 = a[1].f1;
)
(Data Dep:
#(Data Ref:
#  bb: 5
#  stmt: MEM[(int *)&a + 4B] = 1;
#  ref: MEM[(int *)&a + 4B];
#  base_object: MEM[(int *)&a + 4B];
#)
#(Data Ref:
#  bb: 5
#  stmt: D.1724_6 = a[1].f1;
#  ref: a[1].f1;
#  base_object: a[0].f1;
#  Access function 0: 1
#)
    (no dependence)
)

hmpf.  dr_may_alias_p returns false because it feeds the oracle with
the base objects!

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |50067

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-28 14:32:58 UTC ---
The issue is that we want to disambiguate a[i].f1 and a[i].f2, so for the
"base object" we zero out all known indices, resulting in a[0].f1 and a[0].f2
which we then disambiguate (and conclude that for all i there cannot be
a dependence).

Now, when we mix pointer accesses with array accesses most of the index
analysis falls apart (which is what the fix for PR50067 tries to make
work more reliably - see its comment #13 on the dr_may_alias_p issue ...)

So, it's really wrong to try to fixup DR_BASE_OBJECT to make dr_may_alias_p
work, and it is equally wrong to use DR_BASE_OBJECT in dr_may_alias_p.
Using DR_REF (a safe bet) falls foul of failing a load of testcases, for
example gcc.dg/vect/pr37027.c which is no longer vectorized because

(compute_affine_dependence
  stmt_a: D.1722_7 = a[i_24].f1;
  stmt_b: D.1725_11 = a[i_24].f2;
) -> dependence analysis failed

previously we'd have used a[0].f1 and a[0].f2 in the disambiguation in
dr_may_alias_p and disambiguated the accesses.

We can try a similar trick as with REALPART/IMAGPART_EXPR to recover this.
Add a constant access function for outer COMPONENT_REFs (those we can strip
off the base object).

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.0                       |4.7.1

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-01 11:42:06 UTC ---
Defered to 4.7.1.

[Bug tree-optimization/52406] [4.7/4.8 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-02 14:59:01 UTC ---
Author: rguenth
Date: Fri Mar  2 14:58:55 2012
New Revision: 184789

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

    PR tree-optimization/52406
    * tree-data-ref.h: Update documentation about DR_BASE_OBJECT.
    (struct indices): Add unconstrained_base member.
    (struct dr_alias): Remove unused vops member.
    (DR_UNCONSTRAINED_BASE): New define.
    * tree-data-ref.c (dr_analyze_indices): For COMPONENT_REFs
    add indices to allow their disambiguation.  Make DR_BASE_OBJECT
    be an artificial access that covers the whole indexed object,
    or mark it with DR_UNCONSTRAINED_BASE if we cannot do so.  Canonicalize
    plain decl base-objects to their MEM_REF variant.
    (dr_may_alias_p): When the base-object of either data reference
    has unknown size use only points-to information.
    (compute_affine_dependence): Make dumps easier to read and
    more verbose.
    * tree-vect-data-ref.c (vector_alignment_reachable_p): Use
    DR_REF when looking for packed references.
    (vect_supportable_dr_alignment): Likewise.

    * gcc.dg/torture/pr52406.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr52406.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-data-ref.c
    trunk/gcc/tree-data-ref.h
    trunk/gcc/tree-vect-data-refs.c

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.8.0
            Summary|[4.7/4.8 Regression] likely |[4.7 Regression] likely
                   |wrong code bug              |wrong code bug

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-02 15:04:56 UTC ---
Fixed on trunk sofar.

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[izamyatin at gmail dot com]

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

Igor Zamyatin <izamyatin at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |izamyatin at gmail dot com

--- Comment #10 from Igor Zamyatin <izamyatin at gmail dot com> 2012-03-14 08:47:54 UTC ---
This caused http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52580

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-03 14:30:38 UTC ---
Author: rguenth
Date: Tue Apr  3 14:30:32 2012
New Revision: 186108

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

    Backport from mainline
    2012-03-02  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/52406
    * tree-data-ref.h: Update documentation about DR_BASE_OBJECT.
    (struct indices): Add unconstrained_base member.
    (struct dr_alias): Remove unused vops member.
    (DR_UNCONSTRAINED_BASE): New define.
    * tree-data-ref.c (dr_analyze_indices): For COMPONENT_REFs
    add indices to allow their disambiguation.  Make DR_BASE_OBJECT
    be an artificial access that covers the whole indexed object,
    or mark it with DR_UNCONSTRAINED_BASE if we cannot do so.  Canonicalize
    plain decl base-objects to their MEM_REF variant.
    (dr_may_alias_p): When the base-object of either data reference
    has unknown size use only points-to information.
    (compute_affine_dependence): Make dumps easier to read and
    more verbose.
    * tree-vect-data-ref.c (vector_alignment_reachable_p): Use
    DR_REF when looking for packed references.
    (vect_supportable_dr_alignment): Likewise.

    * gcc.dg/torture/pr52406.c: New testcase.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr52406.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-data-ref.c
    branches/gcc-4_7-branch/gcc/tree-data-ref.h
    branches/gcc-4_7-branch/gcc/tree-vect-data-refs.c

[Bug tree-optimization/52406] [4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |4.7.1
         Resolution|                            |FIXED
      Known to fail|                            |4.7.0

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-03 14:31:04 UTC ---
Fixed.