summaryrefslogtreecommitdiff
path: root/gcc/lto
diff options
context:
space:
mode:
authorJan Hubicka <hubicka@ucw.cz>2019-12-08 18:02:30 +0100
committerJan Hubicka <hubicka@gcc.gnu.org>2019-12-08 17:02:30 +0000
commit6d8fd122c4f856e9c6037adc310505f2d65347d9 (patch)
tree8e29349bdc425541a2785219964901a5ba270470 /gcc/lto
parenta8d9d6649e621e6e0d0887bd7f5a6069cb9cfa72 (diff)
Fix overflows in -fprofile-reorder-functions
This patch fixes three sissues with -fprofile-reorder-functions: 1) First is that tp_first_run is stored as 32bit integer while it can easily overflow (and does so during Firefox profiling). 2) Second problem is that flag_profile_functions can not be tested w/o function context. The changes to expand_all_functions makes it to work on mixed units by first outputting all functions w/o -fprofile-reorder-function (or with no profile info) and then outputting in first_run order 3) LTO partitioner was mixing up order by tp_first_run and by order. for no_reorder we definitly want to order via first, while for everything else we want to roder by second. I have also merged duplicated comparators since they are bit fragile into tp_first_run_node_cmp. I originaly started to look into this because of undefined symbols with Firefox PGO builds. These symbols went away with fixing these bug but I am not quite sure how. it is possible that there is another problem in lto_blanced_map but even after reading the noreorder code few times carefuly I did not find it. Other explanation would be that our new qsort with broken comparator due to overflow can actualy remove some entries in the array, but that sounds bit crazy. Bootstrapped/regested x86_64-linux. * cgraph.c (cgraph_node::dump): Make tp_first_run 64bit. * cgraph.h (cgrpah_node): Likewise. (tp_first_run_node_cmp): Deeclare. * cgraphunit.c (node_cmp): Rename to ... (tp_first_run_node_cmp): ... this; export; watch for 64bit overflows; clear tp_first_run for no_reorder and !flag_profile_reorder_functions. (expand_all_functions): Collect tp_first_run and normal functions to two vectors so the other functions remain sorted. Do not check for flag_profile_reorder_functions it is function local flag. * profile.c (compute_value_histograms): Update tp_first_run printing. * lto-partition.c (node_cmp): Turn into simple order comparsions. (varpool_node_cmp): Remove. (add_sorted_nodes): Use node_cmp. (lto_balanced_map): Use tp_first_run_node_cmp. From-SVN: r279093
Diffstat (limited to 'gcc/lto')
-rw-r--r--gcc/lto/ChangeLog8
-rw-r--r--gcc/lto/lto-partition.c45
2 files changed, 15 insertions, 38 deletions
diff --git a/gcc/lto/ChangeLog b/gcc/lto/ChangeLog
index df9ddfa3432..d80b2bd61bf 100644
--- a/gcc/lto/ChangeLog
+++ b/gcc/lto/ChangeLog
@@ -1,5 +1,11 @@
-2019-11-25 Joseph Myers <joseph@codesourcery.com>
+2019-12-07 Jan Hubicka <hubicka@ucw.cz>
+ * lto-partition.c (node_cmp): Turn into simple order comparsions.
+ (varpool_node_cmp): Remove.
+ (add_sorted_nodes): Use node_cmp.
+ (lto_balanced_map): Use tp_first_run_node_cmp.
+
+/bin/bash: :q: command not found
PR c/91985
* lto-lang.c (lto_type_for_mode): Handle decimal floating-point
types being NULL_TREE.
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index 75cfdaabf42..4da76509616 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -372,38 +372,9 @@ lto_max_map (void)
new_partition ("empty");
}
-/* Helper function for qsort; sort nodes by order. noreorder functions must have
- been removed earlier. */
-static int
-node_cmp (const void *pa, const void *pb)
-{
- const struct cgraph_node *a = *(const struct cgraph_node * const *) pa;
- const struct cgraph_node *b = *(const struct cgraph_node * const *) pb;
-
- /* Profile reorder flag enables function reordering based on first execution
- of a function. All functions with profile are placed in ascending
- order at the beginning. */
-
- if (flag_profile_reorder_functions)
- {
- /* Functions with time profile are sorted in ascending order. */
- if (a->tp_first_run && b->tp_first_run)
- return a->tp_first_run != b->tp_first_run
- ? a->tp_first_run - b->tp_first_run
- : a->order - b->order;
-
- /* Functions with time profile are sorted before the functions
- that do not have the profile. */
- if (a->tp_first_run || b->tp_first_run)
- return b->tp_first_run - a->tp_first_run;
- }
-
- return b->order - a->order;
-}
-
/* Helper function for qsort; sort nodes by order. */
static int
-varpool_node_cmp (const void *pa, const void *pb)
+node_cmp (const void *pa, const void *pb)
{
const symtab_node *a = *static_cast<const symtab_node * const *> (pa);
const symtab_node *b = *static_cast<const symtab_node * const *> (pb);
@@ -418,7 +389,7 @@ add_sorted_nodes (vec<symtab_node *> &next_nodes, ltrans_partition partition)
unsigned i;
symtab_node *node;
- next_nodes.qsort (varpool_node_cmp);
+ next_nodes.qsort (node_cmp);
FOR_EACH_VEC_ELT (next_nodes, i, node)
if (!symbol_partitioned_p (node))
add_symbol_to_partition (partition, node);
@@ -537,17 +508,17 @@ lto_balanced_map (int n_lto_partitions, int max_partition_size)
unit tends to import a lot of global trees defined there. We should
get better about minimizing the function bounday, but until that
things works smoother if we order in source order. */
- order.qsort (node_cmp);
+ order.qsort (tp_first_run_node_cmp);
noreorder.qsort (node_cmp);
if (dump_file)
{
for (unsigned i = 0; i < order.length (); i++)
- fprintf (dump_file, "Balanced map symbol order:%s:%u\n",
- order[i]->name (), order[i]->tp_first_run);
+ fprintf (dump_file, "Balanced map symbol order:%s:%" PRId64 "\n",
+ order[i]->name (), (int64_t) order[i]->tp_first_run);
for (unsigned i = 0; i < noreorder.length (); i++)
- fprintf (dump_file, "Balanced map symbol no_reorder:%s:%u\n",
- noreorder[i]->name (), noreorder[i]->tp_first_run);
+ fprintf (dump_file, "Balanced map symbol no_reorder:%s:%" PRId64 "\n",
+ noreorder[i]->name (), (int64_t) noreorder[i]->tp_first_run);
}
/* Collect all variables that should not be reordered. */
@@ -556,7 +527,7 @@ lto_balanced_map (int n_lto_partitions, int max_partition_size)
&& vnode->no_reorder)
varpool_order.safe_push (vnode);
n_varpool_nodes = varpool_order.length ();
- varpool_order.qsort (varpool_node_cmp);
+ varpool_order.qsort (node_cmp);
/* Compute partition size and create the first partition. */
if (param_min_partition_size > max_partition_size)