> 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.
Hi,
this is version I comitted with minor change of using the coverage checksum
because I think some of anonymous namespace functions do get exported with
random seed attached in a very side case.
To answer Martin's question, I am just removing the tp_first_run merging code
becuase
it is done twice - second time in ipa_merge_profiles.
Jakub/Richi: I would like to see this in 4.9 (it is missed optimization compared
to 4.8). Let me know when it is OK to commit it (perhaps minus the lto-symtab.c
change that is not necessary).
* ipa-utils.c (ipa_merge_profiles): Merge profile_id.
* coverage.c (coverage_compute_profile_id): Handle externally visible
symbols.
* lto/lto-symtab.c (lto_cgraph_replace_node): Don't re-merge
tp_first_run.
Index: ipa-utils.c
===================================================================
--- ipa-utils.c (revision 209385)
+++ ipa-utils.c (working copy)
@@ -660,6 +660,14 @@ 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
+ gcc_assert (src->profile_id == dst->profile_id);
+ }
+
if (!dst->count)
return;
if (cgraph_dump_file)
Index: coverage.c
===================================================================
--- coverage.c (revision 209385)
+++ coverage.c (working copy)
@@ -555,18 +555,29 @@ 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))
+ {
+ chksum = coverage_checksum_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: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c (revision 209385)
+++ 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;
-
/* Finally remove the replaced node. */
cgraph_remove_node (node);
}
