This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: lto-symtab and cgraph aliases


On Mon, Jul 5, 2010 at 9:55 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> While compiling Mozilla, we end up with ICE becuase we replace thunk
>
> for _ZN7nanojit9LirWriterD1Ev by ordinary node. WHile merging in
> unmerged cgraph we get couple
> __base_dtor /9369(-1) @0x7f2f66f8c2b0 (asm: _ZN7nanojit9LirWriterD2Ev) analyzed 1 time, 12 benefit 1 size, 3 benefit address_taken externally_visible finalized inlinable
> ?called by:
> ?calls:
> ?References: ?var:_ZTVN7nanojit9LirWriterE (addr)
> ?Refering this function: ?var:_ZTVN7nanojit9LirWriterE (addr) var:_ZTVN7nanojit9LirWriterE (addr) var:_ZTVN7nanojit9LirWriterE (addr)
> ?aliases & thunks: __comp_dtor /9370 (asm: _ZN7nanojit9LirWriterD1Ev)
>
> We end p removing all and producing new cgraph node:
>
> __comp_dtor /0(-1) @0x7f2f674f2408 (asm: _ZN7nanojit9LirWriterD1Ev) availability:not_available reachable
> ?called by:
> ?calls:
> ?References:
> ?Refering this function: ?var:_ZTVN7nanojit9LirWriterE (addr) var:_ZTVN7nanojit9LirWriterE (addr) var:_ZTVN7nanojit9LirWriterE (addr)
>
> That is bogys (it is alias node put into main linked list of cgraph nodes,
> see UID of 0 etc.)
>
> I ended up reorganizing lto-symtab code since the original did not make much
> sense to me. ?I also run into problem producing resonable testcase for this.
> We need multiple files and the library linked into it. Moreover one needs
> linker plugin (otherwise we do not ice because of different prevailing
> decisions)
>
> Our current code seems to be build around assumption that aliases will always
> be the same (this is not neccesarily true with object files with different ABI
> settings) and that we can thus merge en-masse first time we merge two symbols
> from the alias list.
>
> Thus entry->node points to function, even for DECLs that are actually thunks
> or aliases. ?This is different from varpool where entry->vnode points to variable
> only for node prepresenting the variable and to alias for aliases.
>
> This patch turns cgraph nodes into same logic as we do for varpool nodes.
> entry->node points to node or alias (we need new accestor function for that
> cgraph_get_node_or_alias) and when merging symbols we merge the corresponding
> nodes and aliases only.
>
> Bootstrapped/regtested x86_64-linux and also tested on whopr bootstrap + mozilla
> build. Seems to make sense?

Changelog is incomplete (missing cgraph.h change).

Otherwise it makes sense.

Thanks,
Richard.


> ? ? ? ?* lto-symtab.c (lto_cgraph_replace_node): Handle aliases.
> Index: lto-symtab.c
> ===================================================================
> --- lto-symtab.c ? ? ? ?(revision 161847)
> +++ lto-symtab.c ? ? ? ?(working copy)
> @@ -206,6 +206,24 @@ lto_cgraph_replace_node (struct cgraph_n
> ? ? ? ? ? ? ? ? ? ? ? ? struct cgraph_node *prevailing_node)
> ?{
> ? struct cgraph_edge *e, *next;
> + ?bool no_aliases_please = false;
> +
> + ?if (cgraph_dump_file)
> + ? ?{
> + ? ? ?fprintf (cgraph_dump_file, "Replacing cgraph node %s/%i by %s/%i"
> + ? ? ? ? ? ? ?" for symbol %s\n",
> + ? ? ? ? ? ? ?cgraph_node_name (node), node->uid,
> + ? ? ? ? ? ? ?cgraph_node_name (prevailing_node),
> + ? ? ? ? ? ? ?prevailing_node->uid,
> + ? ? ? ? ? ? ?IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (node->decl)));
> + ? ?}
> +
> + ?if (prevailing_node->same_body_alias)
> + ? ?{
> + ? ? ?if (prevailing_node->thunk.thunk_p)
> + ? ? ? no_aliases_please = true;
> + ? ? ?prevailing_node = prevailing_node->same_body;
> + ? ?}
>
> ? /* Merge node flags. ?*/
> ? if (node->needed)
> @@ -227,27 +244,37 @@ lto_cgraph_replace_node (struct cgraph_n
> ? /* Redirect incomming references. ?*/
> ? ipa_clone_refering (prevailing_node, NULL, &node->ref_list);
>
> - ?if (node->same_body)
> + ?/* If we have aliases, redirect them to the prevailing node. ?*/
> + ?if (!node->same_body_alias && node->same_body)
> ? ? {
> - ? ? ?struct cgraph_node *alias;
> + ? ? ?struct cgraph_node *alias, *last;
> + ? ? ?/* We prevail aliases/tunks by a thunk. ?This is doable but
> + ? ? ? ? would need thunk combination. ?Hopefully no ABI changes will
> + ? ? ? ? every be crazy enough. ?*/
> + ? ? ?gcc_assert (!no_aliases_please);
>
> ? ? ? for (alias = node->same_body; alias; alias = alias->next)
> - ? ? ? if (DECL_ASSEMBLER_NAME_SET_P (alias->decl))
> - ? ? ? ? {
> - ? ? ? ? ? lto_symtab_entry_t se
> - ? ? ? ? ? ? = lto_symtab_get (DECL_ASSEMBLER_NAME (alias->decl));
> -
> - ? ? ? ? ? for (; se; se = se->next)
> - ? ? ? ? ? ? if (se->node == node)
> - ? ? ? ? ? ? ? {
> - ? ? ? ? ? ? ? ? se->node = NULL;
> - ? ? ? ? ? ? ? ? break;
> - ? ? ? ? ? ? ? }
> - ? ? ? ? }
> + ? ? ? {
> + ? ? ? ? last = alias;
> + ? ? ? ? gcc_assert (alias->same_body_alias);
> + ? ? ? ? alias->same_body = prevailing_node;
> + ? ? ? ? alias->thunk.alias = prevailing_node->decl;
> + ? ? ? }
> + ? ? ?last->next = prevailing_node->same_body;
> + ? ? ?/* Node with aliases is prevailed by alias.
> + ? ? ? ?We could handle this, but combining thunks together will be tricky.
> + ? ? ? ?Hopefully this does not happen. ?*/
> + ? ? ?if (prevailing_node->same_body)
> + ? ? ? prevailing_node->same_body->previous = last;
> + ? ? ?prevailing_node->same_body = node->same_body;
> + ? ? ?node->same_body = NULL;
> ? ? }
>
> ? /* Finally remove the replaced node. ?*/
> - ?cgraph_remove_node (node);
> + ?if (node->same_body_alias)
> + ? ?cgraph_remove_same_body_alias (node);
> + ?else
> + ? ?cgraph_remove_node (node);
> ?}
>
> ?/* Replace the cgraph node NODE with PREVAILING_NODE in the cgraph, merging
> @@ -433,7 +460,9 @@ lto_symtab_resolve_can_prevail_p (lto_sy
>
> ? /* For functions we need a non-discarded body. ?*/
> ? if (TREE_CODE (e->decl) == FUNCTION_DECL)
> - ? ?return (e->node && e->node->analyzed);
> + ? ?return (e->node
> + ? ? ? ? ? && (e->node->analyzed
> + ? ? ? ? ? ? ? || (e->node->same_body_alias && e->node->same_body->analyzed)));
>
> ? /* A variable should have a size. ?*/
> ? else if (TREE_CODE (e->decl) == VAR_DECL)
> @@ -461,7 +490,7 @@ lto_symtab_resolve_symbols (void **slot)
> ? for (e = (lto_symtab_entry_t) *slot; e; e = e->next)
> ? ? {
> ? ? ? if (TREE_CODE (e->decl) == FUNCTION_DECL)
> - ? ? ? e->node = cgraph_get_node (e->decl);
> + ? ? ? e->node = cgraph_get_node_or_alias (e->decl);
> ? ? ? else if (TREE_CODE (e->decl) == VAR_DECL)
> ? ? ? ?{
> ? ? ? ? ?e->vnode = varpool_get_node (e->decl);
> @@ -751,22 +780,7 @@ lto_symtab_merge_cgraph_nodes_1 (void **
> ? for (e = prevailing->next; e; e = e->next)
> ? ? {
> ? ? ? if (e->node != NULL)
> - ? ? ? {
> - ? ? ? ? if (e->node->decl != e->decl && e->node->same_body)
> - ? ? ? ? ? {
> - ? ? ? ? ? ? struct cgraph_node *alias;
> -
> - ? ? ? ? ? ? for (alias = e->node->same_body; alias; alias = alias->next)
> - ? ? ? ? ? ? ? if (alias->decl == e->decl)
> - ? ? ? ? ? ? ? ? break;
> - ? ? ? ? ? ? if (alias)
> - ? ? ? ? ? ? ? {
> - ? ? ? ? ? ? ? ? cgraph_remove_same_body_alias (alias);
> - ? ? ? ? ? ? ? ? continue;
> - ? ? ? ? ? ? ? }
> - ? ? ? ? ? }
> - ? ? ? ? lto_cgraph_replace_node (e->node, prevailing->node);
> - ? ? ? }
> + ? ? ? lto_cgraph_replace_node (e->node, prevailing->node);
> ? ? ? if (e->vnode != NULL)
> ? ? ? ?lto_varpool_replace_node (e->vnode, prevailing->vnode);
> ? ? }
> Index: cgraph.c
> ===================================================================
> --- cgraph.c ? ?(revision 161847)
> +++ cgraph.c ? ?(working copy)
> @@ -604,6 +604,29 @@ cgraph_add_thunk (tree alias, tree decl,
> ? ?is assigned. ?*/
>
> ?struct cgraph_node *
> +cgraph_get_node_or_alias (tree decl)
> +{
> + ?struct cgraph_node key, *node = NULL, **slot;
> +
> + ?gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
> +
> + ?if (!cgraph_hash)
> + ? ?return NULL;
> +
> + ?key.decl = decl;
> +
> + ?slot = (struct cgraph_node **) htab_find_slot (cgraph_hash, &key,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?NO_INSERT);
> +
> + ?if (slot && *slot)
> + ? ?node = *slot;
> + ?return node;
> +}
> +
> +/* Returns the cgraph node assigned to DECL or NULL if no cgraph node
> + ? is assigned. ?*/
> +
> +struct cgraph_node *
> ?cgraph_get_node (tree decl)
> ?{
> ? struct cgraph_node key, *node = NULL, **slot;
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]