Hi, a bit late, but I do have two tiny comments nevertheless...
On Mon, Apr 16, 2012 at 06:09:40PM +0200, Jan Hubicka wrote: > Hi, > this patch moves cgraph/varpool hashes into symbol table hashes, so the > symbol table is actually almost a symbol table ;) > Work done. > > Bootstrapped/regtested x86_64-linux. Will commit it after bit of more > testing - the assembler name handling is slipperly wrt aliases. > > Honza > > * cgraph.c (cgraph_hash, assembler_name_hash): Remove. > (hash_node, eq_node): Remove. > (cgraph_create_node): Do not handle hashtable. > (cgraph_get_node): Remove. > (cgraph_insert_node_to_hashtable): Remove. > (hash_node_by_assembler_name): Remove. > (eq_assembler_name): Remove. > (cgraph_node_for_asm): Rewrite. > (cgraph_find_replacement_node): Break out from ... > (cgraph_remove_node): ... here; do not maintain hashtables. > (change_decl_assembler_name): Remove. > (cgraph_clone_node): Do not maintain hashtables. > * cgraph.h (const_symtab_node): New typedef. > (cgraph_insert_node_to_hashtable): Remove. > (symtab_get_node, symtab_node_for_asm, > symtab_insert_node_to_hashtable): Declare. > (cgraph_find_replacement_node): Declare. > (cgraph_get_node, varpool_get_node): Turn into inlines. > (cgraph, varpool): Work sanely on NULL pointers. > (FOR_EACH_SYMBOL): New walker. > * ipa-inline-transform.c (save_inline_function_body): Use > symtab_insert_node_to_hashtable. > * symtab.c: Include ggc.h and diagnostics.h > (symtab_hash, assembler_name_hash): New static vars; > (hash_node, eq_node, hash_node_by_assembler_name, > eq_assembler_name): New. > (symtab_register_node): Update hashtables. > (symtab_insert_node_to_hashtable): New. > (symtab_unregister_node): Update hashtables. > (symtab_get_node): New. > (symtab_node_for_asm): New. > (change_decl_assembler_name): New. > * Makefile.in (symtab.o): Needs GTY. > * varpool.c (varpool_hash): Remove. > (hash_varpool_node, eq_varpool_node, varpool_get_node): Remove. > (varpool_node): Rewrite using varpool_get_node. > (varpool_remove_node): DO not maintain hashtables. > (varpool_node_for_asm); Rewrite. > Index: cgraph.c > =================================================================== > *** cgraph.c (revision 186496) > --- cgraph.c (working copy) ... > *************** cgraph_create_node_1 (void) > *** 479,520 **** > struct cgraph_node * > cgraph_create_node (tree decl) > { > ! struct cgraph_node key, *node, **slot; > ! > gcc_assert (TREE_CODE (decl) == FUNCTION_DECL); > > - if (!cgraph_hash) > - cgraph_hash = htab_create_ggc (10, hash_node, eq_node, NULL); > - > - key.symbol.decl = decl; > - slot = (struct cgraph_node **) htab_find_slot (cgraph_hash, &key, INSERT); > - gcc_assert (!*slot); Here we used to assert that we do not already have that declaration in the hash... > Index: symtab.c > =================================================================== > *** symtab.c (revision 186496) > --- symtab.c (working copy) > *************** symtab_node symtab_nodes; > *** 35,65 **** > --- 42,166 ---- > them, to support -fno-toplevel-reorder. */ > int symtab_order; > > + /* Returns a hash code for P. */ > + > + static hashval_t > + hash_node (const void *p) > + { > + const_symtab_node n = (const_symtab_node ) p; > + return (hashval_t) DECL_UID (n->symbol.decl); > + } > + > + > + /* Returns nonzero if P1 and P2 are equal. */ > + > + static int > + eq_node (const void *p1, const void *p2) > + { > + const_symtab_node n1 = (const_symtab_node) p1; > + const_symtab_node n2 = (const_symtab_node) p2; > + return DECL_UID (n1->symbol.decl) == DECL_UID (n2->symbol.decl); > + } > + > + /* Returns a hash code for P. */ > + > + static hashval_t > + hash_node_by_assembler_name (const void *p) > + { > + const_symtab_node n = (const_symtab_node) p; > + return (hashval_t) decl_assembler_name_hash (DECL_ASSEMBLER_NAME > (n->symbol.decl)); > + } > + > + /* Returns nonzero if P1 and P2 are equal. */ > + > + static int > + eq_assembler_name (const void *p1, const void *p2) > + { > + const_symtab_node n1 = (const_symtab_node) p1; > + const_tree name = (const_tree)p2; > + return (decl_assembler_name_equal (n1->symbol.decl, name)); > + } > + > + > /* Add node into symbol table. This function is not used directly, but via > cgraph/varpool node creation routines. */ > > void > symtab_register_node (symtab_node node) > { > + struct symtab_node_base key; > + symtab_node *slot; > + > node->symbol.next = symtab_nodes; > node->symbol.previous = NULL; > if (symtab_nodes) > symtab_nodes->symbol.previous = node; > symtab_nodes = node; > > + if (!symtab_hash) > + symtab_hash = htab_create_ggc (10, hash_node, eq_node, NULL); > + key.decl = node->symbol.decl; > + slot = (symtab_node *) htab_find_slot (symtab_hash, &key, INSERT); > + if (*slot == NULL) > + *slot = node; > + ...yet here we don't. Shouldn't we? ... > *************** symtab_remove_node (symtab_node node) > *** 95,97 **** > --- 250,341 ---- > else if (symtab_variable_p (node)) > varpool_remove_node (varpool (node)); > } > + > + /* Return the cgraph node that has ASMNAME for its DECL_ASSEMBLER_NAME. > + Return NULL if there's no such node. */ symtab node :-) Otherwise, these are all really very nice simplifications. Martin