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 <mli...@suse.cz> * 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 <mli...@suse.cz> * 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);