This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: DSE calls to builtins (memset, etc)


On Wed, 20 Aug 2014, Richard Biener wrote:

On Wed, Aug 20, 2014 at 4:18 PM, Marc Glisse <marc.glisse@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?

Ok, both testcases now have noinline,noclone instead of just noinline in my tree.

	* gcc.c-torture/execute/pr35472.c: Add noclone attribute.
	* gcc.c-torture/execute/20071219-1.c: Likewise.

And try -fno-tree-fre/pre?

No problem.
(it makes it harder to find a killing stmt)

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.

DSE goes backwards, so it first removes *p=*p (already tested by gcc.dg/tree-ssa/pr57361.c) and when we look at *p=*a; there is no second statement left, so it wouldn't really test what you want.

--
Marc Glisse


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]