Re: [PATCH] Fix part of PR bootstrap/55051 (issue6846063)

2012-11-18 Thread Teresa Johnson
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)
-  

Re: [PATCH] Fix part of PR bootstrap/55051 (issue6846063)

2012-11-16 Thread Jan Hubicka
 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


[PATCH] Fix part of PR bootstrap/55051 (issue6846063)

2012-11-15 Thread Teresa Johnson
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)))
-   {
- 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);
- all_prg.checksum = ~0u;
-   }
}
 
   prg.checksum = crc32;

--
This patch is available for review at http://codereview.appspot.com/6846063