| Message ID | 20100705195535.GB12132@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
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: 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;