Re: [PATCH 1/4] Remove indirect call top N counter type.
> On 6/6/19 2:52 PM, Jan Hubicka wrote: > > Hi, > > so the only point of removing this is the fact that builds would be > > not reproducible with indir-call-topn-profile? > > That's one reason. But the main reason is that the code is dead and not used. > If you take a look at gcc/ipa-profile.c:189, the code is not supporting > HIST_TYPE_INDIR_CALL_TOPN. Yep, i know. It is not that hard to add support for this - we currently assume that speculative edges have one direct and one indirect call. We need to add support to have multiple indirect calls which would not be too hard, but it never got high enough in my TODO list. Option is to do the mutiple value speuclation the old way dirrect in VPT. I was thinking that other possible use of TOPN is the division used in growing hashtable implementations. Those usually have small set of built in prime numbers which they use as array size and modulo always use one of the values. This is however usually not quite visible to compiler and TOPN counter could work that out. So we could do automatically more or less what htab_mod does. But yep, I have no time to work on this currently, so we may just drop it and see if we will want to recover it later. Honza > So it's broken for few years and nobody is using that. Last reason is > implementation of: > __gcov_merge_icall_topn. It's over-complicated. > > Martin > > > I still kind of thing it may be useful to track multiple most common > > values, so I would be in favour of keeping it just updating the > > documentation > > of indir-call-topn-profile that it is currently incomplete and does not > > lead to reproducible builds and does not handle speculation... > > > > Honza >
Re: [PATCH 1/4] Remove indirect call top N counter type.
On 6/6/19 2:52 PM, Jan Hubicka wrote: > Hi, > so the only point of removing this is the fact that builds would be > not reproducible with indir-call-topn-profile? That's one reason. But the main reason is that the code is dead and not used. If you take a look at gcc/ipa-profile.c:189, the code is not supporting HIST_TYPE_INDIR_CALL_TOPN. So it's broken for few years and nobody is using that. Last reason is implementation of: __gcov_merge_icall_topn. It's over-complicated. Martin > I still kind of thing it may be useful to track multiple most common > values, so I would be in favour of keeping it just updating the documentation > of indir-call-topn-profile that it is currently incomplete and does not > lead to reproducible builds and does not handle speculation... > > Honza
Re: [PATCH 1/4] Remove indirect call top N counter type.
Hi, so the only point of removing this is the fact that builds would be not reproducible with indir-call-topn-profile? I still kind of thing it may be useful to track multiple most common values, so I would be in favour of keeping it just updating the documentation of indir-call-topn-profile that it is currently incomplete and does not lead to reproducible builds and does not handle speculation... Honza > > gcc/ChangeLog: > > 2019-06-04 Martin Liska > > * doc/invoke.texi: Remove param. > * gcov-counter.def (GCOV_COUNTER_ICALL_TOPNV): > Remove. > * gcov-io.h (GCOV_ICALL_TOPN_VAL): Likewise. > (GCOV_ICALL_TOPN_NCOUNTS): Likewise. > * params.def (PARAM_INDIR_CALL_TOPN_PROFILE): Likewise. > * profile.c (instrument_values): Remove > HIST_TYPE_INDIR_CALL_TOPN. > * tree-profile.c (init_ic_make_global_vars): > Always build __gcov_indirect_call only. > (gimple_init_gcov_profiler): Remove usage > of PARAM_INDIR_CALL_TOPN_PROFILE. > (gimple_gen_ic_profiler): Likewise. > * value-prof.c (dump_histogram_value): Likewise. > (stream_in_histogram_value): Likewise. > (gimple_indirect_call_to_profile): Likewise. > (gimple_find_values_to_profile): Likewise. > * value-prof.h (enum hist_type): Likewise. > > libgcc/ChangeLog: > > 2019-06-04 Martin Liska > > * Makefile.in: Remove usage of > _gcov_merge_icall_topn. > * libgcov-driver.c (gcov_sort_n_vals): Remove. > (gcov_sort_icall_topn_counter): Likewise. > (gcov_sort_topn_counter_arrays): Likewise. > (dump_one_gcov): Remove call to gcov_sort_topn_counter_arrays. > * libgcov-merge.c (__gcov_merge_icall_topn): Remove. > * libgcov-profiler.c (__gcov_topn_value_profiler_body): > Likewise. > (GCOV_ICALL_COUNTER_CLEAR_THRESHOLD): Remove. > (struct indirect_call_tuple): Remove. > (__gcov_indirect_call_topn_profiler): Remove. > * libgcov-util.c (__gcov_icall_topn_counter_op): Remove. > * libgcov.h (gcov_sort_n_vals): Remove. > (L_gcov_merge_icall_topn): Likewise. > (__gcov_merge_icall_topn): Likewise. > (__gcov_indirect_call_topn_profiler): Likewise. > --- > gcc/doc/invoke.texi | 3 - > gcc/gcov-counter.def | 3 - > gcc/gcov-io.h | 6 -- > gcc/params.def| 8 --- > gcc/profile.c | 1 - > gcc/tree-profile.c| 14 +--- > gcc/value-prof.c | 32 + > gcc/value-prof.h | 2 - > libgcc/Makefile.in| 5 +- > libgcc/libgcov-driver.c | 80 --- > libgcc/libgcov-merge.c| 62 -- > libgcc/libgcov-profiler.c | 133 -- > libgcc/libgcov-util.c | 19 -- > libgcc/libgcov.h | 7 -- > 14 files changed, 5 insertions(+), 370 deletions(-) > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 91c9bb89651..50e50e39413 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -12140,9 +12140,6 @@ will not try to thread through its block. > Maximum number of nested calls to search for control dependencies > during uninitialized variable analysis. > > -@item indir-call-topn-profile > -Track top N target addresses in indirect-call profile. > - > @item max-once-peeled-insns > The maximum number of insns of a peeled loop that rolls only once. > > diff --git a/gcc/gcov-counter.def b/gcc/gcov-counter.def > index 3a0e620987a..b0596c8dc6b 100644 > --- a/gcc/gcov-counter.def > +++ b/gcc/gcov-counter.def > @@ -49,6 +49,3 @@ DEF_GCOV_COUNTER(GCOV_COUNTER_IOR, "ior", _ior) > > /* Time profile collecting first run of a function */ > DEF_GCOV_COUNTER(GCOV_TIME_PROFILER, "time_profiler", _time_profile) > - > -/* Top N value tracking for indirect calls. */ > -DEF_GCOV_COUNTER(GCOV_COUNTER_ICALL_TOPNV, "indirect_call_topn", _icall_topn) > diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h > index 9edb2923982..69c9a73dba8 100644 > --- a/gcc/gcov-io.h > +++ b/gcc/gcov-io.h > @@ -266,12 +266,6 @@ GCOV_COUNTERS > #define GCOV_N_VALUE_COUNTERS \ >(GCOV_LAST_VALUE_COUNTER - GCOV_FIRST_VALUE_COUNTER + 1) > > -/* The number of hottest callees to be tracked. */ > -#define GCOV_ICALL_TOPN_VAL 2 > - > -/* The number of counter entries per icall callsite. */ > -#define GCOV_ICALL_TOPN_NCOUNTS (1 + GCOV_ICALL_TOPN_VAL * 4) > - > /* Convert a counter index to a tag. */ > #define GCOV_TAG_FOR_COUNTER(COUNT) \ > (GCOV_TAG_COUNTER_BASE + ((gcov_unsigned_t)(COUNT) << 17)) > diff --git a/gcc/params.def b/gcc/params.def > index 6b7f7eb5bae..b4a4e4a4190 100644 > --- a/gcc/params.def > +++ b/gcc/params.def > @@ -992,14 +992,6 @@ DEFPARAM (PARAM_PROFILE_FUNC_INTERNAL_ID, > "Use internal function id in profile lookup.", > 0, 0, 1) > > -/* When the parameter is 1, track the most frequent N target > - addresses in indirect-call profile. This
[PATCH 1/4] Remove indirect call top N counter type.
gcc/ChangeLog: 2019-06-04 Martin Liska * doc/invoke.texi: Remove param. * gcov-counter.def (GCOV_COUNTER_ICALL_TOPNV): Remove. * gcov-io.h (GCOV_ICALL_TOPN_VAL): Likewise. (GCOV_ICALL_TOPN_NCOUNTS): Likewise. * params.def (PARAM_INDIR_CALL_TOPN_PROFILE): Likewise. * profile.c (instrument_values): Remove HIST_TYPE_INDIR_CALL_TOPN. * tree-profile.c (init_ic_make_global_vars): Always build __gcov_indirect_call only. (gimple_init_gcov_profiler): Remove usage of PARAM_INDIR_CALL_TOPN_PROFILE. (gimple_gen_ic_profiler): Likewise. * value-prof.c (dump_histogram_value): Likewise. (stream_in_histogram_value): Likewise. (gimple_indirect_call_to_profile): Likewise. (gimple_find_values_to_profile): Likewise. * value-prof.h (enum hist_type): Likewise. libgcc/ChangeLog: 2019-06-04 Martin Liska * Makefile.in: Remove usage of _gcov_merge_icall_topn. * libgcov-driver.c (gcov_sort_n_vals): Remove. (gcov_sort_icall_topn_counter): Likewise. (gcov_sort_topn_counter_arrays): Likewise. (dump_one_gcov): Remove call to gcov_sort_topn_counter_arrays. * libgcov-merge.c (__gcov_merge_icall_topn): Remove. * libgcov-profiler.c (__gcov_topn_value_profiler_body): Likewise. (GCOV_ICALL_COUNTER_CLEAR_THRESHOLD): Remove. (struct indirect_call_tuple): Remove. (__gcov_indirect_call_topn_profiler): Remove. * libgcov-util.c (__gcov_icall_topn_counter_op): Remove. * libgcov.h (gcov_sort_n_vals): Remove. (L_gcov_merge_icall_topn): Likewise. (__gcov_merge_icall_topn): Likewise. (__gcov_indirect_call_topn_profiler): Likewise. --- gcc/doc/invoke.texi | 3 - gcc/gcov-counter.def | 3 - gcc/gcov-io.h | 6 -- gcc/params.def| 8 --- gcc/profile.c | 1 - gcc/tree-profile.c| 14 +--- gcc/value-prof.c | 32 + gcc/value-prof.h | 2 - libgcc/Makefile.in| 5 +- libgcc/libgcov-driver.c | 80 --- libgcc/libgcov-merge.c| 62 -- libgcc/libgcov-profiler.c | 133 -- libgcc/libgcov-util.c | 19 -- libgcc/libgcov.h | 7 -- 14 files changed, 5 insertions(+), 370 deletions(-) diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 91c9bb89651..50e50e39413 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -12140,9 +12140,6 @@ will not try to thread through its block. Maximum number of nested calls to search for control dependencies during uninitialized variable analysis. -@item indir-call-topn-profile -Track top N target addresses in indirect-call profile. - @item max-once-peeled-insns The maximum number of insns of a peeled loop that rolls only once. diff --git a/gcc/gcov-counter.def b/gcc/gcov-counter.def index 3a0e620987a..b0596c8dc6b 100644 --- a/gcc/gcov-counter.def +++ b/gcc/gcov-counter.def @@ -49,6 +49,3 @@ DEF_GCOV_COUNTER(GCOV_COUNTER_IOR, "ior", _ior) /* Time profile collecting first run of a function */ DEF_GCOV_COUNTER(GCOV_TIME_PROFILER, "time_profiler", _time_profile) - -/* Top N value tracking for indirect calls. */ -DEF_GCOV_COUNTER(GCOV_COUNTER_ICALL_TOPNV, "indirect_call_topn", _icall_topn) diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h index 9edb2923982..69c9a73dba8 100644 --- a/gcc/gcov-io.h +++ b/gcc/gcov-io.h @@ -266,12 +266,6 @@ GCOV_COUNTERS #define GCOV_N_VALUE_COUNTERS \ (GCOV_LAST_VALUE_COUNTER - GCOV_FIRST_VALUE_COUNTER + 1) -/* The number of hottest callees to be tracked. */ -#define GCOV_ICALL_TOPN_VAL 2 - -/* The number of counter entries per icall callsite. */ -#define GCOV_ICALL_TOPN_NCOUNTS (1 + GCOV_ICALL_TOPN_VAL * 4) - /* Convert a counter index to a tag. */ #define GCOV_TAG_FOR_COUNTER(COUNT)\ (GCOV_TAG_COUNTER_BASE + ((gcov_unsigned_t)(COUNT) << 17)) diff --git a/gcc/params.def b/gcc/params.def index 6b7f7eb5bae..b4a4e4a4190 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -992,14 +992,6 @@ DEFPARAM (PARAM_PROFILE_FUNC_INTERNAL_ID, "Use internal function id in profile lookup.", 0, 0, 1) -/* When the parameter is 1, track the most frequent N target - addresses in indirect-call profile. This disables - indirect_call_profiler_v3 which tracks single target. */ -DEFPARAM (PARAM_INDIR_CALL_TOPN_PROFILE, - "indir-call-topn-profile", - "Track top N target addresses in indirect-call profile.", - 0, 0, 1) - /* Avoid SLP vectorization of large basic blocks. */ DEFPARAM (PARAM_SLP_MAX_INSNS_IN_BB, "slp-max-insns-in-bb", diff --git a/gcc/profile.c b/gcc/profile.c index a1dba1ac8fb..9aff9ef2b21 100644 --- a/gcc/profile.c +++ b/gcc/profile.c @@ -172,7 +172,6 @@ instrument_values (histogram_values values) break; case HIST_TYPE_INDIR_CALL: - case HIST_TYPE_INDIR_CALL_TOPN:
