On Fri, Nov 16, 2012 at 11:07 AM, Jan Hubicka <hubi...@ucw.cz> wrote: >> This patch addresses the bogus "Invocation mismatch" messages seen in >> parallel >> profiledbootstrap builds of gcc. See PR bootstrap/55051 for a discussion of >> why this is occurring and why this checking is inaccurate. >> >> Profilebootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk? >> >> 2012-11-15 Teresa Johnson <tejohn...@google.com> >> >> PR bootstrap/55051 >> * libgcov.c (gcov_exit): Remove checking against first >> merged summary for program, as multiple simultaneous >> processes attempting to update gcda files may cause >> summaries to temporarily differ. >> >> Index: libgcov.c >> =================================================================== >> --- libgcov.c (revision 193522) >> +++ libgcov.c (working copy) >> @@ -365,7 +365,6 @@ gcov_exit (void) >> struct gcov_info *gi_ptr; >> const struct gcov_fn_info *gfi_ptr; >> struct gcov_summary this_prg; /* summary for program. */ >> - struct gcov_summary all_prg; /* summary for all instances of program. */ >> struct gcov_ctr_summary *cs_ptr; >> const struct gcov_ctr_info *ci_ptr; >> unsigned t_ix; >> @@ -382,7 +381,6 @@ gcov_exit (void) >> if (gcov_dump_complete) >> return; >> >> - memset (&all_prg, 0, sizeof (all_prg)); >> /* Find the totals for this execution. */ >> memset (&this_prg, 0, sizeof (this_prg)); >> for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next) >> @@ -469,7 +467,7 @@ gcov_exit (void) >> unsigned n_counts; >> struct gcov_summary prg; /* summary for this object over all >> program. */ >> - struct gcov_ctr_summary *cs_prg, *cs_tprg, *cs_all; >> + struct gcov_ctr_summary *cs_prg, *cs_tprg; >> int error = 0; >> gcov_unsigned_t tag, length; >> gcov_position_t summary_pos = 0; >> @@ -684,7 +682,6 @@ gcov_exit (void) >> { >> cs_prg = &prg.ctrs[t_ix]; >> cs_tprg = &this_prg.ctrs[t_ix]; >> - cs_all = &all_prg.ctrs[t_ix]; >> >> if (gi_ptr->merge[t_ix]) >> { >> @@ -702,24 +699,6 @@ gcov_exit (void) >> } >> else if (cs_prg->runs) >> goto read_mismatch; >> - >> - if (!cs_all->runs && cs_prg->runs) >> - memcpy (cs_all, cs_prg, sizeof (*cs_all)); >> - else if (!all_prg.checksum >> - && (!GCOV_LOCKED || cs_all->runs == cs_prg->runs) >> - /* Don't compare the histograms, which may have slight >> - variations depending on the order they were updated >> - due to the truncating integer divides used in the >> - merge. */ >> - && memcmp (cs_all, cs_prg, >> - sizeof (*cs_all) - (sizeof (gcov_bucket_type) >> - * GCOV_HISTOGRAM_SIZE))) > Please keep the massage around with !GCOV_LOCKED. In that case user should be > informed that parallel exeuction is bad idea (tm). > > The memcpy/memcmp pair with sizeof (gcov_bucket_type) adjustment is ugly. Just > copy those few relevant values into temporary vars. > > OK with this change. > > Honza
Ok, just committed the following: 012-11-18 Teresa Johnson <tejohn...@google.com> PR bootstrap/55051 * libgcov.c (gcov_exit): Remove merged program summary comparison unless !GCOV_LOCKED. Index: libgcov.c =================================================================== --- libgcov.c (revision 193522) +++ libgcov.c (working copy) @@ -365,7 +365,9 @@ gcov_exit (void) struct gcov_info *gi_ptr; const struct gcov_fn_info *gfi_ptr; struct gcov_summary this_prg; /* summary for program. */ +#if !GCOV_LOCKED struct gcov_summary all_prg; /* summary for all instances of program. */ +#endif struct gcov_ctr_summary *cs_ptr; const struct gcov_ctr_info *ci_ptr; unsigned t_ix; @@ -382,7 +384,9 @@ gcov_exit (void) if (gcov_dump_complete) return; +#if !GCOV_LOCKED memset (&all_prg, 0, sizeof (all_prg)); +#endif /* Find the totals for this execution. */ memset (&this_prg, 0, sizeof (this_prg)); for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next) @@ -469,7 +473,10 @@ gcov_exit (void) unsigned n_counts; struct gcov_summary prg; /* summary for this object over all program. */ - struct gcov_ctr_summary *cs_prg, *cs_tprg, *cs_all; + struct gcov_ctr_summary *cs_prg, *cs_tprg; +#if !GCOV_LOCKED + struct gcov_ctr_summary *cs_all; +#endif int error = 0; gcov_unsigned_t tag, length; gcov_position_t summary_pos = 0; @@ -684,7 +691,6 @@ gcov_exit (void) { cs_prg = &prg.ctrs[t_ix]; cs_tprg = &this_prg.ctrs[t_ix]; - cs_all = &all_prg.ctrs[t_ix]; if (gi_ptr->merge[t_ix]) { @@ -703,23 +709,34 @@ gcov_exit (void) else if (cs_prg->runs) goto read_mismatch; +#if !GCOV_LOCKED + cs_all = &all_prg.ctrs[t_ix]; if (!cs_all->runs && cs_prg->runs) - memcpy (cs_all, cs_prg, sizeof (*cs_all)); + { + cs_all->num = cs_prg->num; + cs_all->runs = cs_prg->runs; + cs_all->sum_all = cs_prg->sum_all; + cs_all->run_max = cs_prg->run_max; + cs_all->sum_max = cs_prg->sum_max; + } else if (!all_prg.checksum - && (!GCOV_LOCKED || cs_all->runs == cs_prg->runs) /* Don't compare the histograms, which may have slight variations depending on the order they were updated due to the truncating integer divides used in the - && memcmp (cs_all, cs_prg, - sizeof (*cs_all) - (sizeof (gcov_bucket_type) - * GCOV_HISTOGRAM_SIZE))) + && (cs_all->num != cs_prg->num + || cs_all->runs != cs_prg->runs + || cs_all->sum_all != cs_prg->sum_all + || cs_all->run_max != cs_prg->run_max + || cs_all->sum_max != cs_prg->sum_max)) { - fprintf (stderr, "profiling:%s:Invocation mismatch - some data files may have been removed%s\n", - gi_filename, GCOV_LOCKED - ? "" : " or concurrently updated without locking support"); + fprintf (stderr, + "profiling:%s:Data file mismatch - some data files may " + "have been concurrently updated without locking support\n", + gi_filename); all_prg.checksum = ~0u; } +#endif } prg.checksum = crc32; Thanks, Teresa -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413