> On 1/21/21 8:03 PM, Jan Hubicka wrote: > > What exactly is suggested? > > This one. > > Martin
> From 22bbf5342f2b73fad6c0286541bba6699c617380 Mon Sep 17 00:00:00 2001 > From: Martin Liska <mli...@suse.cz> > Date: Thu, 21 Jan 2021 09:02:31 +0100 > Subject: [PATCH 1/2] Restore -fprofile-reproducibility flag. > > gcc/ChangeLog: > > PR gcov-profile/98739 > * common.opt: Add missing equal symbol. > * value-prof.c (get_nth_most_common_value): Fix comment > and skip TOP N counter properly when -fprofile-reproducibility > is not serial. > --- > gcc/common.opt | 2 +- > gcc/value-prof.c | 18 ++++++++---------- > 2 files changed, 9 insertions(+), 11 deletions(-) > > bool > get_nth_most_common_value (gimple *stmt, const char *counter_type, > @@ -765,15 +762,16 @@ get_nth_most_common_value (gimple *stmt, const char > *counter_type, > *count = 0; > *value = 0; > > - gcov_type read_all = abs_hwi (hist->hvalue.counters[0]); > + gcov_type read_all = hist->hvalue.counters[0]; > + gcov_type covered = 0; > + for (unsigned i = 0; i < counters; ++i) > + covered += hist->hvalue.counters[2 * i + 3]; > > gcov_type v = hist->hvalue.counters[2 * n + 2]; > gcov_type c = hist->hvalue.counters[2 * n + 3]; > > - if (hist->hvalue.counters[0] < 0 > - && (flag_profile_reproducible == PROFILE_REPRODUCIBILITY_PARALLEL_RUNS > - || (flag_profile_reproducible > - == PROFILE_REPRODUCIBILITY_MULTITHREADED))) > + if (read_all != covered > + && flag_profile_reproducible != PROFILE_REPRODUCIBILITY_SERIAL) This should be right for REPRODUCIBILITY_MULTITHREADED but is too strict for PARALLEL_RUNS (and I think we now have data that this difference matters). If you 1) re-add logic that avoids stremaing targets with no more than 1/32 of overall execution counts from each run (we may want to have way to tweak the threshold, but I guess we may want to first see if it is necessary since it is easy to add and we already have bit too many environment variables) 2) re-add logic tracking if any values was lost during merging using the sign of first counter 3) make PARALLEL_RUNS to disregard the profile if the first counter is negetaive We sould be able to track most of cases where number of values exceeds 32 but there is one or two really dominating. Also I think it makes sense to default to =serial and use the new flag in the few packages where we do profile feedback and care about reproducibility. Thanks a lot for looking into this! Honza > return false; > > /* Indirect calls can't be verified. */ > -- > 2.30.0 >
