diff mbox

Coverage unlinking

Message ID 4FE6043C.6020608@acm.org
State New
Headers show

Commit Message

Nathan Sidwell June 23, 2012, 6 p.m. UTC 
This patch changes the coverage unlinking to be less aggressive.  As David 
pointed out, when experimenting with random optimization options along side 
-fuse-coverage and -frandom-seed, it is inconvenient for gcc to remove the 
coverage data file.  The reason it was doing so, is because -frandom-seed 
prevents a unique stamp being placed in the coverage data to distinguish 
execution of the newly optimized program from execution of the older incarnation 
of it.  libgcov would get confused and give errors about mismatched function 
checksums.

This patch tweaks things.  With -fuse-coverage we read in the coverage data file 
and its stamp.  We can feed that stamp into the stamp to be used for the output 
coverage data, and hence effectively distinguish generations of coverage data 
even when -frandom-seed prevents randomness.  We also don't need to delete the 
data file if we're not producing coverage data for the new compilation.

Manual testing shows this patch is preventing deletion of the data file in the 
circumstances I understand David's using.  David, care to try this patch out to 
see if it resolves your use case?

The changes to tree.c merely extend the crc32 code to allow it to generate a crc 
from a 32-bit unsigned, in addition to the byte variant already present.

testing on i686-pc-linux-gnu shows no regressions.

nathan

Comments

Xinliang David Li June 24, 2012, 12:14 a.m. UTC | #1 
thanks for the fix. It works fine for me.

David

On Sat, Jun 23, 2012 at 11:00 AM, Nathan Sidwell <nathan@acm.org> wrote:
> This patch changes the coverage unlinking to be less aggressive.  As David
> pointed out, when experimenting with random optimization options along side
> -fuse-coverage and -frandom-seed, it is inconvenient for gcc to remove the
> coverage data file.  The reason it was doing so, is because -frandom-seed
> prevents a unique stamp being placed in the coverage data to distinguish
> execution of the newly optimized program from execution of the older
> incarnation of it.  libgcov would get confused and give errors about
> mismatched function checksums.
>
> This patch tweaks things.  With -fuse-coverage we read in the coverage data
> file and its stamp.  We can feed that stamp into the stamp to be used for
> the output coverage data, and hence effectively distinguish generations of
> coverage data even when -frandom-seed prevents randomness.  We also don't
> need to delete the data file if we're not producing coverage data for the
> new compilation.
>
> Manual testing shows this patch is preventing deletion of the data file in
> the circumstances I understand David's using.  David, care to try this patch
> out to see if it resolves your use case?
>
> The changes to tree.c merely extend the crc32 code to allow it to generate a
> crc from a 32-bit unsigned, in addition to the byte variant already present.
>
> testing on i686-pc-linux-gnu shows no regressions.
>
> nathan
Nathan Sidwell June 30, 2012, 2:59 p.m. UTC | #2 
On 06/24/12 01:14, Xinliang David Li wrote:
> thanks for the fix. It works fine for me.

Thanks. I have committed it.
diff mbox

Patch

2012-06-23  Nathan Sidwell  <nathan@acm.org>

	* coverage.c (bbg_file_stamp): New.
	(read_counts_file): Merge incoming stamp with bbg_file_stamp.
	(build_info): Write bbg_file_stamp.
	(coverage_init): Initialize bbg_file_stamp.  Read counts file
	before writing graph header.
	(coverage_finish): Don't unlink the data file if we can generate a
	unique file stamp.
	* tree.h (crc32_unsigned): Declare.
	* tree.c (crc32_unsigned_bits): New, broken out of ...
	(crc32_byte): ... here.  Use it.
	(crc32_unsigned): New.

Index: coverage.c
===================================================================
--- coverage.c	(revision 188900)
+++ coverage.c	(working copy)
@@ -101,6 +101,9 @@  static GTY(()) tree gcov_fn_info_ptr_typ
    we're not writing to the notes file.  */
 static char *bbg_file_name;
 
+/* File stamp for graph file.  */
+static unsigned bbg_file_stamp;
+
 /* Name of the count data file.  */
 static char *da_file_name;
 
@@ -205,8 +208,9 @@  read_counts_file (void)
       return;
     }
 
-  /* Read and discard the stamp.  */
-  gcov_read_unsigned ();
+  /* Read the stamp, used for creating a generation count.  */
+  tag = gcov_read_unsigned ();
+  bbg_file_stamp = crc32_unsigned (bbg_file_stamp, tag);
 
   counts_hash = htab_create (10,
 			     htab_counts_entry_hash, htab_counts_entry_eq,
@@ -905,7 +909,7 @@  build_info (tree info_type, tree fn_ary)
   /* stamp */
   CONSTRUCTOR_APPEND_ELT (v1, info_fields,
 			  build_int_cstu (TREE_TYPE (info_fields),
-					  local_tick));
+					  bbg_file_stamp));
   info_fields = DECL_CHAIN (info_fields);
 
   /* Filename */
@@ -1101,6 +1105,11 @@  coverage_init (const char *filename)
   memcpy (da_file_name + prefix_len, filename, len);
   strcpy (da_file_name + prefix_len + len, GCOV_DATA_SUFFIX);
 
+  bbg_file_stamp = local_tick;
+  
+  if (flag_branch_probabilities)
+    read_counts_file ();
+
   /* Name of bbg file.  */
   if (flag_test_coverage && !flag_compare_debug)
     {
@@ -1117,12 +1126,9 @@  coverage_init (const char *filename)
 	{
 	  gcov_write_unsigned (GCOV_NOTE_MAGIC);
 	  gcov_write_unsigned (GCOV_VERSION);
-	  gcov_write_unsigned (local_tick);
+	  gcov_write_unsigned (bbg_file_stamp);
 	}
     }
-
-  if (flag_branch_probabilities)
-    read_counts_file ();
 }
 
 /* Performs file-level cleanup.  Close graph file, generate coverage
@@ -1133,10 +1139,11 @@  coverage_finish (void)
 {
   if (bbg_file_name && gcov_close ())
     unlink (bbg_file_name);
-  
-  if (!local_tick || local_tick == (unsigned)-1)
-    /* Only remove the da file, if we cannot stamp it.  If we can
-       stamp it, libgcov will DTRT.  */
+
+  if (!flag_branch_probabilities && flag_test_coverage
+      && (!local_tick || local_tick == (unsigned)-1))
+    /* Only remove the da file, if we're emitting coverage code and
+       cannot uniquely stamp it.  If we can stamp it, libgcov will DTRT.  */
     unlink (da_file_name);
 
   if (coverage_obj_init ())
Index: tree.c
===================================================================
--- tree.c	(revision 188900)
+++ tree.c	(working copy)
@@ -8734,23 +8734,37 @@  dump_tree_statistics (void)
 
 /* Generate a crc32 of a byte.  */
 
-unsigned
-crc32_byte (unsigned chksum, char byte)
+static unsigned
+crc32_unsigned_bits (unsigned chksum, unsigned value, unsigned bits)
 {
-  unsigned value = (unsigned) byte << 24;
-      unsigned ix;
+  unsigned ix;
 
-      for (ix = 8; ix--; value <<= 1)
-  	{
-  	  unsigned feedback;
-
-  	  feedback = (value ^ chksum) & 0x80000000 ? 0x04c11db7 : 0;
- 	  chksum <<= 1;
- 	  chksum ^= feedback;
-  	}
+  for (ix = bits; ix--; value <<= 1)
+    {
+      unsigned feedback;
+      
+      feedback = (value ^ chksum) & 0x80000000 ? 0x04c11db7 : 0;
+      chksum <<= 1;
+      chksum ^= feedback;
+    }
   return chksum;
 }
 
+/* Generate a crc32 of a 32-bit unsigned.  */
+
+unsigned
+crc32_unsigned (unsigned chksum, unsigned value)
+{
+  return crc32_unsigned_bits (chksum, value, 32);
+}
+
+/* Generate a crc32 of a byte.  */
+
+unsigned
+crc32_byte (unsigned chksum, char byte)
+{
+  return crc32_unsigned_bits (chksum, (unsigned) byte << 24, 8);
+}
 
 /* Generate a crc32 of a string.  */
 
Index: tree.h
===================================================================
--- tree.h	(revision 188900)
+++ tree.h	(working copy)
@@ -5168,6 +5168,7 @@  inlined_function_outer_scope_p (const_tr
 /* In tree.c */
 extern unsigned crc32_string (unsigned, const char *);
 extern unsigned crc32_byte (unsigned, char);
+extern unsigned crc32_unsigned (unsigned, unsigned);
 extern void clean_symbol_name (char *);
 extern tree get_file_function_name (const char *);
 extern tree get_callee_fndecl (const_tree);