| Message ID | 20141006182239.GB6942@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 6, 2014 at 11:22 AM, Jan Hubicka <hubicka@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. 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. 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. > > 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; >
> On Mon, Oct 6, 2014 at 11:22 AM, Jan Hubicka <hubicka@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. > > 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. > > > > 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; > >
On Mon, Oct 6, 2014 at 12:21 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >> On Mon, Oct 6, 2014 at 11:22 AM, Jan Hubicka <hubicka@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; >> >
> > 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. I suppose we want real histogram profiler here. I.e. something that is given value ranges and fill in the counts depending on value ranges. Other case where your profiler may be useful is the division/modulo conversion. For hashtables divisions/modules are often small sets of prime numbers. > > 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? Yes, it is OK, thanks! Honza > > 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; > >> >
On Mon, Oct 6, 2014 at 1:31 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >> >> 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. > > I suppose we want real histogram profiler here. I.e. something that is given > value ranges and fill in the counts depending on value ranges. > Other case where your profiler may be useful is the division/modulo conversion. > For hashtables divisions/modules are often small sets of prime numbers. yes, tracking value ranges sounds interesting. The topn is also configurable to track more values, but it will become more expensive to use. David >> >> 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? > > Yes, it is OK, thanks! > > Honza >> >> 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; >> >> >
On 6 October 2014 22:31:18 CEST, Jan Hubicka <hubicka@ucw.cz> wrote: >> >> Is it ok to commit these two patches now? > >Yes, it is OK, thanks! I do not see documentation of the new parameter added to doc in the ChangeLog? Also, I would not abbreviate "indir" in the param name. Thanks,
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;