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