On Mon, Oct 6, 2014 at 12:21 PM, Jan Hubicka <hubi...@ucw.cz> wrote: >> On Mon, Oct 6, 2014 at 11:22 AM, Jan Hubicka <hubi...@ucw.cz> wrote: >> > Rong, >> > Would be possible to use topn profiler to get resonale histograms for >> > switch >> > expansion, too? In that case it may make sense to have value version of >> > it as well. >>
Yes. That's doable. I'm just not sure if top 2 entries are enough. I can see from your example that it will be useful for BBs having multiple case stmt. >> The underlying topn_profiler can be used the same way as the current >> one_value profiler for different scenarios. However, I wonder if it is >> useful for switch value profiling -- the arc profiling can do the >> same. > > Well, for switch expansion one really wants a histogram of values across the > switch > values for the decision tree balancing. But I guess this profiler does not > fit > that very well. > > Consider cases like > > switch (a) > { > case 3: > case 5: > case 7: > prime(); > case 2: > case 4: > case 6: > even(); > default: > bogus (); > } > > Arc profiling does tell you how often a is prime or even, but it does not > help you > to balance the tree, because the value ranges are iinterlacing >> >> David >> >> > >> > Otherwise the patch seems OK. I would implement it myself by introducing >> > separate >> > variables holding the topn profiler declaration, but can not think of >> > downsides of >> > doing it your way. >> > >> > The topn profiler should be better on determining the dominating target, >> > so perhaps >> > you could look into hooking it into ipa-profile.c. Extending speculative >> > edges for >> > multiple targets ought to be also quite easy - we need to update >> > speculative_call_info >> > to collect edges annotated to a given call stmt into a vector instead of >> > taking the >> > two references it does now. >> > >> > It would be also very nice to update it to handle case where all the edges >> > are direct >> > so we can use it tospeculatively inlinine functions that can be interposed >> > at runtime. Yes. I'm looking at this too. I actually ported the fdo-use part of the patch and it passes spec2006. But it performs the transformation using the old way (in gimple_ic_transform()) -- so I did not send out that patch. I'm trying to do this in the new way (in ipa-profile()). In any case, that will be a separated patch. Also, the switch expansion profile should be also a separated patch. Is it ok to commit these two patches now? Thanks, -Rong >> > >> > Thanks, >> > Honza >> > >> > Index: gcc/tree-profile.c >> > =================================================================== >> > --- gcc/tree-profile.c (revision 215886) >> > +++ gcc/tree-profile.c (working copy) >> > @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3. If not see >> > #include "target.h" >> > #include "tree-cfgcleanup.h" >> > #include "tree-nested.h" >> > +#include "params.h" >> > >> > static GTY(()) tree gcov_type_node; >> > static GTY(()) tree tree_interval_profiler_fn; >> > @@ -101,7 +102,10 @@ init_ic_make_global_vars (void) >> > { >> > ic_void_ptr_var >> > = build_decl (UNKNOWN_LOCATION, VAR_DECL, >> > - get_identifier ("__gcov_indirect_call_callee"), >> > + get_identifier ( >> > + (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) >> > ? >> > + "__gcov_indirect_call_topn_callee" : >> > + "__gcov_indirect_call_callee")), >> > ptr_void); >> > TREE_PUBLIC (ic_void_ptr_var) = 1; >> > DECL_EXTERNAL (ic_void_ptr_var) = 1; >> > @@ -131,7 +135,10 @@ init_ic_make_global_vars (void) >> > { >> > ic_gcov_type_ptr_var >> > = build_decl (UNKNOWN_LOCATION, VAR_DECL, >> > - get_identifier ("__gcov_indirect_call_counters"), >> > + get_identifier ( >> > + (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) >> > ? >> > + "__gcov_indirect_call_topn_counters" : >> > + "__gcov_indirect_call_counters")), >> > gcov_type_ptr); >> > TREE_PUBLIC (ic_gcov_type_ptr_var) = 1; >> > DECL_EXTERNAL (ic_gcov_type_ptr_var) = 1; >> > @@ -226,8 +233,10 @@ gimple_init_edge_profiler (void) >> > ptr_void, >> > NULL_TREE); >> > tree_indirect_call_profiler_fn >> > - = build_fn_decl ("__gcov_indirect_call_profiler_v2", >> > - ic_profiler_fn_type); >> > + = build_fn_decl ( (PARAM_VALUE >> > (PARAM_INDIR_CALL_TOPN_PROFILE) ? >> > + "__gcov_indirect_call_topn_profiler": >> > + "__gcov_indirect_call_profiler_v2"), >> > + ic_profiler_fn_type); >> > } >> > TREE_NOTHROW (tree_indirect_call_profiler_fn) = 1; >> > DECL_ATTRIBUTES (tree_indirect_call_profiler_fn) >> > @@ -398,6 +407,12 @@ gimple_gen_ic_profiler (histogram_value value, uns >> > gimple_stmt_iterator gsi = gsi_for_stmt (stmt); >> > tree ref_ptr = tree_coverage_counter_addr (tag, base); >> > >> > + if ( (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) && >> > + tag == GCOV_COUNTER_V_INDIR) || >> > + (!PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) && >> > + tag == GCOV_COUNTER_ICALL_TOPNV)) >> > + return; >> > + >> > ref_ptr = force_gimple_operand_gsi (&gsi, ref_ptr, >> > true, NULL_TREE, true, >> > GSI_SAME_STMT); >> > >> > @@ -442,8 +457,7 @@ gimple_gen_ic_func_profiler (void) >> > stmt1: __gcov_indirect_call_profiler_v2 (profile_id, >> > ¤t_function_decl) >> > */ >> > - gsi = >> > - gsi_after_labels (split_edge >> > (single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)))); >> > + gsi = gsi_after_labels (split_edge (single_succ_edge >> > (ENTRY_BLOCK_PTR_FOR_FN (cfun)))); >> > >> > cur_func = force_gimple_operand_gsi (&gsi, >> > build_addr (current_function_decl, >> > Index: gcc/value-prof.c >> > =================================================================== >> > --- gcc/value-prof.c (revision 215886) >> > +++ gcc/value-prof.c (working copy) >> > @@ -60,6 +60,7 @@ along with GCC; see the file COPYING3. If not see >> > #include "builtins.h" >> > #include "tree-nested.h" >> > #include "hash-set.h" >> > +#include "params.h" >> > >> > /* In this file value profile based optimizations are placed. Currently >> > the >> > following optimizations are implemented (for more detailed descriptions >> > @@ -359,6 +360,22 @@ dump_histogram_value (FILE *dump_file, histogram_v >> > } >> > fprintf (dump_file, ".\n"); >> > break; >> > + case HIST_TYPE_INDIR_CALL_TOPN: >> > + fprintf (dump_file, "Indirect call topn "); >> > + if (hist->hvalue.counters) >> > + { >> > + int i; >> > + >> > + fprintf (dump_file, "accu:%"PRId64, hist->hvalue.counters[0]); >> > + for (i = 1; i < (GCOV_ICALL_TOPN_VAL << 2); i += 2) >> > + { >> > + fprintf (dump_file, " target:%"PRId64 " value:%"PRId64, >> > + (int64_t) hist->hvalue.counters[i], >> > + (int64_t) hist->hvalue.counters[i+1]); >> > + } >> > + } >> > + fprintf (dump_file, ".\n"); >> > + break; >> > case HIST_TYPE_MAX: >> > gcc_unreachable (); >> > } >> > @@ -432,9 +449,14 @@ stream_in_histogram_value (struct lto_input_block >> > break; >> > >> > case HIST_TYPE_IOR: >> > - case HIST_TYPE_TIME_PROFILE: >> > + case HIST_TYPE_TIME_PROFILE: >> > ncounters = 1; >> > break; >> > + >> > + case HIST_TYPE_INDIR_CALL_TOPN: >> > + ncounters = (GCOV_ICALL_TOPN_VAL << 2) + 1; >> > + break; >> > + >> > case HIST_TYPE_MAX: >> > gcc_unreachable (); >> > } >> > @@ -1920,8 +1942,12 @@ gimple_indirect_call_to_profile (gimple stmt, hist >> > >> > values->reserve (3); >> > >> > - values->quick_push (gimple_alloc_histogram_value (cfun, >> > HIST_TYPE_INDIR_CALL, >> > - stmt, callee)); >> > + values->quick_push (gimple_alloc_histogram_value ( >> > + cfun, >> > + PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ? >> > + HIST_TYPE_INDIR_CALL_TOPN : >> > + HIST_TYPE_INDIR_CALL, >> > + stmt, callee)); >> > >> > return; >> > } >> > @@ -2011,9 +2037,9 @@ gimple_find_values_to_profile (histogram_values *v >> > hist->n_counters = 3; >> > break; >> > >> > - case HIST_TYPE_TIME_PROFILE: >> > - hist->n_counters = 1; >> > - break; >> > + case HIST_TYPE_TIME_PROFILE: >> > + hist->n_counters = 1; >> > + break; >> > >> > case HIST_TYPE_AVERAGE: >> > hist->n_counters = 2; >> > @@ -2023,6 +2049,10 @@ gimple_find_values_to_profile (histogram_values *v >> > hist->n_counters = 1; >> > break; >> > >> > + case HIST_TYPE_INDIR_CALL_TOPN: >> > + hist->n_counters = GCOV_ICALL_TOPN_NCOUNTS; >> > + break; >> > + >> > default: >> > gcc_unreachable (); >> > } >> > Index: gcc/value-prof.h >> > =================================================================== >> > --- gcc/value-prof.h (revision 215886) >> > +++ gcc/value-prof.h (working copy) >> > @@ -35,6 +35,8 @@ enum hist_type >> > HIST_TYPE_AVERAGE, /* Compute average value (sum of all values). */ >> > HIST_TYPE_IOR, /* Used to compute expected alignment. */ >> > HIST_TYPE_TIME_PROFILE, /* Used for time profile */ >> > + HIST_TYPE_INDIR_CALL_TOPN, /* Tries to identify the top N most >> > frequently >> > + called functions in indirect call. */ >> > HIST_TYPE_MAX >> > }; >> > >> > Index: gcc/profile.c >> > =================================================================== >> > --- gcc/profile.c (revision 215886) >> > +++ gcc/profile.c (working copy) >> > @@ -183,6 +183,7 @@ instrument_values (histogram_values values) >> > break; >> > >> > case HIST_TYPE_INDIR_CALL: >> > + case HIST_TYPE_INDIR_CALL_TOPN: >> > gimple_gen_ic_profiler (hist, t, 0); >> > break; >> >