DSE calls to builtins (memset, etc)

[Richard Biener]

On Tue, Aug 19, 2014 at 4:16 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Tue, 19 Aug 2014, Richard Biener wrote:
>
>>>  /* Return true whether REF may refer to global memory.  */
>>>
>>>  bool
>>> -ref_may_alias_global_p (tree ref)
>>> +ref_may_alias_global_p (ao_ref *ref)
>>>  {
>>> -  tree base = get_base_address (ref);
>>> +  tree base = ao_ref_base (ref);
>>>    if (DECL_P (base))
>>>      return is_global_var (base);
>>>    else if (TREE_CODE (base) == MEM_REF
>>>            || TREE_CODE (base) == TARGET_MEM_REF)
>>>      return ptr_deref_may_alias_global_p (TREE_OPERAND (base, 0));
>>>    return true;
>>>  }
>>>
>>> +bool
>>> +ref_may_alias_global_p (tree ref)
>>> +{
>>> +  ao_ref r;
>>> +  ao_ref_init (&r, ref);
>>> +  return ref_may_alias_global_p (&r);
>>> +}
>>> +
>>
>>
>> I'd do two wrapers then since get_base_address is way cheaper
>> than doing ao_ref_base.  Thus, split out the code doing the
>> actual job into a static ref_may_alias_global_1?
>
>
> Ok, for this function the extra precision in ao_ref_base is indeed
> useless.
>
>
>>> +  /* We know we have virtual definitions.  We can handle assignments and
>>> +     some builtin calls.  */
>>> +  if (is_gimple_call (stmt))
>>
>>
>>  if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
>>
>> which also does argument verification
>
>
> Good point, then I can remove the test 'nargs < 2' below.
>
>
>>> +    {
>>> +      tree callee = gimple_call_fndecl (stmt);
>>> +      if (!callee || DECL_BUILT_IN_CLASS (callee) != BUILT_IN_NORMAL)
>>> +       return;
>>> +      switch (DECL_FUNCTION_CODE (callee))
>>> +       {
>>> +         case BUILT_IN_MEMCPY:
>>> +         case BUILT_IN_MEMMOVE:
>>> +         case BUILT_IN_MEMSET:
>>> +           {
>>> +             gimple use_stmt;
>>> +             ao_ref ref;
>>> +             tree size = NULL_TREE;
>>> +             int nargs = gimple_call_num_args (stmt);
>>> +             if (nargs < 2)
>>> +               return;
>>> +             if (nargs == 3)
>>> +               size = gimple_call_arg (stmt, 2);
>>> +             tree ptr = gimple_call_arg (stmt, 0);
>>> +             ao_ref_init_from_ptr_and_size (&ref, ptr, size);
>>> +             if (!dse_possible_dead_store_p (&ref, stmt, &use_stmt))
>>> +               return;
>>> +
>>> +             if (dump_file && (dump_flags & TDF_DETAILS))
>>> +               {
>>> +                 fprintf (dump_file, "  Deleted dead store '");
>>
>>
>> dead call
>>
>>> +                 print_gimple_stmt (dump_file, gsi_stmt (*gsi),
>>> dump_flags,
>>> 0);
>>> +                 fprintf (dump_file, "'\n");
>>> +               }
>>> +
>>> +             tree lhs = gimple_call_lhs (stmt);
>>> +             if (lhs)
>>> +               {
>>> +                 gimple new_stmt = gimple_build_assign (lhs, ptr);
>>> +                 unlink_stmt_vdef (stmt);
>>> +                 gsi_replace (gsi, new_stmt, true);
>>
>>
>> You may need eh cleanup here as well.
>
>
> Cleanup is one of the most mysterious parts of gcc, I am never sure what
> to release/update/unlink/purge/etc :-(
>
>
>> Bonus points if you make gsi_replace return whether
>> maybe_clean_or_replace_eh_stmt returned true.
>
>
> Ok, that seems trivial.
>
>
>>> -      basic_block bb;
>>> -
>>> -      /* 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;
>>> -
>>
>>
>> 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...)

Richard.

> Thanks,
>
> --
> Marc Glisse