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; }
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];
It is caused by revision 170984: http://gcc.gnu.org/ml/gcc-cvs/2011-03/msg00405.html
statement_sink_location in /* A killing definition is not a use. */ if (gimple_assign_single_p (use_stmt) && gimple_vdef (use_stmt) && operand_equal_p (gimple_assign_lhs (stmt), gimple_assign_lhs (use_stmt), 0)) continue; doesn't check: (gdb) call debug_gimple_stmt (use_stmt) # .MEM_11 = VDEF <.MEM_10> y.f0 = y.f0;
Mine.
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.
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);
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).
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
(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.
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
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
Fixed.