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

[Richard Biener <rguenther at suse dot de>]

On Sun, 31 Jul 2016, Prathamesh Kulkarni wrote:

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

Maybe it's possible to restructure it in a way to rely on
TREE_USED on the decl the result is assigned to and only warn
in the case the FE didn't set that (and the function later became
pure/const).  That would not warn for, say,

  x = foo ();
  x;

Well, I hope so.

Richard.