diff mbox

Fix part of PR bootstrap/55051 (issue6846063)

Message ID CAAe5K+Xk6SzG8wocObKsCwY-EGuk6Vm9mr_vfm+bXzbDgo2K0g@mail.gmail.com
State New
Headers show

Commit Message

Teresa Johnson Nov. 19, 2012, 5:25 a.m. UTC 
On Fri, Nov 16, 2012 at 11:07 AM, Jan Hubicka <hubicka@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  <tejohnson@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  <tejohnson@google.com>

        PR bootstrap/55051
        * libgcov.c (gcov_exit): Remove merged program summary
        comparison unless !GCOV_LOCKED.


       prg.checksum = crc32;

Thanks,
Teresa



--
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
diff mbox

Patch

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
        }