Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Richard Biener <rguenther@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]

On Wed, 26 Sep 2018, Jason Merrill wrote:

> On Fri, Sep 21, 2018 at 11:12 AM, Qing Zhao <qing.zhao@oracle.com> wrote:
> > Hi, this is the 4th version of the patch.
> >
> > mainly address Martin’s comments on some spelling issues.
> >
> > I have tested the patch on both x86 and aarch64, no issue.
> >
> > Okay for commit?
> >
> > thanks.
> >
> > Qing.
> >
> > gcc/ChangeLog
> >
> > +2018-09-20  Qing Zhao  <qing.zhao@oracle.com>
> > +
> > +       * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
> > +       * common.opt (-finline-only-static): New option.
> > +       * doc/invoke.texi: Document -finline-only-static.
> > +       * ipa-inline.c (can_inline_edge_p): Control inlining based on
> > +       function's linkage.
> 
> I would suggest "internal" rather than "static" in general.  So
> 
> +    N_("function has external linkage when the user requests only"
> +       " inlining functions with internal linkage"))
> 
> +Inline functions only if they have internal linkage.
> 
> +@item -finline-only-internal
> +@opindex finline-only-internal
> +By default, GCC inlines functions without considering their linkage.
> +This flag guides the inliner to only inline functions with internal linkage.
> +This option has any effect only when inlining itself is turned on by the
> +-finline-functions or -finline-small-functions options.
> 
> This should also mention whether it applies to functions explicitly
> declared inline; I assume it does not.

IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
what 'internal' would mean in this context.

But then the implementation looks at callee->externally_visible which
matches hidden visibility... externally_visible is probably not
the very best thing to look at depending on what we intend to do.

What about 'static' functions with their address taken?

Note you shouldn't look at individual cgraph_node fields but use
some of the accessors with more well-constrained semantics.
Why are you not simply checking !TREE_PUBLIC?

Honza might be able to suggest one.

Richard.