DSE calls to builtins (memset, etc)

[Richard Biener]

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?  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