This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[pph] Detect state mutation in DECLs/TYPEs [1/8] (issue5180042)


This series of patches changes the streamer cache so we can:

1- Detect when certain tree nodes have changed during parsing.  This
   is useful when generating a PPH image after reading a set of
   images included by it.  During parsing, a DECL may change from a
   forward declaration to a full definition, or a FUNCTION_DECL may
   get its body filled-in, etc.
   
   In these cases, instead of emitting an external reference to the
   declaration, we start a new (mutated) record that fills in the
   new data in the mutated object.  Currently, this means that we
   overwrite ALL the fields in the mutated object.

2- Tag cache entries with the data type of the pointed-to objects.
   This is needed to make sure we are reading the data type we expect
   to be reading.  Again, this happens when multiple PPH images are
   being read to generate another one.

   After the children PPH images have been read, the parser may
   invalidate some/most of the data in those images (e.g., when
   merging declarations).  In those cases, the memory used by the
   original object is re-used, so when we get a cache hit by
   pointer-matching, we make sure that the cache entry is for the same
   data type that we expect.  If not, we don't consider that a cache
   hit and re-pickle the pointer.

This first patch introduces signatures.  It uses libiberty's crc32
computation to sign the tree.  We only care to sign certain trees, and
only when generating a PPH from other PPHs, so we do not always need
to sign trees.

In particular, we never need to sign trees when compiling translation
units (i.e. "pure" readers).

Tested on x86_64.  Committed to branch.


Diego.

	* pph-streamer-in.c (pph_is_reference_marker): Move to
	pph-streamer.h.
 	(pph_read_namespace_tree): Call tree_needs_signature to
	determine if EXPR should be signed.
	Call pph_get_signature.
	* pph-streamer.c (pph_cache_sign): Add argument CRC.
	Change return value to void.  Update all users.
 	(pph_get_signature): New.
 	* pph-streamer.h (pph_cache_sign): Declare.
	(pph_get_signature): Declare.
	(pph_is_reference_marker): Move from pph-streamer-in.c.
 	(tree_needs_signature): New.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 8e7c772..1fd810f 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -145,18 +145,6 @@ pph_init_read (pph_stream *stream)
 }
 
 
-/* Return true if MARKER is PPH_RECORD_IREF, PPH_RECORD_XREF,
-   or PPH_RECORD_PREF.  */
-
-static inline bool
-pph_is_reference_marker (enum pph_record_marker marker)
-{
-  return marker == PPH_RECORD_IREF
-         || marker == PPH_RECORD_XREF
-         || marker == PPH_RECORD_PREF;
-}
-
-
 /* Read and return a record header from STREAM.  When a PPH_RECORD_START
    marker is read, the next word read is an index into the streamer
    cache where the rematerialized data structure should be stored.
@@ -2128,8 +2116,7 @@ pph_read_namespace_tree (pph_stream *stream, tree enclosing_namespace)
   if (tag == LTO_builtin_decl)
     {
       /* If we are going to read a built-in function, all we need is
-	 the code and class.  Note that builtins are never stored in
-         the pickle cache.  */
+	 the code and class.  */
       expr = streamer_get_builtin_tree (ib, data_in);
     }
   else if (tag == lto_tree_code_to_tag (INTEGER_CST))
@@ -2155,12 +2142,26 @@ pph_read_namespace_tree (pph_stream *stream, tree enclosing_namespace)
               expr = expr;
             }
         }
+
+      /* Add the new tree to the cache and read its body.  The tree
+         is added to the cache before we read its body to handle
+         circular references and references from children nodes.  */
       pph_cache_insert_at (&stream->cache, expr, ix);
       pph_read_tree_body (stream, expr);
 
-      /* If needed, sign the recently materialized tree to detect mutations.  */
-      if (DECL_P (expr) || TYPE_P (expr))
-        pph_cache_sign (&stream->cache, ix, tree_size (expr));
+      /* If needed, sign the recently materialized tree to detect
+         mutations.  Note that we only need to compute signatures
+         if we are generating a PPH image.  That is the only time
+         where we need to determine whether a tree read from PPH
+         was updated while parsing the header file that we are
+         currently generating.  */
+      if (pph_writer_enabled_p () && tree_needs_signature (expr))
+        {
+          unsigned crc;
+          size_t nbytes;
+          crc = pph_get_signature (expr, &nbytes);
+          pph_cache_sign (&stream->cache, ix, crc, nbytes);
+        }
     }
 
   return expr;
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index 668f96c..d0fac57 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -533,20 +533,58 @@ pph_cache_add (pph_cache *cache, void *data, unsigned *ix_p)
 }
 
 
-/* Generate a CRC32 signature for the first NBYTES of the area memory
-   pointed to by slot IX of CACHE.  The signature is stored in
-   CACHE[IX] and returned.  */
+/* Associate signature CRC with the first NBYTES of the area memory
+   pointed to by slot IX of CACHE.  */
 
-unsigned
-pph_cache_sign (pph_cache *cache, unsigned ix, size_t nbytes)
+void
+pph_cache_sign (pph_cache *cache, unsigned ix, unsigned crc, size_t nbytes)
 {
   pph_cache_entry *e;
 
   gcc_assert (nbytes == (size_t) (int) nbytes);
 
   e = pph_cache_get_entry (cache, ix);
-  e->crc = xcrc32 ((const unsigned char *) e->data, nbytes, -1);
+  e->crc = crc;
   e->crc_nbytes = nbytes;
+}
+
 
-  return e->crc;
+/* Return a signature for tree T.  Store the length of the signed area
+   in *NBYTES_P.  */
+
+unsigned
+pph_get_signature (tree t, size_t *nbytes_p)
+{
+  tree prev_chain = NULL;
+  rtx prev_rtl = NULL;
+  int prev_used;
+  size_t nbytes;
+  unsigned crc;
+
+  nbytes = tree_size (t);
+  if (nbytes_p)
+    *nbytes_p = nbytes;
+
+  /* Preserve the value of the fields not included in the signature.  */
+  prev_chain = (DECL_P (t)) ? DECL_CHAIN (t) : NULL;
+  prev_rtl = (HAS_RTL_P (t)) ? DECL_RTL_IF_SET (t) : NULL;
+  prev_used = TREE_USED (t);
+
+  /* Clear the fields not included in the signature.  */
+  if (DECL_P (t))
+    DECL_CHAIN (t) = NULL;
+  if (HAS_RTL_P (t))
+    SET_DECL_RTL (t, NULL);
+  TREE_USED (t) = 0;
+
+  crc = xcrc32 ((const unsigned char *) t, nbytes, -1);
+
+  /* Restore fields we did not include in the signature.  */
+  if (DECL_P (t))
+    DECL_CHAIN (t) = prev_chain;
+  if (HAS_RTL_P (t))
+    SET_DECL_RTL (t, prev_rtl);
+  TREE_USED (t) = prev_used;
+
+  return crc;
 }
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index cefa1b3..9adc908 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -124,7 +124,7 @@ typedef struct pph_file_header {
    converted into their definition.
 
    When the cache notices a cache hit on a mutated data, it writes a
-   PPH_RECORD_MUTATED_REF to indicate to the reader that it is about
+   PPH_RECORD_MREF to indicate to the reader that it is about
    to read an already instantiated tree.  */
 typedef struct pph_cache_entry {
   /* Pointer to cached data.  */
@@ -289,7 +289,8 @@ void pph_cache_insert_at (pph_cache *, void *, unsigned);
 bool pph_cache_lookup (pph_cache *, void *, unsigned *);
 bool pph_cache_lookup_in_includes (void *, unsigned *, unsigned *);
 bool pph_cache_add (pph_cache *, void *, unsigned *);
-unsigned pph_cache_sign (pph_cache *, unsigned, size_t);
+void pph_cache_sign (pph_cache *, unsigned, unsigned, size_t);
+unsigned pph_get_signature (tree, size_t *);
 
 /* In pph-streamer-out.c.  */
 void pph_flush_buffers (pph_stream *);
@@ -715,4 +716,42 @@ pph_in_record_marker (pph_stream *stream)
   return m;
 }
 
+
+/* Return true if MARKER is PPH_RECORD_IREF, PPH_RECORD_XREF,
+   or PPH_RECORD_PREF.  */
+
+static inline bool
+pph_is_reference_marker (enum pph_record_marker marker)
+{
+  return marker == PPH_RECORD_IREF
+         || marker == PPH_RECORD_XREF
+         || marker == PPH_RECORD_PREF;
+}
+
+
+/* Return true if tree T needs to be signed to detect state mutations.
+   This is used when multiple PPH images contain different versions of
+   the same tree node (e.g., decl.pph contains the declaration of
+   function F while impl.pph contains its definition).
+
+   When generating the image for impl.pph, we will read F's
+   declaration from decl.pph.  This becomes an external reference.
+   When we go to write F in impl.pph, the cache will find the external
+   reference to F in decl.pph and write it.
+
+   This causes us to lose all the information added to F's node in
+   impl.h (its body, return value, etc).  So, a translation unit
+   reading impl.pph will never get that data and compilation will
+   fail.
+
+   We notice state mutations by computing CRC signatures on the body
+   of trees.  The first signature is computed when the tree is read
+   from an image.  The second signature is computed when we go to write
+   the tree again (pph_out_start_tree_record).  */
+static inline bool
+tree_needs_signature (tree t)
+{
+  return DECL_P (t) || TYPE_P (t);
+}
+
 #endif  /* GCC_CP_PPH_STREAMER_H  */
-- 
1.7.3.1


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]