Re: [RFC] warn on dead function calls in ipa-pure-const [1/4]

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

On 31 July 2016 at 22:01, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Tue, 26 Jul 2016, Prathamesh Kulkarni wrote:
>>
>> > +   warning_at (gimple_location (g), OPT_Wunused_value,
>> > +               "Call from %s to %s has no effect",
>> > +               e->caller->name (), e->callee->name ());
>>
>> Diagnostics should not start with capital letters.  Function names in
>> diagnostics should be quoted, so %qs.  Also, what form is this name in?
>> If it's the internal UTF-8 form, you need to use identifier_to_locale on
>> it to produce something suitable for a diagnostic.  And for C++ you need
>> to make sure the name is a proper pretty name (including classes /
>> namespaces / type etc.) as produced by the decl_printable_name langhook,
>> before passing it to identifier_to_locale.
>
> I think you just want to pass e->caller->decl (with corresponding % formatter)
> rather than name()
Hi,
Thanks for the reviews. However after discussing with Richard,
we decided to drop this warning for now, because it can lead to
potentially false positives
like for the following case in stor-layout.c:

   /* Stop if the mode requires too much alignment.  */
      if (GET_MODE_ALIGNMENT (m_mode) > m_align
          && SLOW_UNALIGNED_ACCESS (m_mode, m_align))
        break;

On x86_64, SLOW_UNALIGNED_ACCESS is #defined to 0,
so the condition essentially becomes:

if (get_mode_alignment (m_mode) > m_align && 0)
  break;

and the patch warns for the above dead call.
However the call might not always be dead, since it depends
on conditionally defined macro SLOW_UNALIGNED_ACCESS,
which other targets may perhaps define as a run-time value.

Unfortunately I don't have any good ideas to address this issue.
We could restrict the warning for cases when call is not a
sub-expression, however I suppose we would need some help from
FE's to determine if call_expr is outermost expression ?
I thought of adding another flag to tree_exp for this purpose,
but that doesn't look like a good idea.
I would be grateful for suggestions for addressing this issue.

Thanks,
Prathamesh
>
> Honza