[Bug rtl-optimization/57303] New: struct miscompiled at -O1 and above

[Bug rtl-optimization/57303] New: struct miscompiled at -O1 and above

[dhazeghi at yahoo dot com]

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

            Bug ID: 57303
           Summary: struct miscompiled at -O1 and above
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dhazeghi at yahoo dot com

With current gcc trunk, the code below is miscompiled at -O1 and higher
optimization levels on x86_64-linux-gnu, outputting '1' instead of '0' as
expected.  This behavior appears in gcc 4.8.x and 4.7.x as well, but not in
4.6.x or earlier.  It occurs in both 32-bit and 64-bit compiles.

$ gcc-trunk -v
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc-trunk/configure
--enable-languages=c,c++,objc,obj-c++,fortran,lto --disable-checking
--with-gmp=/usr/local/gcc-trunk --with-mpfr=/usr/local/gcc-trunk
--with-mpc=/usr/local/gcc-trunk --with-cloog=/usr/local/gcc-trunk
--prefix=/usr/local/gcc-trunk
Thread model: posix
gcc version 4.9.0 20130516 (experimental) [trunk revision 198967] (GCC) 
$ gcc-trunk -O0 small.c
$ ./a.out 
0
$ gcc-4.6 -O1 small.c
$ ./a.out 
0
$ gcc-trunk -O1 small.c
$ ./a.out 
1
$
-----------------------------

int printf(const char *, ...);

struct S0
{
    int f0;
};
struct S1
{
    struct S0 f0;
};

struct S1 x = { {0} };
struct S1 y = { {1} };

static void
foo (struct S0 p)
{
    struct S0 *l = &y.f0;
    *l = x.f0;
    if (p.f0)
        *l = *l;
}

int
main ()
{
    foo(y.f0);
    printf("%d\n", y.f0.f0);
    return 0;
}

[Bug tree-optimization/57303] struct miscompiled at -O1 and above

[glisse at gcc dot gnu.org]

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

Marc Glisse <glisse at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-05-16
          Component|rtl-optimization            |tree-optimization
     Ever confirmed|0                           |1

--- Comment #1 from Marc Glisse <glisse at gcc dot gnu.org> ---
Seems to come from the "sink" pass. But it is suspicious that nothing in gcc
removes the following long before we get there:
  MEM[(struct S0 *)&y] = MEM[(struct S0 *)&y];

[Bug tree-optimization/57303] [4.7/4.8/4.9 Regression] struct miscompiled at -O1 and above

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
   Target Milestone|---                         |4.7.4

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine.

[Bug tree-optimization/57303] [4.7/4.8/4.9 Regression] struct miscompiled at -O1 and above

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
DSE has

          /* If use_stmt is or might be a nop assignment, e.g. for
             struct { ... } S a, b, *p; ...
             b = a; b = b;
             or
             b = a; b = *p; where p might be &b,
             or
             *p = a; *p = b; where p might be &b,
             or
             *p = *u; *p = *v; where p might be v, then USE_STMT
             acts as a use as well as definition, so store in STMT
             is not dead.  */
          if (stmt != use_stmt
              && ref_maybe_used_by_stmt_p (use_stmt, gimple_assign_lhs (stmt)))
            return;

for this very reason.

[Bug tree-optimization/57303] [4.7/4.8/4.9 Regression] struct miscompiled at -O1 and above

[glisse at gcc dot gnu.org]

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

--- Comment #6 from Marc Glisse <glisse at gcc dot gnu.org> ---
I wonder if, in addition to fixing the sink pass, we should add an optimization
like the following (it passes bootstrap+testsuite, but I am not so sure where
it should go and what it should look like exactly).

--- gimple-fold.c    (revision 199093)
+++ gimple-fold.c    (working copy)
@@ -1174,20 +1174,27 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
     if ((commutative_tree_code (subcode)
          || commutative_ternary_tree_code (subcode))
         && tree_swap_operands_p (gimple_assign_rhs1 (stmt),
                      gimple_assign_rhs2 (stmt), false))
       {
         tree tem = gimple_assign_rhs1 (stmt);
         gimple_assign_set_rhs1 (stmt, gimple_assign_rhs2 (stmt));
         gimple_assign_set_rhs2 (stmt, tem);
         changed = true;
       }
+    /* Remove *p = *p.  */
+    if (!inplace && TREE_CODE_CLASS (subcode) == tcc_reference
+        && operand_equal_p (lhs, gimple_assign_rhs1 (stmt), 0))
+      {
+        gsi_remove (gsi, true);
+        return true;
+      }
     new_rhs = fold_gimple_assign (gsi);
     if (new_rhs
         && !useless_type_conversion_p (TREE_TYPE (lhs),
                        TREE_TYPE (new_rhs)))
       new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs);
     if (new_rhs
         && (!inplace
         || get_gimple_rhs_num_ops (TREE_CODE (new_rhs)) < old_num_ops))
       {
         gimple_assign_set_rhs_from_tree (gsi, new_rhs);

[Bug tree-optimization/57303] [4.7/4.8/4.9 Regression] struct miscompiled at -O1 and above

[rguenther at suse dot de]

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

--- Comment #7 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 20 May 2013, glisse at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57303
> 
> --- Comment #6 from Marc Glisse <glisse at gcc dot gnu.org> ---
> I wonder if, in addition to fixing the sink pass, we should add an optimization
> like the following (it passes bootstrap+testsuite, but I am not so sure where
> it should go and what it should look like exactly).
> 
> --- gimple-fold.c    (revision 199093)
> +++ gimple-fold.c    (working copy)
> @@ -1174,20 +1174,27 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
>      if ((commutative_tree_code (subcode)
>           || commutative_ternary_tree_code (subcode))
>          && tree_swap_operands_p (gimple_assign_rhs1 (stmt),
>                       gimple_assign_rhs2 (stmt), false))
>        {
>          tree tem = gimple_assign_rhs1 (stmt);
>          gimple_assign_set_rhs1 (stmt, gimple_assign_rhs2 (stmt));
>          gimple_assign_set_rhs2 (stmt, tem);
>          changed = true;
>        }
> +    /* Remove *p = *p.  */
> +    if (!inplace && TREE_CODE_CLASS (subcode) == tcc_reference
> +        && operand_equal_p (lhs, gimple_assign_rhs1 (stmt), 0))
> +      {
> +        gsi_remove (gsi, true);
> +        return true;
> +      }

The obvious place would be dead store elimination.  But beware that
the above, if not _literally_ being *p = *p can be changing the
effective type of the memory location and thus might be not dead
in terms of type-based aliasing rules (which basically means that
we need to do sth more clever than just removing the store).

[Bug tree-optimization/57303] [4.7/4.8 Regression] struct miscompiled at -O1 and above

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.9.0
            Summary|[4.7/4.8/4.9 Regression]    |[4.7/4.8 Regression] struct
                   |struct miscompiled at -O1   |miscompiled at -O1 and
                   |and above                   |above

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Tue May 21 08:11:23 2013
New Revision: 199135

URL: http://gcc.gnu.org/viewcvs?rev=199135&root=gcc&view=rev
Log:
2013-05-21  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/57303
    * tree-ssa-sink.c (statement_sink_location): Improve killing
    stmt detection and properly handle self-assignments.

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

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr57303.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-sink.c

[Bug tree-optimization/57303] [4.7/4.8 Regression] struct miscompiled at -O1 and above

[glisse at gcc dot gnu.org]

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

--- Comment #9 from Marc Glisse <glisse at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #7)
> On Mon, 20 May 2013, glisse at gcc dot gnu.org wrote:
> > +    /* Remove *p = *p.  */
> > +    if (!inplace && TREE_CODE_CLASS (subcode) == tcc_reference
> > +        && operand_equal_p (lhs, gimple_assign_rhs1 (stmt), 0))
> > +      {
> > +        gsi_remove (gsi, true);
> > +        return true;
> > +      }
> 
> The obvious place would be dead store elimination.  But beware that
> the above, if not _literally_ being *p = *p can be changing the
> effective type of the memory location and thus might be not dead
> in terms of type-based aliasing rules (which basically means that
> we need to do sth more clever than just removing the store).

So operand_equal_p on a tcc_reference is not enough? Aliasing is complicated, I
guess I'll open a PR about this optimization.

[Bug tree-optimization/57303] [4.7 Regression] struct miscompiled at -O1 and above

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.8.1
            Summary|[4.7/4.8 Regression] struct |[4.7 Regression] struct
                   |miscompiled at -O1 and      |miscompiled at -O1 and
                   |above                       |above
      Known to fail|                            |4.7.3, 4.8.0

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Wed May 22 07:50:40 2013
New Revision: 199179

URL: http://gcc.gnu.org/viewcvs?rev=199179&root=gcc&view=rev
Log:
2013-05-22  Richard Biener  <rguenther@suse.de>

    Backport from mainline
    2013-05-21  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/57318
    * tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Do not
    estimate stmts with side-effects as likely eliminated.

    2013-05-21  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/57330
    * cgraph.c (cgraph_redirect_edge_call_stmt_to_callee): Properly
    preserve the call stmts fntype.

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

    2013-05-21  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/57303
    * tree-ssa-sink.c (statement_sink_location): Properly handle
    self-assignments.

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

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/torture/pr57303.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/torture/pr57330.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/cgraph.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/tree-ssa-loop-ivcanon.c
    branches/gcc-4_8-branch/gcc/tree-ssa-sink.c

[Bug tree-optimization/57303] [4.7 Regression] struct miscompiled at -O1 and above

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Mon Mar 17 14:38:55 2014
New Revision: 208618

URL: http://gcc.gnu.org/viewcvs?rev=208618&root=gcc&view=rev
Log:
2014-03-17  Richard Biener  <rguenther@suse.de>

    Backport from mainline
    2013-05-21  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/57303
    * tree-ssa-sink.c (statement_sink_location): Properly handle
    self-assignments.

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

    2013-12-02  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/59139
    * tree-ssa-loop-niter.c (chain_of_csts_start): Properly match
    code in get_val_for.
    (get_val_for): Use gcc_checking_asserts.

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

    2014-02-14  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/60183
    * tree-ssa-phiprop.c (propagate_with_phi): Avoid speculating
    loads.
    (tree_ssa_phiprop): Calculate and free post-dominators.

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

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr57303.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr59139.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr60183.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-ssa-loop-niter.c
    branches/gcc-4_7-branch/gcc/tree-ssa-phiprop.c
    branches/gcc-4_7-branch/gcc/tree-ssa-sink.c

[Bug tree-optimization/57303] [4.7 Regression] struct miscompiled at -O1 and above

[rguenth at gcc dot gnu.org]

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

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

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

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