Fix indirect call profiling for COMDAT symbols

Hi,
while looking into firefox profiles, I noticed that we miss devirtualizations
to comdat symbols, because we manage to get different profile_id in each
unit.  This is easily fixed by the following patch that makes profiled_id
to by crc32 of the symbol name in this case.

Bootstrapped/regtested x86_64-linux, tested with firefox, will
commit it tomorrow.

	* coverage.c (coverage_compute_profile_id): Make stable for
	global symbols
	* ipa-utils.c (ipa_merge_profiles): Merge profile_id.
	* lto/lto-symtab.c (lto_cgraph_replace_node): Don't re-merge
	tp_first_run.

On 04/11/2014 08:00 AM, Jan Hubicka wrote:
> Hi,
> while looking into firefox profiles, I noticed that we miss devirtualizations
> to comdat symbols, because we manage to get different profile_id in each
> unit.  This is easily fixed by the following patch that makes profiled_id
> to by crc32 of the symbol name in this case.
>
> Bootstrapped/regtested x86_64-linux, tested with firefox, will
> commit it tomorrow.
>
> 	* coverage.c (coverage_compute_profile_id): Make stable for
> 	global symbols
> 	* ipa-utils.c (ipa_merge_profiles): Merge profile_id.
> 	* lto/lto-symtab.c (lto_cgraph_replace_node): Don't re-merge
> 	tp_first_run.
> Index: coverage.c
> ===================================================================
> --- coverage.c	(revision 209170)
> +++ coverage.c	(working copy)
> @@ -555,18 +555,31 @@ coverage_compute_lineno_checksum (void)
>   unsigned
>   coverage_compute_profile_id (struct cgraph_node *n)
>   {
> -  expanded_location xloc
> -    = expand_location (DECL_SOURCE_LOCATION (n->decl));
> -  unsigned chksum = xloc.line;
> +  unsigned chksum;
>   
> -  chksum = coverage_checksum_string (chksum, xloc.file);
> -  chksum = coverage_checksum_string
> -    (chksum, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (n->decl)));
> -  if (first_global_object_name)
> -    chksum = coverage_checksum_string
> -      (chksum, first_global_object_name);
> -  chksum = coverage_checksum_string
> -    (chksum, aux_base_name);
> +  /* Externally visible symbols have unique name.  */
> +  if (TREE_PUBLIC (n->decl) || DECL_EXTERNAL (n->decl))
> +    {
> +      /* Do not use coverage_checksum_string here; we really want unique
> +	 symbol name id.  */
> +      chksum = crc32_string
> +	(0, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (n->decl)));
> +    }
> +  else
> +    {
> +      expanded_location xloc
> +	= expand_location (DECL_SOURCE_LOCATION (n->decl));
> +
> +      chksum = xloc.line;
> +      chksum = coverage_checksum_string (chksum, xloc.file);
> +      chksum = coverage_checksum_string
> +	(chksum, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (n->decl)));
> +      if (first_global_object_name)
> +	chksum = coverage_checksum_string
> +	  (chksum, first_global_object_name);
> +      chksum = coverage_checksum_string
> +	(chksum, aux_base_name);
> +    }
>   
>     /* Non-negative integers are hopefully small enough to fit in all targets.  */
>     return chksum & 0x7fffffff;
> Index: ipa-utils.c
> ===================================================================
> --- ipa-utils.c	(revision 209170)
> +++ ipa-utils.c	(working copy)
> @@ -660,6 +660,21 @@ ipa_merge_profiles (struct cgraph_node *
>     if (dst->tp_first_run > src->tp_first_run && src->tp_first_run)
>       dst->tp_first_run = src->tp_first_run;
>   
> +  if (src->profile_id)
> +    {
> +      if (!dst->profile_id)
> +	dst->profile_id = src->profile_id;
> +      else
> +	{
> +	 if (src->profile_id != dst->profile_id)
> +	   {
> +	     dump_cgraph_node (stderr, src);
> +	     dump_cgraph_node (stderr, dst);
> +	   }
> +	 gcc_assert (src->profile_id == dst->profile_id);
> +	}
> +    }
> +
>     if (!dst->count)
>       return;
>     if (cgraph_dump_file)
> Index: lto/lto-symtab.c
> ===================================================================
> --- lto/lto-symtab.c	(revision 209170)
> +++ lto/lto-symtab.c	(working copy)
> @@ -91,12 +91,6 @@ lto_cgraph_replace_node (struct cgraph_n
>     if (node->decl != prevailing_node->decl)
>       cgraph_release_function_body (node);
>   
> -  /* Time profile merging */
> -  if (node->tp_first_run)
> -    prevailing_node->tp_first_run = prevailing_node->tp_first_run ?
> -      MIN (prevailing_node->tp_first_run, node->tp_first_run) :
> -      node->tp_first_run;
> -

Hello Honza,
    I just want to ask if this time profile merging is not necessary any 
more?

Martin
>     /* Finally remove the replaced node.  */
>     cgraph_remove_node (node);
>   }