On Fri, 4 Apr 2014, Jan Hubicka wrote:
> > 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?
Yes, they are only expected to appear with fortification and yes, we
ignore them ;)
Richard.
> 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
>
>
--
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
