I really like the way this is implemented! In particular having our
own cache is so great!
A few comments inline below.
Cheers,
Gab
On Mon, Aug 15, 2011 at 8:05 PM, Diego Novillo <dnovi...@google.com> wrote:
> This patch finishes the support for external references to symbols in
> other PPH files.
>
> This is used when one PPH image includes another. The symbols in
> the included image should not be emitted from the parent, and references
> to them should go to the correct location in the child's pickle cache.
>
> The main change is the addition of two kinds of references. We used
> to mark references to already pickled nodes with PPH_RECORD_SHARED.
> We now distinguish internal (PPH_RECORD_IREF) and external
> (PPH_RECORD_XREF) references.
>
> In the first variant, only one index is written as reference: the slot
> into the pickle cache. In the second variant, two indices are
> written: an index into the include table specifying which child image
> contains the data, and an index into the child's pickle cache.
>
> When writing PPH images, we also need to make sure that we do not
> write any symbols in the file bindings that belong to included images.
> Otherwise, we will emit the same symbol twice. This is implemented by
> a new filter PPHF_NO_XREFS, which is used when writing the current
> image's global bindings.
>
> Tested on x86_64. Committed to branch.
>
>
> Diego.
>
>
> cp/ChangeLog.pph
> * name-lookup.c (pph_out_binding_table): Call
> pph_out_record_marker instead of pph_out_uchar.
> (pph_in_binding_table): Call pph_in_record_marker instead of
> pph_in_uchar.
> * pph-streamer-in.c (pph_read_images): Declare.
> (pph_is_reference_marker): New. Update all previous users of
> PPH_RECORD_SHARED.
> (pph_in_start_record): Add argument INCLUDE_IX. Update all
> users.
> Handle PPH_RECORD_XREF markers.
> (pph_in_includes): After reading INCLUDE_NAME, call
> pph_add_include.
> (pph_read_file_contents): Move debugging output from ...
> (pph_read_file): ... here.
> Change it to return the opened stream.
> Add STREAM to pph_read_images.
> (pph_reader_finish): New.
> (pph_read_file_1): Rename from pph_read_file_contents. Update
> all users.
> * pph-streamer-out.c (pph_out_start_record): Re-write to
> handle PPH_RECORD_IREF and PPH_RECORD_XREF.
> (pph_write_file_contents): Embed ...
> (pph_write_file): ... here.
> (pph_add_include): Add new argument INCLUDE. Update all
> users.
> (pph_writer_finish): Do nothing if pph_out_stream is NULL.
> (pph_tree_matches): New.
> (pph_out_tree_vec_filtered): New.
> (pph_out_binding_level): Add new argument FILTER. Update all users.
> * pph-streamer.c (pph_stream_close_1): Embed ...
> (pph_stream_close): ... here.
> Do not traverse the list of includes.
> (pph_cache_lookup): Factor out of ...
> (pph_cache_add): ... here.
> (pph_cache_lookup_in_includes): New.
> (pph_cache_get): Add new argument INCLUDE_IX. Update all users.
> * pph-streamer.h (enum pph_record_marker): Add PPH_RECORD_IREF
> and PPH_RECORD_XREF. Remove PPH_RECORD_SHARED. Update all users.
> (struct pph_stream): Remove field open_p and nested_p. Update
> all users.
> (enum chain_filter): Remove. Change values to #define constants.
> Update all users.
> (PPHF_NO_XREFS): Define.
> (pph_out_record_marker): New.
> (pph_in_record_marker): New.
> * pph.c (pph_finish): Always call pph_writer_finish.
> Call pph_reader_finish.
>
> testsuite/ChangeLog.pph
> * g++.dg/pph/x1tmplclass2.cc: Update expected ICE.
> * g++.dg/pph/x4tmplclass2.cc: Update expected ICE.
> * g++.dg/pph/z4tmplclass2.cc: Update expected ICE.
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 50c6b66..7798f24 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -6002,12 +6002,12 @@ pph_out_binding_table (pph_stream *stream,
> binding_table bt)
> {
> if (bt->chain[i])
> {
> - pph_out_uchar (stream, PPH_RECORD_START);
> + pph_out_record_marker (stream, PPH_RECORD_START);
> pph_out_tree_or_ref (stream, bt->chain[i]->name);
> pph_out_tree_or_ref (stream, bt->chain[i]->type);
> }
> else
> - pph_out_uchar (stream, PPH_RECORD_END);
> + pph_out_record_marker (stream, PPH_RECORD_END);
> }
> pph_out_uint (stream, bt->entry_count);
> }
> @@ -6025,13 +6025,15 @@ pph_in_binding_table (pph_stream *stream)
> bt = binding_table_new (chain_count);
> for (i = 0; i < chain_count; i++)
> {
> - unsigned char record_marker = pph_in_uchar (stream);
> - if (record_marker == PPH_RECORD_START)
> + enum pph_record_marker marker = pph_in_record_marker (stream);
> + if (marker == PPH_RECORD_START)
> {
> tree name = pph_in_tree (stream);
> tree type = pph_in_tree (stream);
> binding_table_insert (bt, name, type);
> }
> + else
> + gcc_assert (marker == PPH_RECORD_END);
> }
> bt->entry_count = pph_in_uint (stream);
>
> diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
> index e644113..f87e6a5 100644
> --- a/gcc/cp/pph-streamer-in.c
> +++ b/gcc/cp/pph-streamer-in.c
> @@ -33,6 +33,13 @@ along with GCC; see the file COPYING3. If not see
> #include "cppbuiltin.h"
> #include "toplev.h"
>
> +/* List of PPH images read during parsing. Images opened during #include
> + processing and opened from pph_in_includes cannot be closed
> + immediately after reading, because the pickle cache contained in
> + them may be referenced from other images. We delay closing all of
> + them until the end of parsing (when pph_reader_finish is called). */
> +static VEC(pph_stream_ptr,heap) *pph_read_images = NULL;
> +
> typedef char *char_p;
> DEF_VEC_P(char_p);
> DEF_VEC_ALLOC_P(char_p,heap);
> @@ -135,43 +142,57 @@ pph_init_read (pph_stream *stream)
> }
>
>
> -/* Read and return a record marker from STREAM. When a PPH_RECORD_START
> +/* Return true if MARKER is either PPH_RECORD_IREF or PPH_RECORD_XREF. */
> +
> +static inline bool
> +pph_is_reference_marker (enum pph_record_marker marker)
> +{
> + return marker == PPH_RECORD_IREF || marker == PPH_RECORD_XREF;
> +}
> +
> +
> +/* 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.
> When the writer stored this data structure for the first time, it
> - added it to its own streamer cache at slot number *CACHE_IX.
> + added it to its own streamer cache at slot number *CACHE_IX_P.
>
> This way, if the same data structure was written a second time to
> the stream, instead of writing the whole structure again, only the
> - index *CACHE_IX is written as a PPH_RECORD_SHARED record.
> + index *CACHE_IX_P is written as a PPH_RECORD_IREF record.
>
> - Therefore, when reading a PPH_RECORD_START marker, *CACHE_IX will
> + Therefore, when reading a PPH_RECORD_START marker, *CACHE_IX_P will
> contain the slot number where the materialized data should be
> - cached at. When reading a PPH_RECORD_SHARED marker, *CACHE_IX will
> + cached at. When reading a PPH_RECORD_IREF marker, *CACHE_IX_P will
> contain the slot number the reader can find the previously
> - materialized structure. */
> + materialized structure.
> +
> + If the record starts with PPH_RECORD_XREF, this means that the data
> + we are about to read is located in the pickle cache of one of
> + STREAM's included images. In this case, the record consists of two
> + indices: the first one (*INCLUDE_IX_P) indicates which included
> + image contains the data (it is an index into STREAM->INCLUDES), the
> + second one indicates which slot in that image's pickle cache we can
> + find the data. */
>
> static inline enum pph_record_marker
> -pph_in_start_record (pph_stream *stream, unsigned *cache_ix)
> +pph_in_start_record (pph_stream *stream, unsigned *include_ix_p,
> + unsigned *cache_ix_p)
> {
> - enum pph_record_marker marker;
> + enum pph_record_marker marker = pph_in_record_marker (stream);
>
> - marker = (enum pph_record_marker) pph_in_uchar (stream);
> + *include_ix_p = (unsigned) -1;
> + *cache_ix_p = (unsigned) -1;
Setting *cache_ix to (unsigned) -1 used to be a "hack" (with a comment
explaining it which was removed just below), to avoid a warning about
it being unset in a branch, but that branch was only PPH_RECORD_END,
in which case it didn't matter.
Now that we actually check in pph_cache_get that include_ix ==
(unsigned) -1, I'm not so sure this is good... while (unsigned) -1 is
a very large unsigned int, it's not impossible to have an actual cache
entry with that number...
> - /* For PPH_RECORD_START and PPH_RECORD_SHARED markers, read the
> + /* For PPH_RECORD_START and PPH_RECORD_IREF markers, read the
> streamer cache slot where we should store or find the
> rematerialized data structure (see description above). */
> - if (marker == PPH_RECORD_START || marker == PPH_RECORD_SHARED)
> - *cache_ix = pph_in_uint (stream);
> - else
> + if (marker == PPH_RECORD_START || marker == PPH_RECORD_IREF)
> + *cache_ix_p = pph_in_uint (stream);
> + else if (marker == PPH_RECORD_XREF)
> {
> - gcc_assert (marker == PPH_RECORD_END);
> -
> - /* Initialize CACHE_IX to an invalid index. Even though this
> - is never used in practice, the compiler will throw an error
> - if the optimizer inlines this function in a given build as
> - it will complain that " 'ix' may be used uninitialized". */
...this is the comment I'm referring to above.
> - *cache_ix = -1;
> + *include_ix_p = pph_in_uint (stream);
> + *cache_ix_p = pph_in_uint (stream);
> }
>
> return marker;
> @@ -457,13 +478,13 @@ pph_in_cxx_binding_1 (pph_stream *stream)
> cxx_binding *cb;
> tree value, type;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned ix, include_ix;
Sometimes you call the local variable "include_ix"...
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &include_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
> - else if (marker == PPH_RECORD_SHARED)
> - return (cxx_binding *) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + return (cxx_binding *) pph_cache_get (stream, include_ix, ix);
>
> value = pph_in_tree (stream);
> type = pph_in_tree (stream);
> @@ -505,13 +526,13 @@ pph_in_class_binding (pph_stream *stream)
> {
> cp_class_binding *cb;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned image_ix, ix;
... and sometimes image_ix: consistency would be nice, although not necessary...
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
> - else if (marker == PPH_RECORD_SHARED)
> - return (cp_class_binding *) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + return (cp_class_binding *) pph_cache_get (stream, image_ix, ix);
>
> ALLOC_AND_REGISTER (stream, ix, cb, ggc_alloc_cleared_cp_class_binding ());
> cb->base = pph_in_cxx_binding (stream);
> @@ -528,13 +549,13 @@ pph_in_label_binding (pph_stream *stream)
> {
> cp_label_binding *lb;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned image_ix, ix;
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
> - else if (marker == PPH_RECORD_SHARED)
> - return (cp_label_binding *) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + return (cp_label_binding *) pph_cache_get (stream, image_ix, ix);
>
> ALLOC_AND_REGISTER (stream, ix, lb, ggc_alloc_cleared_cp_label_binding ());
> lb->label = pph_in_tree (stream);
> @@ -565,16 +586,16 @@ pph_in_label_binding (pph_stream *stream)
> static cp_binding_level *
> pph_in_binding_level (pph_stream *stream, cp_binding_level *to_register)
> {
> - unsigned i, num, ix;
> + unsigned i, num, image_ix, ix;
> cp_binding_level *bl;
> struct bitpack_d bp;
> enum pph_record_marker marker;
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
> - else if (marker == PPH_RECORD_SHARED)
> - return (cp_binding_level *) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + return (cp_binding_level *) pph_cache_get (stream, image_ix, ix);
>
> /* If TO_REGISTER is set, register that binding level instead of the newly
> allocated binding level into slot IX. */
> @@ -644,13 +665,13 @@ pph_in_c_language_function (pph_stream *stream)
> {
> struct c_language_function *clf;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned image_ix, ix;
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
> - else if (marker == PPH_RECORD_SHARED)
> - return (struct c_language_function *) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + return (struct c_language_function *) pph_cache_get (stream, image_ix,
> ix);
>
> ALLOC_AND_REGISTER (stream, ix, clf,
> ggc_alloc_cleared_c_language_function ());
> @@ -669,13 +690,13 @@ pph_in_language_function (pph_stream *stream)
> struct bitpack_d bp;
> struct language_function *lf;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned image_ix, ix;
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
> - else if (marker == PPH_RECORD_SHARED)
> - return (struct language_function *) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + return (struct language_function *) pph_cache_get (stream, image_ix, ix);
>
> ALLOC_AND_REGISTER (stream, ix, lf, ggc_alloc_cleared_language_function ());
> memcpy (&lf->base, pph_in_c_language_function (stream),
> @@ -759,17 +780,17 @@ static struct function *
> pph_in_struct_function (pph_stream *stream)
> {
> size_t count, i;
> - unsigned ix;
> + unsigned image_ix, ix;
> enum pph_record_marker marker;
> struct function *fn;
> tree decl;
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
>
> /* Since struct function is embedded in every decl, fn cannot be shared. */
> - gcc_assert (marker != PPH_RECORD_SHARED);
> + gcc_assert (!pph_is_reference_marker (marker));
>
> decl = pph_in_tree (stream);
> allocate_struct_function (decl, false);
> @@ -864,15 +885,15 @@ pph_in_lang_specific (pph_stream *stream, tree decl)
> struct lang_decl *ld;
> struct lang_decl_base *ldb;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned image_ix, ix;
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return;
> - else if (marker == PPH_RECORD_SHARED)
> + else if (pph_is_reference_marker (marker))
> {
> DECL_LANG_SPECIFIC (decl) =
> - (struct lang_decl *) pph_cache_get (stream, ix);
> + (struct lang_decl *) pph_cache_get (stream, image_ix, ix);
> return;
> }
>
> @@ -960,13 +981,13 @@ pph_in_sorted_fields_type (pph_stream *stream)
> unsigned i, num_fields;
> struct sorted_fields_type *v;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned image_ix, ix;
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
> - else if (marker == PPH_RECORD_SHARED)
> - return (struct sorted_fields_type *) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + return (struct sorted_fields_type *) pph_cache_get (stream, image_ix,
> ix);
>
> num_fields = pph_in_uint (stream);
> ALLOC_AND_REGISTER (stream, ix, v, sorted_fields_type_new (num_fields));
> @@ -984,7 +1005,7 @@ pph_in_lang_type_class (pph_stream *stream, struct
> lang_type_class *ltc)
> {
> struct bitpack_d bp;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned image_ix, ix;
>
> ltc->align = pph_in_uchar (stream);
>
> @@ -1039,14 +1060,14 @@ pph_in_lang_type_class (pph_stream *stream, struct
> lang_type_class *ltc)
> ltc->typeinfo_var = pph_in_tree (stream);
> ltc->vbases = pph_in_tree_vec (stream);
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_START)
> {
> ltc->nested_udts = pph_in_binding_table (stream);
> pph_cache_insert_at (stream, ltc->nested_udts, ix);
> }
> - else if (marker == PPH_RECORD_SHARED)
> - ltc->nested_udts = (binding_table) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + ltc->nested_udts = (binding_table) pph_cache_get (stream, image_ix, ix);
>
> ltc->as_base = pph_in_tree (stream);
> ltc->pure_virtuals = pph_in_tree_vec (stream);
> @@ -1079,13 +1100,13 @@ pph_in_lang_type (pph_stream *stream)
> {
> struct lang_type *lt;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned image_ix, ix;
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
> - else if (marker == PPH_RECORD_SHARED)
> - return (struct lang_type *) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + return (struct lang_type *) pph_cache_get (stream, image_ix, ix);
>
> ALLOC_AND_REGISTER (stream, ix, lt,
> ggc_alloc_cleared_lang_type (sizeof (struct lang_type)));
> @@ -1346,10 +1367,8 @@ pph_in_includes (pph_stream *stream)
> for (i = 0; i < num; i++)
> {
> const char *include_name = pph_in_string (stream);
> - /* FIXME pph. Disabled for now. Need to re-work caching so
> - external symbol references are properly saved and restored. */
> - if (0)
> - pph_read_file (include_name);
> + pph_stream *include = pph_read_file (include_name);
> + pph_add_include (stream, include);
> }
> }
>
> @@ -1454,10 +1473,10 @@ pph_in_and_merge_line_table (pph_stream *stream,
> struct line_maps *linetab)
> }
>
>
> -/* Read contents of PPH file in STREAM. */
> +/* Helper for pph_read_file. Read contents of PPH file in STREAM. */
>
> static void
> -pph_read_file_contents (pph_stream *stream)
> +pph_read_file_1 (pph_stream *stream)
> {
> bool verified;
> cpp_ident_use *bad_use;
> @@ -1467,6 +1486,9 @@ pph_read_file_contents (pph_stream *stream)
> unsigned i;
> VEC(tree,gc) *file_unemitted_tinfo_decls;
>
> + if (flag_pph_debug >= 1)
> + fprintf (pph_logfile, "PPH: Reading %s\n", stream->name);
> +
> /* Read all the images included by STREAM. */
> pph_in_includes (stream);
>
> @@ -1483,7 +1505,7 @@ pph_read_file_contents (pph_stream *stream)
> /* Re-instantiate all the pre-processor symbols defined by STREAM. */
> cpp_lt_replay (parse_in, &idents_used);
>
> - /* Read in the pph's line table and merge it in the current line table. */
> + /* Read in STREAM's line table and merge it in the current line table. */
> pph_in_and_merge_line_table (stream, line_table);
>
> /* Read the bindings from STREAM and merge them with the current bindings.
> */
> @@ -1514,28 +1536,27 @@ pph_read_file_contents (pph_stream *stream)
> STREAM will need to be read again the next time we want to read
> the image we are now generating. */
> if (pph_out_file)
> - pph_add_include (stream);
> + pph_add_include (NULL, stream);
> }
>
>
> -/* Read PPH file FILENAME. */
> +/* Read PPH file FILENAME. Return the in-memory pph_stream instance. */
>
> -void
> +pph_stream *
> pph_read_file (const char *filename)
> {
> pph_stream *stream;
>
> - if (flag_pph_debug >= 1)
> - fprintf (pph_logfile, "PPH: Reading %s\n", filename);
> -
> stream = pph_stream_open (filename, "rb");
> if (stream)
> {
> - pph_read_file_contents (stream);
> - pph_stream_close (stream);
> + pph_read_file_1 (stream);
> + VEC_safe_push (pph_stream_ptr, heap, pph_read_images, stream);
> }
> else
> error ("Cannot open PPH file for reading: %s: %m", filename);
> +
> + return stream;
> }
>
>
> @@ -1981,13 +2002,13 @@ pph_read_tree (struct lto_input_block *ib
> ATTRIBUTE_UNUSED,
> tree expr;
> bool fully_read_p;
> enum pph_record_marker marker;
> - unsigned ix;
> + unsigned image_ix, ix;
>
> - marker = pph_in_start_record (stream, &ix);
> + marker = pph_in_start_record (stream, &image_ix, &ix);
> if (marker == PPH_RECORD_END)
> return NULL;
> - else if (marker == PPH_RECORD_SHARED)
> - return (tree) pph_cache_get (stream, ix);
> + else if (pph_is_reference_marker (marker))
> + return (tree) pph_cache_get (stream, image_ix, ix);
>
> /* We did not find the tree in the pickle cache, allocate the tree by
> reading the header fields (different tree nodes need to be
> @@ -1998,3 +2019,17 @@ pph_read_tree (struct lto_input_block *ib
> ATTRIBUTE_UNUSED,
>
> return expr;
> }
> +
> +
> +/* Finalize the PPH reader. */
> +
> +void
> +pph_reader_finish (void)
> +{
> + unsigned i;
> + pph_stream *image;
> +
> + /* Close any images read during parsing. */
> + FOR_EACH_VEC_ELT (pph_stream_ptr, pph_read_images, i, image)
> + pph_stream_close (image);
> +}
> diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
> index 2e5543a..b7a965c 100644
> --- a/gcc/cp/pph-streamer-out.c
> +++ b/gcc/cp/pph-streamer-out.c
> @@ -180,32 +180,43 @@ pph_flush_buffers (pph_stream *stream)
> static inline bool
> pph_out_start_record (pph_stream *stream, void *data)
> {
> - if (data)
> + unsigned ix, include_ix;
> +
> + /* Represent NULL pointers with a single PPH_RECORD_END. */
> + if (data == NULL)
> + {
> + pph_out_record_marker (stream, PPH_RECORD_END);
> + return false;
> + }
> +
> + /* See if we have data in STREAM's cache. If so, write an internal
> + reference to it and inform the caller that it should not write a
> + physical representation for DATA. */
> + if (pph_cache_lookup (stream, data, &ix))
> {
> - bool existed_p;
> - unsigned ix;
> - enum pph_record_marker marker;
> -
> - /* If the memory at DATA has already been streamed out, make
> - sure that we don't write it more than once. Otherwise,
> - the reader will instantiate two different pointers for
> - the same object.
> -
> - Write the index into the cache where DATA has been stored.
> - This way, the reader will know at which slot to
> - re-materialize DATA the first time and where to access it on
> - subsequent reads. */
> - existed_p = pph_cache_add (stream, data, &ix);
> - marker = (existed_p) ? PPH_RECORD_SHARED : PPH_RECORD_START;
> - pph_out_uchar (stream, marker);
> + pph_out_record_marker (stream, PPH_RECORD_IREF);
> pph_out_uint (stream, ix);
> - return marker == PPH_RECORD_START;
> + return false;
> }
> - else
> +
> + /* DATA is not in STREAM's cache. See if it is in any of STREAM's
> + included images. If it is, write an external reference to it
> + and inform the caller that it should not write a physical
> + representation for DATA. */
> + if (pph_cache_lookup_in_includes (stream, data, &include_ix, &ix))
> {
> - pph_out_uchar (stream, PPH_RECORD_END);
> + pph_out_record_marker (stream, PPH_RECORD_XREF);
> + pph_out_uint (stream, include_ix);
> + pph_out_uint (stream, ix);
> return false;
> }
> +
> + /* DATA is in none of the pickle caches, add it to STREAM's pickle
> + cache and tell the caller that it should pickle DATA out. */
> + pph_cache_add (stream, data, &ix);
> + pph_out_record_marker (stream, PPH_RECORD_START);
> + pph_out_uint (stream, ix);
> + return true;
> }
>
>
> @@ -449,7 +460,25 @@ pph_out_ld_min (pph_stream *stream, struct lang_decl_min
> *ldm)
> }
>
>
> -/* Write all the trees in gc VEC V to STREAM. */
> +/* Return true if T matches FILTER for STREAM. */
> +
> +static inline bool
> +pph_tree_matches (pph_stream *stream, tree t, unsigned filter)
> +{
> + if ((filter & PPHF_NO_BUILTINS)
> + && DECL_P (t)
> + && DECL_IS_BUILTIN (t))
> + return false;
> +
> + if ((filter & PPHF_NO_XREFS)
> + && pph_cache_lookup_in_includes (stream, t, NULL, NULL))
> + return false;
> +
> + return true;
> +}
> +
> +
> +/* Write all the trees in VEC V to STREAM. */
>
> static void
> pph_out_tree_vec (pph_stream *stream, VEC(tree,gc) *v)
> @@ -463,6 +492,34 @@ pph_out_tree_vec (pph_stream *stream, VEC(tree,gc) *v)
> }
>
>
> +/* Write all the trees matching FILTER in VEC V to STREAM. */
> +
> +static void
> +pph_out_tree_vec_filtered (pph_stream *stream, VEC(tree,gc) *v, unsigned
> filter)
> +{
> + unsigned i;
> + tree t;
> + VEC(tree, heap) *to_write = NULL;
> +
> + /* Special case. If the caller wants no filtering, it is much
> + faster to just call pph_out_tree_vec. */
> + if (filter == PPHF_NONE)
> + {
> + pph_out_tree_vec (stream, v);
> + return;
> + }
> +
> + /* Collect all the nodes that match the filter. */
> + FOR_EACH_VEC_ELT (tree, v, i, t)
> + if (pph_tree_matches (stream, t, filter))
> + VEC_safe_push (tree, heap, to_write, t);
> +
> + /* Write them. */
> + pph_out_tree_vec (stream, (VEC(tree,gc) *)to_write);
> + VEC_free (tree, heap, to_write);
> +}
> +
> +
> /* Write all the qualified_typedef_usage_t in VEC V to STREAM. */
>
> static void
> @@ -482,7 +539,7 @@ pph_out_qual_use_vec (pph_stream *stream,
> VEC(qualified_typedef_usage_t,gc) *v)
>
>
> /* Forward declaration to break cyclic dependencies. */
> -static void pph_out_binding_level (pph_stream *, cp_binding_level *);
> +static void pph_out_binding_level (pph_stream *, cp_binding_level *,
> unsigned);
>
>
> /* Helper for pph_out_cxx_binding. STREAM and CB are as in
> @@ -498,7 +555,7 @@ pph_out_cxx_binding_1 (pph_stream *stream, cxx_binding
> *cb)
>
> pph_out_tree_or_ref (stream, cb->value);
> pph_out_tree_or_ref (stream, cb->type);
> - pph_out_binding_level (stream, cb->scope);
> + pph_out_binding_level (stream, cb->scope, PPHF_NONE);
> bp = bitpack_create (stream->encoder.w.ob->main_stream);
> bp_pack_value (&bp, cb->value_is_inherited, 1);
> bp_pack_value (&bp, cb->is_local, 1);
> @@ -573,61 +630,51 @@ pph_out_chained_tree (pph_stream *stream, tree t)
> nodes that do not match FILTER. */
>
> static void
> -pph_out_chain_filtered (pph_stream *stream, tree first,
> - enum chain_filter filter)
> +pph_out_chain_filtered (pph_stream *stream, tree first, unsigned filter)
> {
> - unsigned count;
> tree t;
> + VEC(tree, heap) *to_write = NULL;
>
> /* Special case. If the caller wants no filtering, it is much
> faster to just call pph_out_chain directly. */
> - if (filter == NONE)
> + if (filter == PPHF_NONE)
> {
> pph_out_chain (stream, first);
> return;
> }
>
> - /* Count all the nodes that match the filter. */
> - for (t = first, count = 0; t; t = TREE_CHAIN (t))
> - {
> - if (filter == NO_BUILTINS && DECL_P (t) && DECL_IS_BUILTIN (t))
> - continue;
> - count++;
> - }
> - pph_out_uint (stream, count);
> -
> - /* Output all the nodes that match the filter. */
> + /* Collect all the nodes that match the filter. */
> for (t = first; t; t = TREE_CHAIN (t))
> - {
> - /* Apply filters to T. */
> - if (filter == NO_BUILTINS && DECL_P (t) && DECL_IS_BUILTIN (t))
> - continue;
> + if (pph_tree_matches (stream, t, filter))
> + VEC_safe_push (tree, heap, to_write, t);
>
> - pph_out_chained_tree (stream, t);
> - }
> + /* Write them. */
> + pph_out_tree_vec (stream, (VEC(tree,gc) *)to_write);
> + VEC_free (tree, heap, to_write);
> }
>
>
> -/* Write all the fields of cp_binding_level instance BL to STREAM. */
> +/* Helper for pph_out_binding_level. Write all the fields of BL to
> + STREAM, without checking whether BL was in the streamer cache or not.
> + Do not emit any nodes in BL that do not match FILTER. */
>
> static void
> -pph_out_binding_level (pph_stream *stream, cp_binding_level *bl)
> +pph_out_binding_level_1 (pph_stream *stream, cp_binding_level *bl,
> + unsigned filter)
> {
> unsigned i;
> cp_class_binding *cs;
> cp_label_binding *sl;
> struct bitpack_d bp;
>
> - if (!pph_out_start_record (stream, bl))
> - return;
> -
> - pph_out_chain_filtered (stream, bl->names, NO_BUILTINS);
> - pph_out_chain_filtered (stream, bl->namespaces, NO_BUILTINS);
> + pph_out_chain_filtered (stream, bl->names, PPHF_NO_BUILTINS | filter);
> + pph_out_chain_filtered (stream, bl->namespaces, PPHF_NO_BUILTINS | filter);
>
> - pph_out_tree_vec (stream, bl->static_decls);
> + pph_out_tree_vec_filtered (stream, bl->static_decls, filter);
>
> - pph_out_chain_filtered (stream, bl->usings, NO_BUILTINS);
> - pph_out_chain_filtered (stream, bl->using_directives, NO_BUILTINS);
> + pph_out_chain_filtered (stream, bl->usings, PPHF_NO_BUILTINS | filter);
> + pph_out_chain_filtered (stream, bl->using_directives,
> + PPHF_NO_BUILTINS | filter);
>
> pph_out_uint (stream, VEC_length (cp_class_binding, bl->class_shadowed));
> FOR_EACH_VEC_ELT (cp_class_binding, bl->class_shadowed, i, cs)
> @@ -641,7 +688,7 @@ pph_out_binding_level (pph_stream *stream,
> cp_binding_level *bl)
>
> pph_out_chain (stream, bl->blocks);
> pph_out_tree_or_ref (stream, bl->this_entity);
> - pph_out_binding_level (stream, bl->level_chain);
> + pph_out_binding_level (stream, bl->level_chain, filter);
> pph_out_tree_vec (stream, bl->dead_vars_from_for);
> pph_out_chain (stream, bl->statement_list);
> pph_out_uint (stream, bl->binding_depth);
> @@ -655,6 +702,20 @@ pph_out_binding_level (pph_stream *stream,
> cp_binding_level *bl)
> }
>
>
> +/* Write all the fields of cp_binding_level instance BL to STREAM. Do not
> + emit any nodes in BL that do not match FILTER. */
> +
> +static void
> +pph_out_binding_level (pph_stream *stream, cp_binding_level *bl,
> + unsigned filter)
> +{
> + if (!pph_out_start_record (stream, bl))
> + return;
> +
> + pph_out_binding_level_1 (stream, bl, filter);
> +}
> +
> +
> /* Write out the tree_common fields from T to STREAM. */
>
> static void
> @@ -708,7 +769,7 @@ pph_out_language_function (pph_stream *stream, struct
> language_function *lf)
>
> /* FIXME pph. We are not writing lf->x_named_labels. */
>
> - pph_out_binding_level (stream, lf->bindings);
> + pph_out_binding_level (stream, lf->bindings, PPHF_NONE);
> pph_out_tree_vec (stream, lf->x_local_names);
>
> /* FIXME pph. We are not writing lf->extern_decl_map. */
> @@ -847,7 +908,7 @@ pph_out_struct_function (pph_stream *stream, struct
> function *fn)
> static void
> pph_out_ld_ns (pph_stream *stream, struct lang_decl_ns *ldns)
> {
> - pph_out_binding_level (stream, ldns->level);
> + pph_out_binding_level (stream, ldns->level, PPHF_NONE);
> }
>
>
> @@ -1058,36 +1119,46 @@ pph_out_lang_type (pph_stream *stream, tree type)
> }
>
>
> -/* Write saved_scope information stored in SS into STREAM.
> - This does NOT output all fields, it is meant to be used for the
> - global variable scope_chain only. */
> +/* Write the global bindings in scope_chain to STREAM. */
>
> static void
> -pph_out_scope_chain (pph_stream *stream, struct saved_scope *ss)
> +pph_out_scope_chain (pph_stream *stream)
> {
> /* old_namespace should be global_namespace and all entries listed below
> should be NULL or 0; otherwise the header parsed was incomplete. */
> - gcc_assert (ss->old_namespace == global_namespace
> - && !(ss->class_name
> - || ss->class_type
> - || ss->access_specifier
> - || ss->function_decl
> - || ss->template_parms
> - || ss->x_saved_tree
> - || ss->class_bindings
> - || ss->prev
> - || ss->unevaluated_operand
> - || ss->inhibit_evaluation_warnings
> - || ss->x_processing_template_decl
> - || ss->x_processing_specialization
> - || ss->x_processing_explicit_instantiation
> - || ss->need_pop_function_context
> - || ss->x_stmt_tree.x_cur_stmt_list
> - || ss->x_stmt_tree.stmts_are_full_exprs_p));
> + gcc_assert (scope_chain->old_namespace == global_namespace
> + && !(scope_chain->class_name
> + || scope_chain->class_type
> + || scope_chain->access_specifier
> + || scope_chain->function_decl
> + || scope_chain->template_parms
> + || scope_chain->x_saved_tree
> + || scope_chain->class_bindings
> + || scope_chain->prev
> + || scope_chain->unevaluated_operand
> + || scope_chain->inhibit_evaluation_warnings
> + || scope_chain->x_processing_template_decl
> + || scope_chain->x_processing_specialization
> + || scope_chain->x_processing_explicit_instantiation
> + || scope_chain->need_pop_function_context
> + || scope_chain->x_stmt_tree.x_cur_stmt_list
> + || scope_chain->x_stmt_tree.stmts_are_full_exprs_p));
>
> /* We only need to write out the bindings, everything else should
> - be NULL or be some temporary disposable state. */
> - pph_out_binding_level (stream, ss->bindings);
> + be NULL or be some temporary disposable state.
> +
> + Note that we explicitly force the pickling of
> + scope_chain->bindings. If we had previously read another PPH
> + image, scope_chain->bindings will be in the other image's pickle
> + cache. This would cause pph_out_binding_level to emit a cache
> + reference to it, instead of writing its fields. */
> + {
> + unsigned ix;
> + pph_cache_add (stream, scope_chain->bindings, &ix);
> + pph_out_record_marker (stream, PPH_RECORD_START);
> + pph_out_uint (stream, ix);
> + pph_out_binding_level_1 (stream, scope_chain->bindings, PPHF_NO_XREFS);
> + }
> }
>
>
> @@ -1278,24 +1349,30 @@ pph_out_line_table (pph_stream *stream, struct
> line_maps *linetab)
> pph_out_uint (stream, linetab->max_column_hint);
> }
>
> -/* Write PPH output symbols and IDENTS_USED to STREAM as an object. */
> +/* Write all the contents of STREAM. */
>
> static void
> -pph_write_file_contents (pph_stream *stream, cpp_idents_used *idents_used)
> -{
> +pph_write_file (pph_stream *stream)
> +{
> + cpp_idents_used idents_used;
> +
> + if (flag_pph_debug >= 1)
> + fprintf (pph_logfile, "PPH: Writing %s\n", pph_out_file);
> +
> /* Emit the list of PPH files included by STREAM. These files will
> be read and instantiated before any of the content in STREAM. */
> pph_out_includes (stream);
>
> /* Emit all the identifiers and pre-processor symbols in the global
> namespace. */
> - pph_out_identifiers (stream, idents_used);
> + idents_used = cpp_lt_capture (parse_in);
> + pph_out_identifiers (stream, &idents_used);
>
> /* Emit the line table entries. */
> pph_out_line_table (stream, line_table);
>
> /* Emit the bindings for the global namespace. */
> - pph_out_scope_chain (stream, scope_chain);
> + pph_out_scope_chain (stream);
> if (flag_pph_dump_tree)
> pph_dump_namespace (pph_logfile, global_namespace);
>
> @@ -1314,21 +1391,6 @@ pph_write_file_contents (pph_stream *stream,
> cpp_idents_used *idents_used)
> }
>
>
> -/* Write all the collected parse trees to STREAM. */
> -
> -static void
> -pph_write_file (pph_stream *stream)
> -{
> - cpp_idents_used idents_used;
> -
> - if (flag_pph_debug >= 1)
> - fprintf (pph_logfile, "PPH: Writing %s\n", pph_out_file);
> -
> - idents_used = cpp_lt_capture (parse_in);
> - pph_write_file_contents (stream, &idents_used);
> -}
> -
> -
> /* Emit the fields of FUNCTION_DECL FNDECL to STREAM. */
>
> static void
> @@ -1794,14 +1856,16 @@ pph_add_decl_to_symtab (tree decl)
> }
>
>
> -/* Add STREAM to the list of files included by pph_out_stream. */
> +/* Add INCLUDE to the list of files included by STREAM. If STREAM is
> + NULL, INCLUDE is added to the list of includes for pph_out_stream
> + (the image that we are currently generating). */
>
> void
> -pph_add_include (pph_stream *stream)
> +pph_add_include (pph_stream *stream, pph_stream *include)
> {
> - pph_stream *out_stream = pph_out_stream;
> - VEC_safe_push (pph_stream_ptr, heap, out_stream->includes, stream);
> - stream->nested_p = true;
> + if (stream == NULL)
> + stream = pph_out_stream;
> + VEC_safe_push (pph_stream_ptr, heap, stream->includes, include);
> }
>
>
> @@ -1823,14 +1887,17 @@ pph_writer_init (void)
> void
> pph_writer_finish (void)
> {
> - pph_stream *out_stream = pph_out_stream;
> - const char *offending_file = cpp_main_missing_guard (parse_in);
> + const char *offending_file;
> +
> + if (pph_out_stream == NULL)
> + return;
>
> + offending_file = cpp_main_missing_guard (parse_in);
> if (offending_file == NULL)
> - pph_write_file (out_stream);
> + pph_write_file (pph_out_stream);
> else
> error ("header lacks guard for PPH: %s", offending_file);
>
> - pph_stream_close (out_stream);
> + pph_stream_close (pph_out_stream);
> pph_out_stream = NULL;
> }
> diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
> index afabeb5..1ac5bf4 100644
> --- a/gcc/cp/pph-streamer.c
> +++ b/gcc/cp/pph-streamer.c
> @@ -114,7 +114,6 @@ pph_stream_open (const char *name, const char *mode)
> stream->name = xstrdup (name);
> stream->write_p = (strchr (mode, 'w') != NULL);
> stream->cache.m = pointer_map_create ();
> - stream->open_p = true;
> stream->symtab.m = pointer_set_create ();
> if (stream->write_p)
> pph_init_write (stream);
> @@ -128,15 +127,11 @@ pph_stream_open (const char *name, const char *mode)
>
>
>
> -/* Helper for pph_stream_close. Do not check whether STREAM is a
> - nested header. */
> +/* Close PPH stream STREAM. */
>
> -static void
> -pph_stream_close_1 (pph_stream *stream)
> +void
> +pph_stream_close (pph_stream *stream)
> {
> - unsigned i;
> - pph_stream *include;
> -
> if (flag_pph_debug >= 1)
> fprintf (pph_logfile, "PPH: Closing %s\n", stream->name);
>
> @@ -147,10 +142,6 @@ pph_stream_close_1 (pph_stream *stream)
>
> fclose (stream->file);
>
> - /* Close all the streams we opened for included PPH images. */
> - FOR_EACH_VEC_ELT (pph_stream_ptr, stream->includes, i, include)
> - pph_stream_close_1 (include);
> -
> /* Deallocate all memory used. */
> stream->file = NULL;
> VEC_free (void_p, heap, stream->cache.v);
> @@ -180,26 +171,6 @@ pph_stream_close_1 (pph_stream *stream)
> }
>
>
> -/* Close PPH stream STREAM. */
> -
> -void
> -pph_stream_close (pph_stream *stream)
> -{
> - /* If STREAM is nested into another PPH file, then we cannot
> - close it just yet. The parent PPH file will need to access
> - STREAM's symbol table (to avoid writing the same symbol
> - more than once). In this case, STREAM will be closed by the
> - parent file. */
> - if (stream->nested_p)
> - {
> - gcc_assert (!stream->write_p);
> - return;
> - }
> -
> - pph_stream_close_1 (stream);
> -}
> -
> -
> /* Data types supported by the PPH tracer. */
> enum pph_trace_type
> {
> @@ -420,13 +391,12 @@ pph_cache_insert_at (pph_stream *stream, void *data,
> unsigned ix)
> }
>
>
> -/* Add pointer DATA to the pickle cache in STREAM. If IX_P is not
> - NULL, on exit *IX_P will contain the slot number where DATA is
> - stored. Return true if DATA already existed in the cache, false
> - otherwise. */
> +/* Return true if DATA exists in STREAM's pickle cache. If IX_P is not
> + NULL, store the cache slot where DATA resides in *IX_P (or (unsigned)-1
> + if DATA is not found). */
>
> bool
> -pph_cache_add (pph_stream *stream, void *data, unsigned *ix_p)
> +pph_cache_lookup (pph_stream *stream, void *data, unsigned *ix_p)
> {
> void **map_slot;
> unsigned ix;
> @@ -436,13 +406,12 @@ pph_cache_add (pph_stream *stream, void *data, unsigned
> *ix_p)
> if (map_slot == NULL)
> {
> existed_p = false;
> - ix = VEC_length (void_p, stream->cache.v);
> - pph_cache_insert_at (stream, data, ix);
> + ix = (unsigned) -1;
> }
> else
> {
> - unsigned HOST_WIDE_INT slot_ix = (unsigned HOST_WIDE_INT) *map_slot;
> - gcc_assert (slot_ix == (unsigned) slot_ix);
> + intptr_t slot_ix = (intptr_t) *map_slot;
> + gcc_assert (slot_ix == (intptr_t)(unsigned) slot_ix);
> ix = (unsigned) slot_ix;
> existed_p = true;
> }
> @@ -454,13 +423,98 @@ pph_cache_add (pph_stream *stream, void *data, unsigned
> *ix_p)
> }
>
>
> -/* Return the pointer at slot IX in STREAM's pickle cache. */
> +/* Return true if DATA is in the pickle cache of one of STREAM's
> + included images.
> +
> + If DATA is found:
> + - the index for INCLUDE_P into IMAGE->INCLUDES is returned in
> + *INCLUDE_IX_P (if INCLUDE_IX_P is not NULL),
> + - the cache slot index for DATA into *INCLUDE_P's pickle cache
> + is returned in *IX_P (if IX_P is not NULL), and,
> + - the function returns true.
> +
> + If DATA is not found:
> + - *INCLUDE_IX_P is set to -1 (if INCLUDE_IX_P is not NULL),
> + - *IX_P is set to -1 (if IX_P is not NULL), and,
> + - the function returns false. */
> +
> +bool
> +pph_cache_lookup_in_includes (pph_stream *stream, void *data,
> + unsigned *include_ix_p, unsigned *ix_p)
> +{
> + unsigned include_ix, ix;
> + pph_stream *include;
> + bool found_it;
> +
> + found_it = false;
> + FOR_EACH_VEC_ELT (pph_stream_ptr, stream->includes, include_ix, include)
> + if (pph_cache_lookup (include, data, &ix))
> + {
> + found_it = true;
> + break;
> + }
> +
> + if (!found_it)
> + {
> + include_ix = ix = (unsigned) -1;
> + ix = (unsigned) -1;
> + }
> +
> + if (include_ix_p)
> + *include_ix_p = include_ix;
> +
> + if (ix_p)
> + *ix_p = ix;
Instead of wasting time on an if to check that include_ix_p or ix_p
are not null, couldn't we simply put a gcc_assert that they aren't
NULL at the beginning of the function? I can't think of a situation
where we want to call this with NULL pointers, i.e. without retrieving
the values we are asking for... unless sometimes it's called only
caring about the boolean returned value?
> +
> + return found_it;
> +}
> +
> +
> +/* Add pointer DATA to the pickle cache in STREAM. If IX_P is not
> + NULL, on exit *IX_P will contain the slot number where DATA is
> + stored. Return true if DATA already existed in the cache, false
> + otherwise. */
> +
> +bool
> +pph_cache_add (pph_stream *stream, void *data, unsigned *ix_p)
> +{
> + unsigned ix;
> + bool existed_p;
> +
> + if (pph_cache_lookup (stream, data, &ix))
> + existed_p = true;
> + else
> + {
> + existed_p = false;
> + ix = VEC_length (void_p, stream->cache.v);
> + pph_cache_insert_at (stream, data, ix);
> + }
> +
> + if (ix_p)
> + *ix_p = ix;
> +
> + return existed_p;
> +}
> +
> +
> +/* Return the pointer at slot IX in STREAM's pickle cache. If INCLUDE_IX
> + is not -1U, then instead of looking up in STREAM's pickle cache,
> + the pointer is looked up in the pickle cache for
> + STREAM->INCLUDES[INCLUDE_IX]. */
>
> void *
> -pph_cache_get (pph_stream *stream, unsigned ix)
> +pph_cache_get (pph_stream *stream, unsigned include_ix, unsigned ix)
> {
> - void *data = VEC_index (void_p, stream->cache.v, ix);
> - gcc_assert (data);
> + void *data;
> + pph_stream *image;
> +
> + /* Determine which image's pickle cache to use. */
> + if (include_ix == (unsigned) -1)
As mentioned above, I feel this is dangerous..
> + image = stream;
> + else
> + image = VEC_index (pph_stream_ptr, stream->includes, include_ix);
>
> + data = VEC_index (void_p, image->cache.v, ix);
> + gcc_assert (data);
> return data;
> }
> diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
> index d9fe53b..19578fd 100644
> --- a/gcc/cp/pph-streamer.h
> +++ b/gcc/cp/pph-streamer.h
> @@ -28,9 +28,26 @@ along with GCC; see the file COPYING3. If not see
>
> /* Record markers. */
> enum pph_record_marker {
> - PPH_RECORD_START = 0xfd,
> + /* This record contains the physical representation of the memory data. */
> + PPH_RECORD_START = 0x23,
> +
> + /* End of record marker. If a record starts with PPH_RECORD_END, the
> + reader should return a NULL pointer. */
> PPH_RECORD_END,
> - PPH_RECORD_SHARED
> +
> + /* Internal reference. This marker indicates that this data has
> + been written before and it resides in the pickle cache for the
> + current image. Following this marker, the reader will find the
> + cache slot where the data has been stored. */
> + PPH_RECORD_IREF,
> +
> + /* External reference. This marker indicates that this data has
> + been written before and it resides in the pickle cache for
> + another image. Following this marker, the reader will find two
> + indices: (1) the index into the include table where the other
> + image lives, and (2) the cache slot into that image's pickle
> + cache where the data resides. */
> + PPH_RECORD_XREF
> };
>
> /* Symbol table markers. These are all the symbols that need to be
> @@ -155,15 +172,6 @@ typedef struct pph_stream {
> /* Nonzero if the stream was opened for writing. */
> unsigned int write_p : 1;
>
> - /* Nonzero if the file associated with this stream is open.
> - After we read a PPH image, we deallocate all the memory used
> - during streaming, but we keep the stream around to access its
> - symbol table. */
> - unsigned int open_p : 1;
> -
> - /* Nonzero if this PPH file is included from another PPH file. */
> - unsigned int nested_p : 1;
> -
> /* Symbol table. This is collected as the compiler instantiates
> symbols and functions. Once we finish parsing the header file,
> this array is written out to the PPH image. This way, the reader
> @@ -177,8 +185,10 @@ typedef struct pph_stream {
> VEC(pph_stream_ptr,heap) *includes;
> } pph_stream;
>
> -/* Filter values for pph_out_chain_filtered. */
> -enum chain_filter { NONE, NO_BUILTINS };
> +/* Filter values to avoid emitting certain objects to a PPH file. */
> +#define PPHF_NONE 0
> +#define PPHF_NO_BUILTINS (1 << 0)
> +#define PPHF_NO_XREFS (1 << 1)
>
> /* In pph-streamer.c. */
> pph_stream *pph_stream_open (const char *, const char *);
> @@ -192,15 +202,18 @@ void pph_trace_location (pph_stream *, location_t);
> void pph_trace_chain (pph_stream *, tree);
> void pph_trace_bitpack (pph_stream *, struct bitpack_d *);
> void pph_cache_insert_at (pph_stream *, void *, unsigned);
> +bool pph_cache_lookup (pph_stream *, void *, unsigned *);
> +bool pph_cache_lookup_in_includes (pph_stream *, void *, unsigned *,
> + unsigned *);
> bool pph_cache_add (pph_stream *, void *, unsigned *);
> -void *pph_cache_get (pph_stream *, unsigned);
> +void *pph_cache_get (pph_stream *, unsigned, unsigned);
>
> /* In pph-streamer-out.c. */
> void pph_flush_buffers (pph_stream *);
> void pph_init_write (pph_stream *);
> void pph_write_tree (struct output_block *, tree, bool);
> void pph_add_decl_to_symtab (tree);
> -void pph_add_include (pph_stream *);
> +void pph_add_include (pph_stream *, pph_stream *);
> void pph_writer_init (void);
> void pph_writer_finish (void);
> void pph_write_location (struct output_block *, location_t);
> @@ -214,7 +227,8 @@ struct binding_table_s *pph_in_binding_table (pph_stream
> *);
> void pph_init_read (pph_stream *);
> tree pph_read_tree (struct lto_input_block *, struct data_in *);
> location_t pph_read_location (struct lto_input_block *, struct data_in *);
> -void pph_read_file (const char *);
> +pph_stream *pph_read_file (const char *);
> +void pph_reader_finish (void);
>
> /* In pt.c. */
> extern void pph_out_pending_templates_list (pph_stream *stream);
> @@ -487,4 +501,24 @@ pph_in_bitpack (pph_stream *stream)
> return bp;
> }
>
> +/* Write record MARKER to STREAM. */
> +static inline void
> +pph_out_record_marker (pph_stream *stream, enum pph_record_marker marker)
> +{
> + gcc_assert (marker == (enum pph_record_marker)(unsigned char) marker);
> + pph_out_uchar (stream, marker);
> +}
> +
> +/* Read and return a record marker from STREAM. */
> +static inline enum pph_record_marker
> +pph_in_record_marker (pph_stream *stream)
> +{
> + enum pph_record_marker m = (enum pph_record_marker) pph_in_uchar (stream);
> + gcc_assert (m == PPH_RECORD_START
> + || m == PPH_RECORD_END
> + || m == PPH_RECORD_IREF
> + || m == PPH_RECORD_XREF);
> + return m;
> +}
> +
> #endif /* GCC_CP_PPH_STREAMER_H */
> diff --git a/gcc/cp/pph.c b/gcc/cp/pph.c
> index 9b92f88..91dc622 100644
> --- a/gcc/cp/pph.c
> +++ b/gcc/cp/pph.c
> @@ -171,9 +171,13 @@ pph_init (void)
> void
> pph_finish (void)
> {
> - if (pph_out_file != NULL)
> + /* Finalize the writer. */
> pph_writer_finish ();
>
> + /* Finalize the reader. */
> + pph_reader_finish ();
> +
> + /* Close log files. */
> if (flag_pph_debug >= 1)
> fprintf (pph_logfile, "PPH: Finishing.\n");
>
> diff --git a/gcc/testsuite/ChangeLog.pph b/gcc/testsuite/ChangeLog.pph
> index 7ece8ce..b2b87f0 100644
> --- a/gcc/testsuite/ChangeLog.pph
> +++ b/gcc/testsuite/ChangeLog.pph
> @@ -1,3 +1,9 @@
> +2011-08-15 Diego Novillo <dnovi...@google.com>
> +
> + * g++.dg/pph/x1tmplclass2.cc: Update expected ICE.
> + * g++.dg/pph/x4tmplclass2.cc: Update expected ICE.
> + * g++.dg/pph/z4tmplclass2.cc: Update expected ICE.
> +
> 2011-08-10 Diego Novillo <dnovi...@google.com>
>
> * g++.dg/pph/x1tmplclass2.cc: Update expected ICE message.
> diff --git a/gcc/testsuite/g++.dg/pph/x1tmplclass2.cc
> b/gcc/testsuite/g++.dg/pph/x1tmplclass2.cc
> index 2c7a8f3..f04335d 100644
> --- a/gcc/testsuite/g++.dg/pph/x1tmplclass2.cc
> +++ b/gcc/testsuite/g++.dg/pph/x1tmplclass2.cc
> @@ -1,5 +1,5 @@
> // { dg-xfail-if "BOGUS" { "*-*-*" } { "-fpph-map=pph.map" } }
> -// { dg-bogus "x1tmplclass2.cc:1:0: internal compiler error: in
> pph_in_start_record, at cp/pph-streamer-in.c" "" { xfail *-*-* } 0 }
> +// { dg-bogus "x1tmplclass2.cc:1:0: internal compiler error: in
> pph_in_record_marker, at cp/pph-streamer.h" "" { xfail *-*-* } 0 }
>
> #include "x0tmplclass23.h"
> #include "a0tmplclass2_u.h"
> diff --git a/gcc/testsuite/g++.dg/pph/x4tmplclass2.cc
> b/gcc/testsuite/g++.dg/pph/x4tmplclass2.cc
> index 7c7e6a5..585d4c0 100644
> --- a/gcc/testsuite/g++.dg/pph/x4tmplclass2.cc
> +++ b/gcc/testsuite/g++.dg/pph/x4tmplclass2.cc
> @@ -1,5 +1,5 @@
> // { dg-xfail-if "BOGUS" { "*-*-*" } { "-fpph-map=pph.map" } }
> -// { dg-bogus "x4tmplclass2.cc:1:0: internal compiler error: in
> pph_in_start_record, at cp/pph-streamer-in.c" "" { xfail *-*-* } 0 }
> +// { dg-bogus "x4tmplclass2.cc:1:0: internal compiler error: in
> pph_in_record_marker, at cp/pph-streamer.h" "" { xfail *-*-* } 0 }
>
> #include "x0tmplclass21.h"
> #include "x0tmplclass22.h"
> diff --git a/gcc/testsuite/g++.dg/pph/z4tmplclass2.cc
> b/gcc/testsuite/g++.dg/pph/z4tmplclass2.cc
> index b932f5e..0243829 100644
> --- a/gcc/testsuite/g++.dg/pph/z4tmplclass2.cc
> +++ b/gcc/testsuite/g++.dg/pph/z4tmplclass2.cc
> @@ -1,5 +1,5 @@
> // { dg-xfail-if "BOGUS" { "*-*-*" } { "-fpph-map=pph.map" } }
> -// { dg-bogus "z4tmplclass2.cc:1:0: internal compiler error: in
> pph_in_start_record, at cp/pph-streamer-in.c" "" { xfail *-*-* } 0 }
> +// { dg-bogus "z4tmplclass2.cc:1:0: internal compiler error: in
> pph_in_record_marker, at cp/pph-streamer.h" "" { xfail *-*-* } 0 }
>
> #include "x0tmplclass23.h"
> #include "x0tmplclass24.h"
> --
> 1.7.3.1
>
>
> --
> This patch is available for review at http://codereview.appspot.com/4873051
>
