Patchwork Make symtab ready for duplicated symtab nodes during streaming

login
register
mail settings
Submitter Jan Hubicka
Date June 12, 2013, 9:17 a.m.
Message ID <20130612091739.GA24302@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/250716/
State New
Headers show

Comments

Jan Hubicka - June 12, 2013, 9:17 a.m.
Hi,
this patch is basically preparation for Richard's new tree merging patch.
It allows merging multiple instances of same variable and function decls
prior symtab streaming by forcingly creating duplicate symtab entries as needed.

There are two issues. First symtab maintain hash mapping declarations to symbols
that is not ready for this one to many mapping. This is handled by re-populating it 
in lto-symtab after merging.  I have followup patch to avoid its early construction
completely saving some extra compile time during LTO.

Second is way we handle resolutions, that are currently recorded into resoution_map
that maps decl to particular resolution. It needs to be updated to be input
file sensitive that is done by creating separate resolution maps for every file.
It should also make things slightly faster since all handling is per-file
and thus we should get better locality.

Bootstrapped/regtested x86_64-linux and tested on Firefox build with Richard's
patch. Comitted.

Honza

	* lto-symtab.c (lto_symtab_merge_symbols): Populate symtab hashtable.
	* cgraph.h (varpool_create_empty_node): Declare.
	* lto-cgraph.c (input_node, input_varpool_node): Forcingly create
	duplicated nodes.
	* symtab.c (symtab_unregister_node): Be lax about missin entries
	in node hash.
	(symtab_get_node): Update comment.
	* varpool.c (varpool_create_empty_node): Break out from ...
	(varpool_node_for_decl): ... here.
	* lto-streamer.h (lto_file_decl_data): Add RESOLUTION_MAP.

	* lto.c (register_resolution): Take lto_file_data argument.
	(lto_register_var_decl_in_symtab,
	lto_register_function_decl_in_symtab): Update.
	(read_cgraph_and_symbols): Update resolution_map handling.

Patch

Index: lto-symtab.c
===================================================================
--- lto-symtab.c	(revision 199988)
+++ lto-symtab.c	(working copy)
@@ -573,16 +573,21 @@  lto_symtab_merge_symbols (void)
     {
       symtab_initialize_asm_name_hash ();
 
-      /* Do the actual merging.  */
+      /* Do the actual merging.  
+         At this point we invalidate hash translating decls into symtab nodes
+	 because after removing one of duplicate decls the hash is not correcly
+	 updated to the ohter dupliate.  */
       FOR_EACH_SYMBOL (node)
 	if (lto_symtab_symbol_p (node)
 	    && node->symbol.next_sharing_asm_name
 	    && !node->symbol.previous_sharing_asm_name)
 	  lto_symtab_merge_symbols_1 (node);
 
-      /* Resolve weakref aliases whose target are now in the compilation unit.  */
+      /* Resolve weakref aliases whose target are now in the compilation unit.  
+	 also re-populate the hash translating decls into symtab nodes*/
       FOR_EACH_SYMBOL (node)
 	{
+	  cgraph_node *cnode;
 	  if (!node->symbol.analyzed && node->symbol.alias_target)
 	    {
 	      symtab_node tgt = symtab_node_for_asm (node->symbol.alias_target);
@@ -591,6 +596,10 @@  lto_symtab_merge_symbols (void)
 		symtab_resolve_alias (node, tgt);
 	    }
 	  node->symbol.aux = NULL;
+	  if (!(cnode = dyn_cast <cgraph_node> (node))
+	      || !cnode->clone_of
+	      || cnode->clone_of->symbol.decl != cnode->symbol.decl)
+	    symtab_insert_node_to_hashtable ((symtab_node)node);
 	}
     }
 }
Index: cgraph.h
===================================================================
--- cgraph.h	(revision 199988)
+++ cgraph.h	(working copy)
@@ -773,6 +773,7 @@  bool cgraph_maybe_hot_edge_p (struct cgr
 bool cgraph_optimize_for_size_p (struct cgraph_node *);
 
 /* In varpool.c  */
+struct varpool_node *varpool_create_empty_node (void);
 struct varpool_node *varpool_node_for_decl (tree);
 struct varpool_node *varpool_node_for_asm (tree asmname);
 void varpool_mark_needed_node (struct varpool_node *);
Index: lto-cgraph.c
===================================================================
--- lto-cgraph.c	(revision 199988)
+++ lto-cgraph.c	(working copy)
@@ -959,7 +959,14 @@  input_node (struct lto_file_decl_data *f
 				vNULL, false);
     }
   else
-    node = cgraph_get_create_node (fn_decl);
+    {
+      /* Declaration of functions can be already merged with a declaration
+	 from other input file.  We keep cgraph unmerged until after streaming
+	 of ipa passes is done.  Alays forcingly create a fresh node.  */
+      node = cgraph_create_empty_node ();
+      node->symbol.decl = fn_decl;
+      symtab_register_node ((symtab_node)node);
+    }
 
   node->symbol.order = order;
   if (order >= symtab_order)
@@ -1035,7 +1042,14 @@  input_varpool_node (struct lto_file_decl
   order = streamer_read_hwi (ib) + order_base;
   decl_index = streamer_read_uhwi (ib);
   var_decl = lto_file_decl_data_get_var_decl (file_data, decl_index);
-  node = varpool_node_for_decl (var_decl);
+
+  /* Declaration of functions can be already merged with a declaration
+     from other input file.  We keep cgraph unmerged until after streaming
+     of ipa passes is done.  Alays forcingly create a fresh node.  */
+  node = varpool_create_empty_node ();
+  node->symbol.decl = var_decl;
+  symtab_register_node ((symtab_node)node);
+
   node->symbol.order = order;
   if (order >= symtab_order)
     symtab_order = order + 1;
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 199988)
+++ lto/lto.c	(working copy)
@@ -1726,18 +1726,16 @@  get_resolution (struct data_in *data_in,
     return LDPR_UNKNOWN;
 }
 
-/* Map assigning declarations their resolutions.  */
-static pointer_map_t *resolution_map;
-
 /* We need to record resolutions until symbol table is read.  */
 static void
-register_resolution (tree decl, enum ld_plugin_symbol_resolution resolution)
+register_resolution (struct lto_file_decl_data *file_data, tree decl,
+		     enum ld_plugin_symbol_resolution resolution)
 {
   if (resolution == LDPR_UNKNOWN)
     return;
-  if (!resolution_map)
-    resolution_map = pointer_map_create ();
-  *pointer_map_insert (resolution_map, decl) = (void *)(size_t)resolution;
+  if (!file_data->resolution_map)
+    file_data->resolution_map = pointer_map_create ();
+  *pointer_map_insert (file_data->resolution_map, decl) = (void *)(size_t)resolution;
 }
 
 /* Register DECL with the global symbol table and change its
@@ -1764,7 +1762,7 @@  lto_register_var_decl_in_symtab (struct
       unsigned ix;
       if (!streamer_tree_cache_lookup (data_in->reader_cache, decl, &ix))
 	gcc_unreachable ();
-      register_resolution (decl, get_resolution (data_in, ix));
+      register_resolution (data_in->file_data, decl, get_resolution (data_in, ix));
     }
 }
 
@@ -1784,7 +1782,7 @@  lto_register_function_decl_in_symtab (st
       unsigned ix;
       if (!streamer_tree_cache_lookup (data_in->reader_cache, decl, &ix))
 	gcc_unreachable ();
-      register_resolution (decl, get_resolution (data_in, ix));
+      register_resolution (data_in->file_data, decl, get_resolution (data_in, ix));
     }
 }
 
@@ -2865,6 +2863,8 @@  read_cgraph_and_symbols (unsigned nfiles
   struct cgraph_node *node;
   int count = 0;
   struct lto_file_decl_data **decl_data;
+  void **res;
+  symtab_node snode;
 
   init_cgraph ();
 
@@ -2971,21 +2971,21 @@  read_cgraph_and_symbols (unsigned nfiles
   input_symtab ();
 
   /* Store resolutions into the symbol table.  */
-  if (resolution_map)
-    {
-      void **res;
-      symtab_node snode;
 
-      FOR_EACH_SYMBOL (snode)
-	if (symtab_real_symbol_p (snode)
-	    && (res = pointer_map_contains (resolution_map,
-				            snode->symbol.decl)))
-	  snode->symbol.resolution
-	    = (enum ld_plugin_symbol_resolution)(size_t)*res;
-
-      pointer_map_destroy (resolution_map);
-      resolution_map = NULL;
-    }
+  FOR_EACH_SYMBOL (snode)
+    if (symtab_real_symbol_p (snode)
+	&& snode->symbol.lto_file_data
+	&& snode->symbol.lto_file_data->resolution_map
+	&& (res = pointer_map_contains (snode->symbol.lto_file_data->resolution_map,
+					snode->symbol.decl)))
+      snode->symbol.resolution
+	= (enum ld_plugin_symbol_resolution)(size_t)*res;
+  for (i = 0; all_file_decl_data[i]; i++)
+    if (all_file_decl_data[i]->resolution_map)
+      {
+        pointer_map_destroy (all_file_decl_data[i]->resolution_map);
+        all_file_decl_data[i]->resolution_map = NULL;
+      }
   
   timevar_pop (TV_IPA_LTO_CGRAPH_IO);
 
Index: symtab.c
===================================================================
--- symtab.c	(revision 199988)
+++ symtab.c	(working copy)
@@ -269,7 +269,11 @@  symtab_unregister_node (symtab_node node
   node->symbol.previous = NULL;
 
   slot = htab_find_slot (symtab_hash, node, NO_INSERT);
-  if (*slot == node)
+
+  /* During LTO symtab merging we temporarily corrupt decl to symtab node
+     hash.  */
+  gcc_assert ((slot && *slot) || in_lto_p);
+  if (slot && *slot && *slot == node)
     {
       symtab_node replacement_node = NULL;
       if (cgraph_node *cnode = dyn_cast <cgraph_node> (node))
@@ -291,10 +295,14 @@  symtab_get_node (const_tree decl)
   symtab_node *slot;
   struct symtab_node_base key;
 
+#ifdef ENABLE_CHECKING
+  /* Check that we are called for sane type of object - functions
+     and static or external variables.  */
   gcc_checking_assert (TREE_CODE (decl) == FUNCTION_DECL
 		       || (TREE_CODE (decl) == VAR_DECL
 			   && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)
 			       || in_lto_p)));
+#endif
 
   if (!symtab_hash)
     return NULL;
Index: varpool.c
===================================================================
--- varpool.c	(revision 199988)
+++ varpool.c	(working copy)
@@ -36,18 +36,26 @@  along with GCC; see the file COPYING3.
 #include "tree-flow.h"
 #include "flags.h"
 
+/* Allocate new callgraph node and insert it into basic data structures.  */
+
+struct varpool_node *
+varpool_create_empty_node (void)
+{   
+  struct varpool_node *node = ggc_alloc_cleared_varpool_node ();
+  node->symbol.type = SYMTAB_VARIABLE;
+  return node;
+}   
+
 /* Return varpool node assigned to DECL.  Create new one when needed.  */
 struct varpool_node *
 varpool_node_for_decl (tree decl)
 {
   struct varpool_node *node = varpool_get_node (decl);
-  gcc_assert (TREE_CODE (decl) == VAR_DECL
-	      && (TREE_STATIC (decl) || DECL_EXTERNAL (decl) || in_lto_p));
+  gcc_checking_assert (TREE_CODE (decl) == VAR_DECL);
   if (node)
     return node;
 
-  node = ggc_alloc_cleared_varpool_node ();
-  node->symbol.type = SYMTAB_VARIABLE;
+  node = varpool_create_empty_node ();
   node->symbol.decl = decl;
   symtab_register_node ((symtab_node)node);
   return node;
Index: lto-streamer.h
===================================================================
--- lto-streamer.h	(revision 199988)
+++ lto-streamer.h	(working copy)
@@ -569,6 +569,9 @@  struct GTY(()) lto_file_decl_data
   unsigned max_index;
 
   struct gcov_ctr_summary GTY((skip)) profile_info;
+
+  /* Map assigning declarations their resolutions.  */
+  pointer_map_t * GTY((skip)) resolution_map;
 };
 
 typedef struct lto_file_decl_data *lto_file_decl_data_ptr;