On Wed, Aug 20, 2014 at 4:18 PM, Marc Glisse <marc.gli...@inria.fr> wrote: > On Wed, 20 Aug 2014, Richard Biener wrote: > >>>>> - if (stmt != use_stmt >>>>> - && ref_maybe_used_by_stmt_p (use_stmt, gimple_assign_lhs >>>>> (stmt))) >>>>> - return; >>>>> - >>>> >>>> >>>> >>>> I don't see how you can remove this code? >>> >>> >>> >>> dse_possible_dead_store_p already tests ref_maybe_used_by_stmt_p and >>> thus cannot return true with such a use_stmt, as far as I can see. As I >>> said, bootstrap+testsuite with an assert instead of 'return' didn't turn >>> up anything. I could keep it as a gcc_checking_assert (with a slight >>> update to the comment) if you like. Or did I miss a path in >>> dse_possible_dead_store_p? >> >> >> Yes, the one that early returns from the operand_equal_p check. >> >> You might want to do some svn blaming to see what testcases >> were added with the above code (and the code surrounding it). >> >> I'm not sure either... so if it passes bootstrap & regtest it must be >> dead code... (well...) > > > The early return operand_equal_p has use_stmt == stmt, so it doesn't even > reach the call to ref_maybe_used_by_stmt_p I am removing. > > svn blame leads me to r132899 (gcc.c-torture/execute/pr35472.c) > and before that to r131101 (gcc.c-torture/execute/20071219-1.c)
Maybe add some noclone attributes as well? And try -fno-tree-fre/pre? But pr35472.c clearly shows what it was trying to fix: *p = a; *p = b; with p == &b. The *p = b; store does _not_ make *p = a dead because *p = b; is a no-op. So a modified testcase would be *p = a; *p = *p; where without your patch the first DSE pass removes *p = *p (but not first *p = a). Maybe if that still works after your patch you can add this as a new testcase, also verifying that *p = *p store indeed is removed. > Both testcases are still in the testsuite and passed. The rest of the code > has changed quite a bit since then, it isn't that surprising if some test > becomes redundant. But if it makes you nervous, we could keep it as a > checking_assert, the cost should be negligible... Nah, I wouldn't do a gcc_checking_assert here. Richard. > -- > Marc Glisse
