>
> Why don't we represent the alias at the cgraph level? That is,
> why do decls come into play at all here? b prevails and it has a
> reference to a.c:a so we need to keep (and emit) that. The only issue
> would be that we'd end up with two 'a's so we have to notice that
> and fixup somehow.
>
> Isn't this in the end a similar issue as with extern inline bodies
> vs. the offline copy body?
The difference is that, for the first time, we need to load two
different bodies for one DECL (because first is reachable by alias in
a.c and other is reahable by the prevailing non-weak declaration).
Function bodies are bound to decls and not symbols and this needs to be done
somehow.
>
> > I would like to do this w/o need to stream in the body, because such
> > situation
> > happens quite commonly local aliases: Until we are able to prove that all
> > referencs to a local aliase of a symbol are gone we need to assume that body
> > needs to be preserved and those are potentially quite frequent.
> >
> > So basically only solution I think of is to make lto-stream-in live with the
> > fact that function decl was changed. So we can stream original decl to the
> > beggining of the function section and during stream in do rewritting of
> > DECL_CONTEXT similar way as tree-inline would do. We need to be careful
> > about
> > keeping debug info in shape too (we are not producing inline copy, we are
> > merely unsharing the main decl).
> >
> > What do you think?
>
> I think the not prevailed but still needed cgraph node for 'a' should
> have its decl cleared ;)
I am not sure I understand what you mean here :)
>
> > Note that I was considering case of simply not sharing decls with
> > aliases at stream time. This does not work because by localizing the
> > decl one will not redirect references to the symbol correctly.
>
> But all remaining references should be via 'b', not via 'a'.
Yes, they are, that is why I create a.static and b becomes alias of it.
Moving body of a.c:a to b would also work, but it is essentially the
same problem. All we would need to do is to pick an "winning" alias,
make it the main definition and make aliases alias of it.
I think this would only introduce furhter problem, since signature of b
is not guaranteed to be same as signature of a.
Honza
>
> > Other option would be to always produce a static symbol for every
> > symbol with an alias. This again can be IMO only done by cloning it and
> > would be relatively expensive because at the stream out time we do have
> > considerable quantity of aliases especially with -fpic.
> >
> > Honza
> >
> > gcc/ChangeLog:
> >
> > 2020-03-26 Jan Hubicka <hubi...@ucw.cz>
> >
> > * cgraph.c (cgraph_node::make_local): Add force parameter.
> > * cgraph.h (symtab_node::clone_referring_skip_aliases): New
> > member function.
> > (symtab_node::stream_in_summary_p): Likewise.
> > * ipa-fnsummary.c (inline_read_section): Use
> > symtab_node::stream_in_summary_p.
> > * ipa-prop.c (ipa_read_node_info): Likewise.
> > * symtab.c (symtab_node::clone_referring): Fix formating.
> > (symtab_node::clone_referring_skip_aliases): New member function.
> > (symtab_node::dump_referring): Fix formating
> > (symtab_node::stream_in_summary_p): New member function.
> >
> > gcc/lto/ChangeLog:
> >
> > 2020-03-26 Jan Hubicka <hubi...@ucw.cz>
> >
> > * lto-symtab.c (lto_symtab_symbol_p): New function.
> > (has_prevailing_alias_p): New function.
> > (lto_symtab_replace_node): Break out from ...; support alias
> > interposition
> > (lto_cgraph_replace_node): ... here.
> > (lto_varpool_replace_node): Use the common code.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2020-03-26 Jan Hubicka <hubi...@ucw.cz>
> >
> > * gcc.dg/lto/alias-resolution-1_0.c: New test.
> > * gcc.dg/lto/alias-resolution-1_1.c: New test.
> >
> > diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> > index 6b780f80eb3..1f1319ed5fe 100644
> > --- a/gcc/cgraph.c
> > +++ b/gcc/cgraph.c
> > @@ -2465,9 +2465,10 @@ cgraph_node::call_for_symbol_thunks_and_aliases
> > (bool (*callback)
> > /* Worker to bring NODE local. */
> >
> > bool
> > -cgraph_node::make_local (cgraph_node *node, void *)
> > +cgraph_node::make_local (cgraph_node *node, void *force)
> > {
> > - gcc_checking_assert (node->can_be_local_p ());
> > + if (!force)
> > + gcc_checking_assert (node->can_be_local_p ());
> > if (DECL_COMDAT (node->decl) || DECL_EXTERNAL (node->decl))
> > {
> > node->make_decl_local ();
> > @@ -2486,12 +2487,14 @@ cgraph_node::make_local (cgraph_node *node, void *)
> > return false;
> > }
> >
> > -/* Bring cgraph node local. */
> > +/* Bring cgraph node local. If FORCE is true skip checking if symbol can
> > + become local. */
> >
> > void
> > -cgraph_node::make_local (void)
> > +cgraph_node::make_local (bool force)
> > {
> > - call_for_symbol_thunks_and_aliases (cgraph_node::make_local, NULL, true);
> > + call_for_symbol_thunks_and_aliases (cgraph_node::make_local,
> > + (void *)(size_t)force, true);
> > }
> >
> > /* Worker to set nothrow flag. */
> > diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> > index 43de3b4a8ac..983cbfe8cd8 100644
> > --- a/gcc/cgraph.h
> > +++ b/gcc/cgraph.h
> > @@ -189,6 +189,9 @@ public:
> > with callgraph edges associated with them. */
> > void clone_referring (symtab_node *node);
> >
> > + /* Same as clone_referring but skip aliases. */
> > + void clone_referring_skip_aliases (symtab_node *node);
> > +
> > /* Clone reference REF to this symtab_node and set its stmt to STMT. */
> > ipa_ref *clone_reference (ipa_ref *ref, gimple *stmt);
> >
> > @@ -340,6 +343,9 @@ public:
> > in question prevails in the linking to save some memory usage. */
> > bool prevailing_p (void);
> >
> > + /* Return true if the summary should be streamed in. */
> > + bool stream_in_summary_p (void);
> > +
> > /* Return true if NODE binds to current definition in final executable
> > when referenced from REF. If REF is NULL return conservative value
> > for any reference. */
> > @@ -1163,8 +1169,9 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node :
> > public symtab_node
> > /* cgraph_node is no longer nested function; update cgraph accordingly.
> > */
> > void unnest (void);
> >
> > - /* Bring cgraph node local. */
> > - void make_local (void);
> > + /* Bring cgraph node local. If FORCE is true skip checking if symbol can
> > + be local. */
> > + void make_local (bool force = false);
> >
> > /* Likewise indicate that a node is having address taken. */
> > void mark_address_taken (void);
> > diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
> > index b411bc4d660..a79e75dd001 100644
> > --- a/gcc/ipa-fnsummary.c
> > +++ b/gcc/ipa-fnsummary.c
> > @@ -4176,10 +4176,10 @@ inline_read_section (struct lto_file_decl_data
> > *file_data, const char *data,
> > encoder = file_data->symtab_node_encoder;
> > node = dyn_cast<cgraph_node *> (lto_symtab_encoder_deref (encoder,
> > index));
> > - info = node->prevailing_p () ? ipa_fn_summaries->get_create (node) :
> > NULL;
> > - params_summary = node->prevailing_p () ? IPA_NODE_REF (node) : NULL;
> > - size_info = node->prevailing_p ()
> > - ? ipa_size_summaries->get_create (node) : NULL;
> > + bool stream_in = node->stream_in_summary_p ();
> > + info = stream_in ? ipa_fn_summaries->get_create (node) : NULL;
> > + params_summary = stream_in ? IPA_NODE_REF (node) : NULL;
> > + size_info = stream_in ? ipa_size_summaries->get_create (node) : NULL;
> >
> > int stack_size = streamer_read_uhwi (&ib);
> > int size = streamer_read_uhwi (&ib);
> > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> > index a77130ded39..4035d903d33 100644
> > --- a/gcc/ipa-prop.c
> > +++ b/gcc/ipa-prop.c
> > @@ -4937,7 +4937,7 @@ ipa_read_node_info (class lto_input_block *ib, struct
> > cgraph_node *node,
> > int k;
> > struct cgraph_edge *e;
> > struct bitpack_d bp;
> > - bool prevails = node->prevailing_p ();
> > + bool prevails = node->stream_in_summary_p ();
> > class ipa_node_params *info = prevails
> > ? IPA_NODE_REF_GET_CREATE (node) : NULL;
> >
> > diff --git a/gcc/lto/lto-symtab.c b/gcc/lto/lto-symtab.c
> > index d5e38beb4b6..3f4ece360d7 100644
> > --- a/gcc/lto/lto-symtab.c
> > +++ b/gcc/lto/lto-symtab.c
> > @@ -36,6 +36,150 @@ along with GCC; see the file COPYING3. If not see
> > #include "stringpool.h"
> > #include "attribs.h"
> >
> > +/* Return true, if the symbol E should be resolved by lto-symtab.
> > + Those are all external symbols and all real symbols that are not static
> > (we
> > + handle renaming of static later in partitioning). */
> > +
> > +static bool
> > +lto_symtab_symbol_p (symtab_node *e)
> > +{
> > + if (!TREE_PUBLIC (e->decl) && !DECL_EXTERNAL (e->decl))
> > + return false;
> > + return e->real_symbol_p ();
> > +}
> > +
> > +/* Return ture if NODE is an prevailing alias or has alias that prevalis.
> > */
> > +
> > +static bool
> > +has_prevailing_alias_p (struct symtab_node *node)
> > +{
> > + ipa_ref *ref;
> > +
> > + FOR_EACH_ALIAS (node, ref)
> > + {
> > + symtab_node *alias = ref->referring;
> > +
> > + if (lto_symtab_symbol_p (alias))
> > + return alias->prevailing_p ();
> > + else
> > + return true;
> > + if (has_prevailing_alias_p (alias))
> > + return true;
> > + }
> > + return false;
> > +}
> > +
> > +/* Common part of lto_cgraph_replace_node and lto_varpool_replace_node
> > + Retun true if NODE can be removed. */
> > +
> > +static bool
> > +lto_symtab_replace_node (struct symtab_node *node,
> > + struct symtab_node *prevailing_node)
> > +{
> > + /* Merge node flags. */
> > + if (node->force_output)
> > + prevailing_node->force_output = true;
> > + if (node->forced_by_abi)
> > + prevailing_node->forced_by_abi = true;
> > + if (node->address_taken)
> > + {
> > + prevailing_node->address_taken = true;
> > + prevailing_node->ultimate_alias_target ()->address_taken = true;
> > + }
> > + /* Redirect incomming references. */
> > + prevailing_node->clone_referring_skip_aliases (node);
> > +
> > + node->set_comdat_group (NULL);
> > + /* Dissolve same comdat group. In most cases this is not necessary
> > because
> > + all symbols in the group will be previaled by the symbols in
> > prevailing
> > + group, but in the case of ODR violations we do not want to end up with
> > + multiple comdat groups of same name. */
> > + if (node->same_comdat_group)
> > + {
> > + if (dyn_cast <cgraph_node *> (node))
> > + dyn_cast <cgraph_node *> (node)->calls_comdat_local = false;
> > + for (symtab_node *next = node->same_comdat_group;
> > + next != node; next = next->same_comdat_group)
> > + {
> > + if (dyn_cast <cgraph_node *> (next))
> > + dyn_cast <cgraph_node *> (next)->calls_comdat_local = false;
> > + next->set_comdat_group (NULL);
> > + }
> > + node->dissolve_same_comdat_group_list ();
> > + }
> > +
> > + /* If there is alias that prevails the definition remains reachable.
> > + Turn it to a static symbol. */
> > + if (has_prevailing_alias_p (node))
> > + {
> > + bool copy = false;
> > +
> > + /* See if decl is shared with another symbol definition. In this
> > case
> > + we must copy it. */
> > + for (symtab_node *e = node->previous_sharing_asm_name;
> > + e && copy; e = e->previous_sharing_asm_name)
> > + if (e->decl == node->decl
> > + && e->stream_in_summary_p ())
> > + copy = true;
> > + for (symtab_node *e = node->next_sharing_asm_name;
> > + e && copy; e = e->next_sharing_asm_name)
> > + if (e->decl == node->decl
> > + && e->stream_in_summary_p ())
> > + copy = true;
> > +
> > + if (copy)
> > + {
> > + struct lto_in_decl_state temp;
> > + struct lto_in_decl_state *state;
> > +
> > + /* We are going to move the decl, we want to remove its file decl
> > + data and link these with the new decl. */
> > + temp.fn_decl = node->decl;
> > + lto_in_decl_state **slot
> > + = node->lto_file_data->function_decl_states->find_slot (&temp,
> > + NO_INSERT);
> > + state = *slot;
> > + node->lto_file_data->function_decl_states->clear_slot (slot);
> > + gcc_assert (state);
> > +
> > + /* Duplicate the decl and be sure it does not link into body
> > + of DST. */
> > + node->decl = copy_node (node->decl);
> > + node->decl->decl_with_vis.symtab_node = node;
> > + DECL_STRUCT_FUNCTION (node->decl) = NULL;
> > + DECL_ARGUMENTS (node->decl) = NULL;
> > + DECL_INITIAL (node->decl) = NULL;
> > + DECL_RESULT (node->decl) = NULL;
> > +
> > + /* Associate the decl state with new declaration, so LTO streamer
> > + can look it up. */
> > + state->fn_decl = node->decl;
> > + slot
> > + = node->lto_file_data->function_decl_states->find_slot (state,
> > + INSERT);
> > + gcc_assert (!*slot);
> > + *slot = state;
> > + }
> > + node->make_decl_local ();
> > + return false;
> > + }
> > + else
> > + {
> > + ipa_ref *ref;
> > +
> > + /* All aliases are going to be removed, but we need to do that only
> > + after all replacements are completed. To keep verifier happy turn
> > + them into forward declarations. */
> > + FOR_EACH_ALIAS (node, ref)
> > + {
> > + ref->referring->definition = false;
> > + ref->referring->alias = false;
> > + ref->referring->analyzed = false;
> > + }
> > + }
> > + return true;
> > +}
> > +
> > /* Replace the cgraph node NODE with PREVAILING_NODE in the cgraph, merging
> > all edges and removing the old node. */
> >
> > @@ -56,16 +200,8 @@ lto_cgraph_replace_node (struct cgraph_node *node,
> > (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (node->decl)))));
> > }
> >
> > - /* Merge node flags. */
> > - if (node->force_output)
> > - prevailing_node->mark_force_output ();
> > - if (node->forced_by_abi)
> > - prevailing_node->forced_by_abi = true;
> > - if (node->address_taken)
> > - {
> > - gcc_assert (!prevailing_node->inlined_to);
> > - prevailing_node->mark_address_taken ();
> > - }
> > + bool remove_p = lto_symtab_replace_node (node, prevailing_node);
> > +
> > if (node->definition && prevailing_node->definition
> > && DECL_COMDAT (node->decl) && DECL_COMDAT (prevailing_node->decl))
> > prevailing_node->merged_comdat = true;
> > @@ -95,15 +231,16 @@ lto_cgraph_replace_node (struct cgraph_node *node,
> > e->call_stmt_cannot_inline_p = 1;
> > }
> > }
> > - /* Redirect incomming references. */
> > - prevailing_node->clone_referring (node);
> > - lto_free_function_in_decl_state_for_node (node);
> > + if (remove_p)
> > + {
> > + lto_free_function_in_decl_state_for_node (node);
> >
> > - if (node->decl != prevailing_node->decl)
> > - node->release_body ();
> > + if (node->decl != prevailing_node->decl)
> > + node->release_body ();
> >
> > - /* Finally remove the replaced node. */
> > - node->remove ();
> > + /* Finally remove the replaced node. */
> > + node->remove ();
> > + }
> > }
> >
> > /* Replace the cgraph node NODE with PREVAILING_NODE in the cgraph, merging
> > @@ -116,11 +253,7 @@ lto_varpool_replace_node (varpool_node *vnode,
> > gcc_assert (!vnode->definition || prevailing_node->definition);
> > gcc_assert (!vnode->analyzed || prevailing_node->analyzed);
> >
> > - prevailing_node->clone_referring (vnode);
> > - if (vnode->force_output)
> > - prevailing_node->force_output = true;
> > - if (vnode->forced_by_abi)
> > - prevailing_node->forced_by_abi = true;
> > + bool remove_p = lto_symtab_replace_node (vnode, prevailing_node);
> >
> > /* Be sure we can garbage collect the initializer. */
> > if (DECL_INITIAL (vnode->decl)
> > @@ -142,7 +275,7 @@ lto_varpool_replace_node (varpool_node *vnode,
> > || vnode->tls_model == TLS_MODEL_NONE
> > || vnode->tls_model == TLS_MODEL_EMULATED)
> > error = true;
> > - /* Linked is silently supporting transitions
> > + /* Linker is silently supporting transitions
> > GD -> IE, GD -> LE, LD -> LE, IE -> LE, LD -> IE.
> > Do the same transitions and error out on others. */
> > else if ((prevailing_node->tls_model == TLS_MODEL_REAL
> > @@ -166,14 +299,16 @@ lto_varpool_replace_node (varpool_node *vnode,
> > if (error)
> > {
> > error_at (DECL_SOURCE_LOCATION (vnode->decl),
> > - "%qD is defined with tls model %s", vnode->decl,
> > tls_model_names [vnode->tls_model]);
> > + "%qD is defined with tls model %s", vnode->decl,
> > + tls_model_names[vnode->tls_model]);
> > inform (DECL_SOURCE_LOCATION (prevailing_node->decl),
> > "previously defined here as %s",
> > - tls_model_names [prevailing_node->tls_model]);
> > + tls_model_names[prevailing_node->tls_model]);
> > }
> > }
> > /* Finally remove the replaced node. */
> > - vnode->remove ();
> > + if (remove_p)
> > + vnode->remove ();
> > }
> >
> > /* Return non-zero if we want to output waring about T1 and T2.
> > @@ -408,18 +543,6 @@ lto_symtab_resolve_replaceable_p (symtab_node *e)
> > return false;
> > }
> >
> > -/* Return true, if the symbol E should be resolved by lto-symtab.
> > - Those are all external symbols and all real symbols that are not static
> > (we
> > - handle renaming of static later in partitioning). */
> > -
> > -static bool
> > -lto_symtab_symbol_p (symtab_node *e)
> > -{
> > - if (!TREE_PUBLIC (e->decl) && !DECL_EXTERNAL (e->decl))
> > - return false;
> > - return e->real_symbol_p ();
> > -}
> > -
> > /* Return true if the symtab entry E can be the prevailing one. */
> >
> > static bool
> > diff --git a/gcc/symtab.c b/gcc/symtab.c
> > index 3022acfc1ea..4745c2c94ec 100644
> > --- a/gcc/symtab.c
> > +++ b/gcc/symtab.c
> > @@ -685,7 +685,7 @@ symtab_node::clone_referring (symtab_node *node)
> > {
> > ipa_ref *ref = NULL, *ref2 = NULL;
> > int i;
> > - for (i = 0; node->iterate_referring(i, ref); i++)
> > + for (i = 0; node->iterate_referring (i, ref); i++)
> > {
> > bool speculative = ref->speculative;
> > unsigned int stmt_uid = ref->lto_stmt_uid;
> > @@ -697,6 +697,27 @@ symtab_node::clone_referring (symtab_node *node)
> > }
> > }
> >
> > +/* Clone all referring from symtab NODE to this symtab_node
> > + Skip aliases. */
> > +
> > +void
> > +symtab_node::clone_referring_skip_aliases (symtab_node *node)
> > +{
> > + ipa_ref *ref = NULL, *ref2 = NULL;
> > + int i;
> > + for (i = 0; node->iterate_referring (i, ref); i++)
> > + if (ref->use != IPA_REF_ALIAS)
> > + {
> > + bool speculative = ref->speculative;
> > + unsigned int stmt_uid = ref->lto_stmt_uid;
> > +
> > + ref2 = ref->referring->create_reference (this, ref->use, ref->stmt);
> > + ref2->speculative = speculative;
> > + ref2->lto_stmt_uid = stmt_uid;
> > + ref2->speculative_id = ref->speculative_id;
> > + }
> > +}
> > +
> > /* Clone reference REF to this symtab_node and set its stmt to STMT. */
> >
> > ipa_ref *
> > @@ -813,7 +834,7 @@ symtab_node::dump_referring (FILE *file)
> > {
> > ipa_ref *ref = NULL;
> > int i;
> > - for (i = 0; iterate_referring(i, ref); i++)
> > + for (i = 0; iterate_referring (i, ref); i++)
> > {
> > fprintf (file, "%s (%s)",
> > ref->referring->dump_asm_name (),
> > @@ -2490,3 +2511,21 @@ symtab_node::output_to_lto_symbol_table_p (void)
> > }
> > return true;
> > }
> > +
> > +/* Return true if summary should be streamed in. */
> > +bool
> > +symtab_node::stream_in_summary_p ()
> > +{
> > + ipa_ref *ref;
> > + if (prevailing_p ())
> > + return true;
> > + FOR_EACH_ALIAS (this, ref)
> > + {
> > + symtab_node *alias = ref->referring;
> > +
> > + if (!alias->externally_visible
> > + || alias->stream_in_summary_p ())
> > + return true;
> > + }
> > + return false;
> > +}
> > diff --git a/gcc/testsuite/gcc.dg/lto/alias-resolution-1_0.c
> > b/gcc/testsuite/gcc.dg/lto/alias-resolution-1_0.c
> > new file mode 100644
> > index 00000000000..4a8471244f6
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/lto/alias-resolution-1_0.c
> > @@ -0,0 +1,20 @@
> > +/* { dg-require-alias "" } */
> > +#define ASMNAME2(prefix, cname) STRING (prefix) cname
> > +#define STRING(x) #x
> > +#define ASMNAME(cname) ASMNAME2 (__USER_LABEL_PREFIX__, cname)
> > +__attribute__((weak))
> > +__attribute__((noinline))
> > +int a() __asm__ (ASMNAME ("a"));
> > +
> > +int a()
> > +{
> > + return 1;
> > +}
> > +__attribute__((noinline))
> > +static int b() __attribute__((alias("a")));
> > +void
> > +test()
> > +{
> > + if (b()!=1)
> > + __builtin_abort ();
> > +}
> > diff --git a/gcc/testsuite/gcc.dg/lto/alias-resolution-1_1.c
> > b/gcc/testsuite/gcc.dg/lto/alias-resolution-1_1.c
> > new file mode 100644
> > index 00000000000..f25cfa27bc7
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/lto/alias-resolution-1_1.c
> > @@ -0,0 +1,19 @@
> > +#define ASMNAME(cname) ASMNAME2 (__USER_LABEL_PREFIX__, cname)
> > +#define ASMNAME2(prefix, cname) STRING (prefix) cname
> > +#define STRING(x) #x
> > +__attribute__((noinline))
> > +int a() __asm__ (ASMNAME ("a"));
> > +
> > +int a()
> > +{
> > + return 2;
> > +}
> > +extern void test ();
> > +int
> > +main()
> > +{
> > + if (a() != 2)
> > + __builtin_abort ();
> > + test();
> > + return 0;
> > +}
> >
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
