Hi.
Attempt of the patch is to remove all histograms right after
"profile_estimate" pass. Then nobody should use them. That
will simplify code we'll not need verification and currently
we leaked some histograms till the end of compilation.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Martin
gcc/ChangeLog:
2018-07-31 Martin Liska <[email protected]>
* auto-profile.c (afdo_indirect_call): Do not remove
histograms.
* cfgexpand.c (pass_expand::execute): Likewise.
* cgraph.c (release_function_body): Likewise.
* gimple-iterator.c (gsi_replace): Likewise.
(gsi_remove): Likewise.
* ipa-profile.c (ipa_profile_generate_summary):
Release all histograms.
* tree-cfg.c (verify_gimple_in_cfg): Do not
verify histograms.
(move_block_to_fn): Do not remove histograms.
* value-prof.c (gimple_remove_histogram_value):
Remove.
(gimple_remove_stmt_histograms): Likewise.
(visit_hist): Likewise.
(verify_histograms): Likewise.
(gimple_divmod_fixed_value_transform): Do not
remove histograms.
(gimple_mod_pow2_value_transform): Likewise.
(gimple_mod_subtract_transform): Likewise.
(gimple_ic_transform): Likewise.
(gimple_stringops_transform): Likewise.
(stringop_block_profile): Likewise.
(gimple_find_values_to_profile): Do not produce
extra dump file output.
* value-prof.h: Remove declarations.
gcc/testsuite/ChangeLog:
2018-07-31 Martin Liska <[email protected]>
* gcc.dg/tree-prof/val-prof-6.c: Do not scan histograms
in optimized dump, it's already removed in that pass.
---
gcc/auto-profile.c | 1 -
gcc/cfgexpand.c | 1 -
gcc/cgraph.c | 2 -
gcc/gimple-iterator.c | 2 -
gcc/ipa-profile.c | 6 +-
gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c | 6 +-
gcc/tree-cfg.c | 2 -
gcc/value-prof.c | 127 +-------------------
gcc/value-prof.h | 3 -
9 files changed, 11 insertions(+), 139 deletions(-)
diff --git a/gcc/auto-profile.c b/gcc/auto-profile.c
index 197fa10e08c..c3b5083f034 100644
--- a/gcc/auto-profile.c
+++ b/gcc/auto-profile.c
@@ -1063,7 +1063,6 @@ afdo_indirect_call (gimple_stmt_iterator *gsi, const icall_target_map &map,
= indirect_edge->make_speculative (direct_call,
profile_count::uninitialized ());
new_edge->redirect_call_stmt_to_callee ();
- gimple_remove_histogram_value (cfun, stmt, hist);
inline_call (new_edge, true, NULL, NULL, false);
}
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index d6e3c382085..1dda8e29f99 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -6397,7 +6397,6 @@ pass_expand::execute (function *fun)
/* Expansion is used by optimization passes too, set maybe_hot_insn_p
conservatively to true until they are all profile aware. */
delete lab_rtx_for_bb;
- free_histograms (fun);
construct_exit_block ();
insn_locations_finalize ();
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index d19f1aacab8..c88029c4631 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1728,8 +1728,6 @@ release_function_body (tree decl)
clear_edges (fn);
fn->cfg = NULL;
}
- if (fn->value_histograms)
- free_histograms (fn);
gimple_set_body (decl, NULL);
/* Struct function hangs a lot of data that would leak if we didn't
removed all pointers to it. */
diff --git a/gcc/gimple-iterator.c b/gcc/gimple-iterator.c
index c0131f3654c..61011f37360 100644
--- a/gcc/gimple-iterator.c
+++ b/gcc/gimple-iterator.c
@@ -445,7 +445,6 @@ gsi_replace (gimple_stmt_iterator *gsi, gimple *stmt, bool update_eh_info)
/* Free all the data flow information for ORIG_STMT. */
gimple_set_bb (orig_stmt, NULL);
- gimple_remove_stmt_histograms (cfun, orig_stmt);
delink_stmt_imm_use (orig_stmt);
gsi_set_stmt (gsi, stmt);
@@ -573,7 +572,6 @@ gsi_remove (gimple_stmt_iterator *i, bool remove_permanently)
close. */
cfun->debug_marker_count--;
require_eh_edge_purge = remove_stmt_from_eh_lp (stmt);
- gimple_remove_stmt_histograms (cfun, stmt);
}
/* Update the iterator and re-wire the links in I->SEQ. */
diff --git a/gcc/ipa-profile.c b/gcc/ipa-profile.c
index f921d1bb6f4..4aa73917274 100644
--- a/gcc/ipa-profile.c
+++ b/gcc/ipa-profile.c
@@ -216,8 +216,6 @@ ipa_profile_generate_summary (void)
e->indirect_info->common_target_probability = REG_BR_PROB_BASE;
}
}
- gimple_remove_histogram_value (DECL_STRUCT_FUNCTION (node->decl),
- stmt, h);
}
}
time += estimate_num_insns (stmt, &eni_time_weights);
@@ -228,6 +226,10 @@ ipa_profile_generate_summary (void)
time, size);
}
histogram.qsort (cmp_counts);
+
+ /* Remove all histograms for all functions. */
+ FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
+ free_histograms (DECL_STRUCT_FUNCTION (node->decl));
}
/* Serialize the ipa info for lto. */
diff --git a/gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c b/gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c
index 597efa6ad70..db87af9f696 100644
--- a/gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c
+++ b/gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-options "-O2 -fdump-ipa-profile-details" } */
char a[1000];
char b[1000];
int size=1000;
@@ -16,5 +16,5 @@ main()
return 0;
}
/* autofdo does not do value profiling so far */
-/* { dg-final-use-not-autofdo { scan-tree-dump "Average value sum:499500" "optimized"} } */
-/* { dg-final-use-not-autofdo { scan-tree-dump "IOR value" "optimized"} } */
+/* { dg-final-use-not-autofdo { scan-tree-dump "Average value sum:499500" "profile"} } */
+/* { dg-final-use-not-autofdo { scan-tree-dump "IOR value" "profile"} } */
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 14d66b7a728..da6fd558437 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -5424,7 +5424,6 @@ verify_gimple_in_cfg (struct function *fn, bool verify_nothrow)
if (err || eh_error_found)
internal_error ("verify_gimple failed");
- verify_histograms ();
timevar_pop (TV_TREE_STMT_VERIFY);
}
@@ -7106,7 +7105,6 @@ move_block_to_fn (struct function *dest_cfun, basic_block bb,
remove_stmt_from_eh_lp_fn (cfun, stmt);
gimple_duplicate_stmt_histograms (dest_cfun, stmt, cfun, stmt);
- gimple_remove_stmt_histograms (cfun, stmt);
/* We cannot leave any operands allocated from the operand caches of
the current function. */
diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index a7c4be7a7d8..416eea18ae1 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -183,29 +183,6 @@ gimple_add_histogram_value (struct function *fun, gimple *stmt,
hist->fun = fun;
}
-/* Remove histogram HIST from STMT's histogram list. */
-
-void
-gimple_remove_histogram_value (struct function *fun, gimple *stmt,
- histogram_value hist)
-{
- histogram_value hist2 = gimple_histogram_value (fun, stmt);
- if (hist == hist2)
- {
- set_histogram_value (fun, stmt, hist->hvalue.next);
- }
- else
- {
- while (hist2->hvalue.next != hist)
- hist2 = hist2->hvalue.next;
- hist2->hvalue.next = hist->hvalue.next;
- }
- free (hist->hvalue.counters);
- if (flag_checking)
- memset (hist, 0xab, sizeof (*hist));
- free (hist);
-}
-
/* Lookup histogram of type TYPE in the STMT. */
histogram_value
@@ -446,16 +423,6 @@ dump_histograms_for_stmt (struct function *fun, FILE *dump_file, gimple *stmt)
dump_histogram_value (dump_file, hist);
}
-/* Remove all histograms associated with STMT. */
-
-void
-gimple_remove_stmt_histograms (struct function *fun, gimple *stmt)
-{
- histogram_value val;
- while ((val = gimple_histogram_value (fun, stmt)) != NULL)
- gimple_remove_histogram_value (fun, stmt, val);
-}
-
/* Duplicate all histograms associates with OSTMT to STMT. */
void
@@ -492,66 +459,6 @@ gimple_move_stmt_histograms (struct function *fun, gimple *stmt, gimple *ostmt)
}
}
-static bool error_found = false;
-
-/* Helper function for verify_histograms. For each histogram reachable via htab
- walk verify that it was reached via statement walk. */
-
-static int
-visit_hist (void **slot, void *data)
-{
- hash_set<histogram_value> *visited = (hash_set<histogram_value> *) data;
- histogram_value hist = *(histogram_value *) slot;
-
- if (!visited->contains (hist)
- && hist->type != HIST_TYPE_TIME_PROFILE)
- {
- error ("dead histogram");
- dump_histogram_value (stderr, hist);
- debug_gimple_stmt (hist->hvalue.stmt);
- error_found = true;
- }
- return 1;
-}
-
-/* Verify sanity of the histograms. */
-
-DEBUG_FUNCTION void
-verify_histograms (void)
-{
- basic_block bb;
- gimple_stmt_iterator gsi;
- histogram_value hist;
-
- error_found = false;
- hash_set<histogram_value> visited_hists;
- FOR_EACH_BB_FN (bb, cfun)
- for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
- {
- gimple *stmt = gsi_stmt (gsi);
-
- for (hist = gimple_histogram_value (cfun, stmt); hist;
- hist = hist->hvalue.next)
- {
- if (hist->hvalue.stmt != stmt)
- {
- error ("Histogram value statement does not correspond to "
- "the statement it is associated with");
- debug_gimple_stmt (stmt);
- dump_histogram_value (stderr, hist);
- error_found = true;
- }
- visited_hists.add (hist);
- }
- }
- if (VALUE_HISTOGRAMS (cfun))
- htab_traverse (VALUE_HISTOGRAMS (cfun), visit_hist, &visited_hists);
- if (error_found)
- internal_error ("verify_histograms failed");
-}
-
-/* Helper function for verify_histograms. For each histogram reachable via htab
- walk verify that it was reached via statement walk. */
static int
free_hist (void **slot, void *data ATTRIBUTE_UNUSED)
@@ -787,7 +694,6 @@ gimple_divmod_fixed_value_transform (gimple_stmt_iterator *si)
val = histogram->hvalue.counters[0];
count = histogram->hvalue.counters[1];
all = histogram->hvalue.counters[2];
- gimple_remove_histogram_value (cfun, stmt, histogram);
/* We require that count is at least half of all; this means
that for the transformation to fire the value must be constant
@@ -948,8 +854,6 @@ gimple_mod_pow2_value_transform (gimple_stmt_iterator *si)
wrong_values = histogram->hvalue.counters[0];
count = histogram->hvalue.counters[1];
- gimple_remove_histogram_value (cfun, stmt, histogram);
-
/* We require that we hit a power of 2 at least half of all evaluations. */
if (simple_cst_equal (gimple_assign_rhs2 (stmt), value) != 1
|| count < wrong_values
@@ -1126,10 +1030,7 @@ gimple_mod_subtract_transform (gimple_stmt_iterator *si)
/* Compute probability of taking the optimal path. */
if (check_counter (stmt, "interval", &count1, &all, gimple_bb (stmt)->count))
- {
- gimple_remove_histogram_value (cfun, stmt, histogram);
- return false;
- }
+ return false;
if (flag_profile_correction && count1 + count2 > all)
all = count1 + count2;
@@ -1149,7 +1050,6 @@ gimple_mod_subtract_transform (gimple_stmt_iterator *si)
|| optimize_bb_for_size_p (gimple_bb (stmt)))
return false;
- gimple_remove_histogram_value (cfun, stmt, histogram);
if (dump_file)
{
fprintf (dump_file, "Mod subtract transformation on insn ");
@@ -1464,10 +1364,7 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
if (check_counter (stmt, "ic", &all, &bb_all, gimple_bb (stmt)->count)
|| check_counter (stmt, "ic", &count, &all,
profile_count::from_gcov_type (all)))
- {
- gimple_remove_histogram_value (cfun, stmt, histogram);
- return false;
- }
+ return false;
if (4 * count <= 3 * all)
return false;
@@ -1499,7 +1396,6 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
fprintf (dump_file, " transformation skipped because of type mismatch");
print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
}
- gimple_remove_histogram_value (cfun, stmt, histogram);
return false;
}
@@ -1686,7 +1582,6 @@ gimple_stringops_transform (gimple_stmt_iterator *gsi)
val = histogram->hvalue.counters[0];
count = histogram->hvalue.counters[1];
all = histogram->hvalue.counters[2];
- gimple_remove_histogram_value (cfun, stmt, histogram);
/* We require that count is at least half of all; this means
that for the transformation to fire the value must be constant
@@ -1763,10 +1658,7 @@ stringop_block_profile (gimple *stmt, unsigned int *expected_align,
if (!histogram)
*expected_size = -1;
else if (!histogram->hvalue.counters[1])
- {
- *expected_size = -1;
- gimple_remove_histogram_value (cfun, stmt, histogram);
- }
+ *expected_size = -1;
else
{
gcov_type size;
@@ -1778,7 +1670,6 @@ stringop_block_profile (gimple *stmt, unsigned int *expected_align,
if (size > INT_MAX)
size = INT_MAX;
*expected_size = size;
- gimple_remove_histogram_value (cfun, stmt, histogram);
}
histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_IOR);
@@ -1786,10 +1677,7 @@ stringop_block_profile (gimple *stmt, unsigned int *expected_align,
if (!histogram)
*expected_align = 0;
else if (!histogram->hvalue.counters[0])
- {
- gimple_remove_histogram_value (cfun, stmt, histogram);
- *expected_align = 0;
- }
+ *expected_align = 0;
else
{
gcov_type count;
@@ -1801,7 +1689,6 @@ stringop_block_profile (gimple *stmt, unsigned int *expected_align,
&& (alignment <= UINT_MAX / 2 / BITS_PER_UNIT))
alignment <<= 1;
*expected_align = alignment * BITS_PER_UNIT;
- gimple_remove_histogram_value (cfun, stmt, histogram);
}
}
@@ -1995,11 +1882,5 @@ gimple_find_values_to_profile (histogram_values *values)
default:
gcc_unreachable ();
}
- if (dump_file && hist->hvalue.stmt != NULL)
- {
- fprintf (dump_file, "Stmt ");
- print_gimple_stmt (dump_file, hist->hvalue.stmt, 0, TDF_SLIM);
- dump_histogram_value (dump_file, hist);
- }
}
}
diff --git a/gcc/value-prof.h b/gcc/value-prof.h
index d0b8cda181f..f644f47e230 100644
--- a/gcc/value-prof.h
+++ b/gcc/value-prof.h
@@ -82,12 +82,9 @@ histogram_value gimple_histogram_value_of_type (struct function *, gimple *,
enum hist_type);
void gimple_add_histogram_value (struct function *, gimple *, histogram_value);
void dump_histograms_for_stmt (struct function *, FILE *, gimple *);
-void gimple_remove_histogram_value (struct function *, gimple *, histogram_value);
-void gimple_remove_stmt_histograms (struct function *, gimple *);
void gimple_duplicate_stmt_histograms (struct function *, gimple *,
struct function *, gimple *);
void gimple_move_stmt_histograms (struct function *, gimple *, gimple *);
-void verify_histograms (void);
void free_histograms (function *);
void stringop_block_profile (gimple *, unsigned int *, HOST_WIDE_INT *);
gcall *gimple_ic (gcall *, struct cgraph_node *, profile_probability);
