Re: [lto] Unify decl and type registration (issue4515186)

[Richard Guenther <rguenther at suse dot de>]

On Fri, 3 Jun 2011, Diego Novillo wrote:

> 
> As discussed in http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00063.html,
> this patch moves decl registration in symbol tables to the LTO front
> end.  It makes type and symbol registration happen at the same time in
> uniquify_nodes.
> 
> Tested with LTO profiledbootstrap on x86_64.  Committed to trunk.
> 
> 
> Diego.
> 
> 
> 2011-06-03  Diego Novillo  <dnovillo@google.com>
> 
> 	* lto-streamer-in.c (get_resolution): Move to lto/lto.c.
> 	(lto_register_var_decl_in_symtab): Likewise.
> 	(lto_register_function_decl_in_symtab): Likewise.
> 	(lto_read_tree): Move VAR_DECL and FUNCTION_DECL registration
> 	logic to lto_read_decls.
> 
> lto/ChangeLog
> 
> 	* lto.c (get_resolution): Move from lto-streamer-in.c.
> 	(lto_register_var_decl_in_symtab): Likewise.
> 	(lto_register_function_decl_in_symtab): Likewise.
> 	(uniquify_nodes): Call lto_register_var_decl and
> 	lto_register_function_decl_in_symtab after reading a new
> 	VAR_DECL or FUNCTION_DECL.
> 
> diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
> index 88966f2..a0c5509 100644
> --- a/gcc/lto-streamer-in.c
> +++ b/gcc/lto-streamer-in.c
> @@ -1533,31 +1533,6 @@ lto_input_constructors_and_inits (struct lto_file_decl_data *file_data,
>  }
>  
>  
> -/* Return the resolution for the decl with index INDEX from DATA_IN. */
> -
> -static enum ld_plugin_symbol_resolution
> -get_resolution (struct data_in *data_in, unsigned index)
> -{
> -  if (data_in->globals_resolution)
> -    {
> -      ld_plugin_symbol_resolution_t ret;
> -      /* We can have references to not emitted functions in
> -	 DECL_FUNCTION_PERSONALITY at least.  So we can and have
> -	 to indeed return LDPR_UNKNOWN in some cases.   */
> -      if (VEC_length (ld_plugin_symbol_resolution_t,
> -		      data_in->globals_resolution) <= index)
> -	return LDPR_UNKNOWN;
> -      ret = VEC_index (ld_plugin_symbol_resolution_t,
> -		       data_in->globals_resolution,
> -		       index);
> -      return ret;
> -    }
> -  else
> -    /* Delay resolution finding until decl merging.  */
> -    return LDPR_UNKNOWN;
> -}
> -
> -
>  /* Unpack all the non-pointer fields of the TS_BASE structure of
>     expression EXPR from bitpack BP.  */
>  
> @@ -2473,117 +2448,6 @@ lto_input_tree_pointers (struct lto_input_block *ib, struct data_in *data_in,
>  }
>  
>  
> -/* Register DECL with the global symbol table and change its
> -   name if necessary to avoid name clashes for static globals across
> -   different files.  */
> -
> -static void
> -lto_register_var_decl_in_symtab (struct data_in *data_in, tree decl)
> -{
> -  tree context;
> -
> -  /* Variable has file scope, not local. Need to ensure static variables
> -     between different files don't clash unexpectedly.  */
> -  if (!TREE_PUBLIC (decl)
> -      && !((context = decl_function_context (decl))
> -	   && auto_var_in_fn_p (decl, context)))
> -    {
> -      /* ??? We normally pre-mangle names before we serialize them
> -	 out.  Here, in lto1, we do not know the language, and
> -	 thus cannot do the mangling again. Instead, we just
> -	 append a suffix to the mangled name.  The resulting name,
> -	 however, is not a properly-formed mangled name, and will
> -	 confuse any attempt to unmangle it.  */
> -      const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
> -      char *label;
> -
> -      ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
> -      SET_DECL_ASSEMBLER_NAME (decl, get_identifier (label));
> -      rest_of_decl_compilation (decl, 1, 0);
> -
> -      VEC_safe_push (tree, gc, lto_global_var_decls, decl);
> -    }
> -
> -  /* If this variable has already been declared, queue the
> -     declaration for merging.  */
> -  if (TREE_PUBLIC (decl))
> -    {
> -      unsigned ix;
> -      if (!lto_streamer_cache_lookup (data_in->reader_cache, decl, &ix))
> -	gcc_unreachable ();
> -      lto_symtab_register_decl (decl, get_resolution (data_in, ix),
> -				data_in->file_data);
> -    }
> -}
> -
> -
> -
> -/* Register DECL with the global symbol table and change its
> -   name if necessary to avoid name clashes for static globals across
> -   different files.  DATA_IN contains descriptors and tables for the
> -   file being read.  */
> -
> -static void
> -lto_register_function_decl_in_symtab (struct data_in *data_in, tree decl)
> -{
> -  /* Need to ensure static entities between different files
> -     don't clash unexpectedly.  */
> -  if (!TREE_PUBLIC (decl))
> -    {
> -      /* We must not use the DECL_ASSEMBLER_NAME macro here, as it
> -	 may set the assembler name where it was previously empty.  */
> -      tree old_assembler_name = decl->decl_with_vis.assembler_name;
> -
> -      /* FIXME lto: We normally pre-mangle names before we serialize
> -	 them out.  Here, in lto1, we do not know the language, and
> -	 thus cannot do the mangling again. Instead, we just append a
> -	 suffix to the mangled name.  The resulting name, however, is
> -	 not a properly-formed mangled name, and will confuse any
> -	 attempt to unmangle it.  */
> -      const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
> -      char *label;
> -
> -      ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
> -      SET_DECL_ASSEMBLER_NAME (decl, get_identifier (label));
> -
> -      /* We may arrive here with the old assembler name not set
> -	 if the function body is not needed, e.g., it has been
> -	 inlined away and does not appear in the cgraph.  */
> -      if (old_assembler_name)
> -	{
> -	  tree new_assembler_name = DECL_ASSEMBLER_NAME (decl);
> -
> -	  /* Make the original assembler name available for later use.
> -	     We may have used it to indicate the section within its
> -	     object file where the function body may be found.
> -	     FIXME lto: Find a better way to maintain the function decl
> -	     to body section mapping so we don't need this hack.  */
> -	  lto_record_renamed_decl (data_in->file_data,
> -				   IDENTIFIER_POINTER (old_assembler_name),
> -				   IDENTIFIER_POINTER (new_assembler_name));
> -
> -	  /* Also register the reverse mapping so that we can find the
> -	     new name given to an existing assembler name (used when
> -	     restoring alias pairs in input_constructors_or_inits.  */
> -	  lto_record_renamed_decl (data_in->file_data,
> -				   IDENTIFIER_POINTER (new_assembler_name),
> -				   IDENTIFIER_POINTER (old_assembler_name));
> -	}
> -    }
> -
> -  /* If this variable has already been declared, queue the
> -     declaration for merging.  */
> -  if (TREE_PUBLIC (decl) && !DECL_ABSTRACT (decl))
> -    {
> -      unsigned ix;
> -      if (!lto_streamer_cache_lookup (data_in->reader_cache, decl, &ix))
> -	gcc_unreachable ();
> -      lto_symtab_register_decl (decl, get_resolution (data_in, ix),
> -				data_in->file_data);
> -    }
> -}
> -
> -
>  /* Read an index IX from input block IB and return the tree node at
>     DATA_IN->FILE_DATA->GLOBALS_INDEX[IX].  */
>  
> @@ -2665,11 +2529,6 @@ lto_read_tree (struct lto_input_block *ib, struct data_in *data_in,
>    if (TREE_CODE (result) == FUNCTION_DECL)
>      gcc_assert (!lto_stream_as_builtin_p (result));
>  
> -  if (TREE_CODE (result) == VAR_DECL)
> -    lto_register_var_decl_in_symtab (data_in, result);
> -  else if (TREE_CODE (result) == FUNCTION_DECL && !DECL_BUILT_IN (result))
> -    lto_register_function_decl_in_symtab (data_in, result);
> -
>    /* end_marker = */ lto_input_1_unsigned (ib);
>  
>  #ifdef LTO_STREAMER_DEBUG
> diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
> index 9d4e2ed..70d5bde 100644
> --- a/gcc/lto/lto.c
> +++ b/gcc/lto/lto.c
> @@ -501,6 +501,141 @@ lto_fixup_types (tree t)
>      }
>  }
>  
> +
> +/* Return the resolution for the decl with index INDEX from DATA_IN. */
> +
> +static enum ld_plugin_symbol_resolution
> +get_resolution (struct data_in *data_in, unsigned index)
> +{
> +  if (data_in->globals_resolution)
> +    {
> +      ld_plugin_symbol_resolution_t ret;
> +      /* We can have references to not emitted functions in
> +	 DECL_FUNCTION_PERSONALITY at least.  So we can and have
> +	 to indeed return LDPR_UNKNOWN in some cases.   */
> +      if (VEC_length (ld_plugin_symbol_resolution_t,
> +		      data_in->globals_resolution) <= index)
> +	return LDPR_UNKNOWN;
> +      ret = VEC_index (ld_plugin_symbol_resolution_t,
> +		       data_in->globals_resolution,
> +		       index);
> +      return ret;
> +    }
> +  else
> +    /* Delay resolution finding until decl merging.  */
> +    return LDPR_UNKNOWN;
> +}
> +
> +
> +/* Register DECL with the global symbol table and change its
> +   name if necessary to avoid name clashes for static globals across
> +   different files.  */
> +
> +static void
> +lto_register_var_decl_in_symtab (struct data_in *data_in, tree decl)
> +{
> +  tree context;
> +
> +  /* Variable has file scope, not local. Need to ensure static variables
> +     between different files don't clash unexpectedly.  */
> +  if (!TREE_PUBLIC (decl)
> +      && !((context = decl_function_context (decl))
> +	   && auto_var_in_fn_p (decl, context)))
> +    {
> +      /* ??? We normally pre-mangle names before we serialize them
> +	 out.  Here, in lto1, we do not know the language, and
> +	 thus cannot do the mangling again. Instead, we just
> +	 append a suffix to the mangled name.  The resulting name,
> +	 however, is not a properly-formed mangled name, and will
> +	 confuse any attempt to unmangle it.  */
> +      const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
> +      char *label;
> +
> +      ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
> +      SET_DECL_ASSEMBLER_NAME (decl, get_identifier (label));
> +      rest_of_decl_compilation (decl, 1, 0);
> +      VEC_safe_push (tree, gc, lto_global_var_decls, decl);
> +    }
> +
> +  /* If this variable has already been declared, queue the
> +     declaration for merging.  */
> +  if (TREE_PUBLIC (decl))
> +    {
> +      unsigned ix;
> +      if (!lto_streamer_cache_lookup (data_in->reader_cache, decl, &ix))
> +	gcc_unreachable ();
> +      lto_symtab_register_decl (decl, get_resolution (data_in, ix),
> +				data_in->file_data);
> +    }
> +}
> +
> +
> +/* Register DECL with the global symbol table and change its
> +   name if necessary to avoid name clashes for static globals across
> +   different files.  DATA_IN contains descriptors and tables for the
> +   file being read.  */
> +
> +static void
> +lto_register_function_decl_in_symtab (struct data_in *data_in, tree decl)
> +{
> +  /* Need to ensure static entities between different files
> +     don't clash unexpectedly.  */
> +  if (!TREE_PUBLIC (decl))
> +    {
> +      /* We must not use the DECL_ASSEMBLER_NAME macro here, as it
> +	 may set the assembler name where it was previously empty.  */
> +      tree old_assembler_name = decl->decl_with_vis.assembler_name;
> +
> +      /* FIXME lto: We normally pre-mangle names before we serialize
> +	 them out.  Here, in lto1, we do not know the language, and
> +	 thus cannot do the mangling again. Instead, we just append a
> +	 suffix to the mangled name.  The resulting name, however, is
> +	 not a properly-formed mangled name, and will confuse any
> +	 attempt to unmangle it.  */
> +      const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
> +      char *label;
> +
> +      ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
> +      SET_DECL_ASSEMBLER_NAME (decl, get_identifier (label));
> +
> +      /* We may arrive here with the old assembler name not set
> +	 if the function body is not needed, e.g., it has been
> +	 inlined away and does not appear in the cgraph.  */
> +      if (old_assembler_name)
> +	{
> +	  tree new_assembler_name = DECL_ASSEMBLER_NAME (decl);
> +
> +	  /* Make the original assembler name available for later use.
> +	     We may have used it to indicate the section within its
> +	     object file where the function body may be found.
> +	     FIXME lto: Find a better way to maintain the function decl
> +	     to body section mapping so we don't need this hack.  */
> +	  lto_record_renamed_decl (data_in->file_data,
> +				   IDENTIFIER_POINTER (old_assembler_name),
> +				   IDENTIFIER_POINTER (new_assembler_name));
> +
> +	  /* Also register the reverse mapping so that we can find the
> +	     new name given to an existing assembler name (used when
> +	     restoring alias pairs in input_constructors_or_inits.  */
> +	  lto_record_renamed_decl (data_in->file_data,
> +				   IDENTIFIER_POINTER (new_assembler_name),
> +				   IDENTIFIER_POINTER (old_assembler_name));
> +	}
> +    }
> +
> +  /* If this variable has already been declared, queue the
> +     declaration for merging.  */
> +  if (TREE_PUBLIC (decl) && !DECL_ABSTRACT (decl))
> +    {
> +      unsigned ix;
> +      if (!lto_streamer_cache_lookup (data_in->reader_cache, decl, &ix))
> +	gcc_unreachable ();
> +      lto_symtab_register_decl (decl, get_resolution (data_in, ix),
> +				data_in->file_data);
> +    }
> +}
> +
> +
>  /* Given a streamer cache structure DATA_IN (holding a sequence of trees
>     for one compilation unit) go over all trees starting at index FROM until the
>     end of the sequence and replace fields of those trees, and the trees
> @@ -513,20 +648,25 @@ uniquify_nodes (struct data_in *data_in, unsigned from)
>    unsigned len = VEC_length (tree, cache->nodes);
>    unsigned i;
>  
> -  /* Go backwards because childs streamed for the first time come
> +  /* Go backwards because children streamed for the first time come
>       as part of their parents, and hence are created after them.  */
>  
> -  /* First register all types in the cache.
> +  /* First register all declarations and types in the cache.
>       This makes sure to have the original structure in the type cycles
>       when registering them and computing hashes.  */
>    for (i = len; i-- > from;)
>      {
>        tree t = VEC_index (tree, cache->nodes, i);
> -      if (!t
> -	  || !TYPE_P (t))
> +
> +      if (t == NULL_TREE)
>  	continue;
>  
> -      gimple_register_type (t);
> +      if (TREE_CODE (t) == VAR_DECL)
> +	lto_register_var_decl_in_symtab (data_in, t);
> +      else if (TREE_CODE (t) == FUNCTION_DECL && !DECL_BUILT_IN (t))
> +	lto_register_function_decl_in_symtab (data_in, t);
> +      else if (TYPE_P (t))
> +	gimple_register_type (t);
>      }

I'd have it done in the loop that computes canonical types, at this
place we do not gain the advantage that the decl register functions
get completely fixed up trees.

Maybe you can move it in a followup.

Thanks,
Richard.