> On Fri, 4 Apr 2014, Jan Hubicka wrote:
>
> > Hi,
> > here is an updated version of my earlier ipa.c change. It turns out that
> > the
> > problem was that I did not drop always_inline.
> > In this version I just drop always_inline attribute on all functions whose
> > body
> > is removed. The patch will affect non-LTO compilation, too, but IMO only by
> > making us to not inline&diagnose the cases where indirect call to always
> > inline
> > is turned direct in between early opts and inline. Does that seem
> > acceptable?
> > (I personally would preffer this over inventing yet another way to special
> > case
> > always_inline for LTO only; we never made any strong promises on
> > always_inline
> > and indirect calls)
>
> I think it's fine if indirect calls to always-inline fns go to an
> offline copy (or cause a link-time error if there is no offline copy).
> I've always thought that taking the address of an always_inline fn
> should get you the address of an "external" symbol (an inline "clone"
> isn't addressable).
>
> So the patch is fine with me.
OK, I comitted it. I forgot to mention that with fortified bootstrap I got
quite few extra warning on return value of some functions being ignored
(such as fscanf). Are those expected to appear only with fortification and
are we expected to cowardly ignore them?
Honza
>
> Thanks,
> Richard.
>
> > Honza
> >
> > * lto-cgraph.c (input_overwrite_node): Check that partitioning
> > flags are set only during streaming.
> > * ipa.c (process_references, walk_polymorphic_call_targets,
> > symtab_remove_unreachable_nodes): Drop bodies of always inline
> > after early inlining.
> > (symtab_remove_unreachable_nodes): Remove always_inline attribute.
> >
> > * gcc.dg/lto/pr59626_0.c: New testcase.
> > * gcc.dg/lto/pr59626_1.c: New testcase.
> > Index: lto-cgraph.c
> > ===================================================================
> > --- lto-cgraph.c (revision 209062)
> > +++ lto-cgraph.c (working copy)
> > @@ -1001,6 +1001,9 @@ input_overwrite_node (struct lto_file_de
> > node->thunk.thunk_p = bp_unpack_value (bp, 1);
> > node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
> > LDPR_NUM_KNOWN);
> > + gcc_assert (flag_ltrans
> > + || (!node->in_other_partition
> > + && !node->used_from_other_partition));
> > }
> >
> > /* Return string alias is alias of. */
> > @@ -1169,6 +1172,9 @@ input_varpool_node (struct lto_file_decl
> > node->same_comdat_group = (symtab_node *) (intptr_t) ref;
> > node->resolution = streamer_read_enum (ib, ld_plugin_symbol_resolution,
> > LDPR_NUM_KNOWN);
> > + gcc_assert (flag_ltrans
> > + || (!node->in_other_partition
> > + && !node->used_from_other_partition));
> >
> > return node;
> > }
> > Index: ipa.c
> > ===================================================================
> > --- ipa.c (revision 209062)
> > +++ ipa.c (working copy)
> > @@ -139,7 +139,10 @@ process_references (struct ipa_ref_list
> >
> > if (node->definition && !node->in_other_partition
> > && ((!DECL_EXTERNAL (node->decl) || node->alias)
> > - || (before_inlining_p
> > + || (((before_inlining_p
> > + && (cgraph_state < CGRAPH_STATE_IPA_SSA
> > + || !lookup_attribute ("always_inline",
> > + DECL_ATTRIBUTES (node->decl)))))
> > /* We use variable constructors during late complation for
> > constant folding. Keep references alive so partitioning
> > knows about potential references. */
> > @@ -191,7 +194,10 @@ walk_polymorphic_call_targets (pointer_s
> > /* Prior inlining, keep alive bodies of possible targets for
> > devirtualization. */
> > if (n->definition
> > - && before_inlining_p)
> > + && (before_inlining_p
> > + && (cgraph_state < CGRAPH_STATE_IPA_SSA
> > + || !lookup_attribute ("always_inline",
> > + DECL_ATTRIBUTES (n->decl)))))
> > pointer_set_insert (reachable, n);
> >
> > /* Even after inlining we want to keep the possible targets in the
> > @@ -491,6 +497,12 @@ symtab_remove_unreachable_nodes (bool be
> > node->alias = false;
> > node->thunk.thunk_p = false;
> > node->weakref = false;
> > + /* After early inlining we drop always_inline attributes on
> > + bodies of functions that are still referenced (have their
> > + address taken). */
> > + DECL_ATTRIBUTES (node->decl)
> > + = remove_attribute ("always_inline",
> > + DECL_ATTRIBUTES (node->decl));
> > if (!node->in_other_partition)
> > node->local.local = false;
> > cgraph_node_remove_callees (node);
> > Index: testsuite/gcc.dg/lto/pr59626_1.c
> > ===================================================================
> > --- testsuite/gcc.dg/lto/pr59626_1.c (revision 0)
> > +++ testsuite/gcc.dg/lto/pr59626_1.c (revision 0)
> > @@ -0,0 +1,4 @@
> > +int bar (int (*fn)(const char *))
> > +{
> > + return fn ("0");
> > +}
> > Index: testsuite/gcc.dg/lto/pr59626_0.c
> > ===================================================================
> > --- testsuite/gcc.dg/lto/pr59626_0.c (revision 0)
> > +++ testsuite/gcc.dg/lto/pr59626_0.c (revision 0)
> > @@ -0,0 +1,15 @@
> > +/* { dg-lto-do run } */
> > +
> > +int __atoi (const char *) __asm__("atoi");
> > +extern inline __attribute__((always_inline,gnu_inline))
> > +int atoi (const char *x)
> > +{
> > + return __atoi (x);
> > +}
> > +
> > +int bar (int (*)(const char *));
> > +
> > +int main()
> > +{
> > + return bar (atoi);
> > +}
> >
> >
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
