Oops forgot to reply all the first time...
On Fri, Sep 9, 2011 at 4:54 PM, Diego Novillo <[email protected]> wrote:
> This was not causing any failures, but it is pretty wasteful to read
> the same PPH more than once.
>
> We cannot just skip them, however. We need to read the line table to
> properly modify the line table for the current translation unit.
I don't think that's right. If the header is not re-read (i.e. ifdef
guarded out), it should not show in the line_table either. I think you
simply want to do this check at the top of pph_read_file itself.
>
> /* Read PPH file FILENAME. Return the in-memory pph_stream instance. */
>
> -pph_stream *
> +void
> pph_read_file (const char *filename)
> {
> pph_stream *stream;
> @@ -1667,7 +1696,7 @@ pph_read_file (const char *filename)
> else
> error ("Cannot open PPH file for reading: %s: %m", filename);
>
> - return stream;
> + pph_add_read_image (stream);
> }
This needs to be after the check for an already read image I think
(having it here wouldn't create test failures, but would create
duplicate entries for skipped headers (as right now they are still
added to pph_read_images when skipped I think)).
Cheers!,
Gab
On Fri, Sep 9, 2011 at 4:54 PM, Diego Novillo <[email protected]> wrote:
> This was not causing any failures, but it is pretty wasteful to read
> the same PPH more than once.
>
> We cannot just skip them, however. We need to read the line table to
> properly modify the line table for the current translation unit.
>
> Tested on x86_64. Committed to branch.
>
>
> Diego.
>
>
> * pph-streamer-in.c (pph_image_already_read): New.
> (pph_read_file_1): Call pph_image_already_read. If it returns
> true, return after reading the line table.
> (pph_add_read_image): New.
> (pph_read_file): Change return value to void. Update all callers.
> Call pph_add_read_image.
> * pph-streamer-out.c (pph_add_include): Remove second argument.
> Update all callers.
> Do not add INCLUDE to pph_read_images.
> * pph-streamer.h (pph_add_include): Update prototype.
>
> diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
> index b111850..ea44460 100644
> --- a/gcc/cp/pph-streamer-in.c
> +++ b/gcc/cp/pph-streamer-in.c
> @@ -1462,7 +1462,6 @@ pph_in_include (pph_stream *stream)
> {
> int old_loc_offset;
> const char *include_name;
> - pph_stream *include;
> source_location prev_start_loc = pph_in_source_location (stream);
>
> /* Simulate highest_location to be as it would be at this point in a non-pph
> @@ -1475,8 +1474,7 @@ pph_in_include (pph_stream *stream)
> old_loc_offset = pph_loc_offset;
>
> include_name = pph_in_string (stream);
> - include = pph_read_file (include_name);
> - pph_add_include (include, false);
> + pph_read_file (include_name);
>
> pph_loc_offset = old_loc_offset;
> }
> @@ -1583,6 +1581,23 @@ pph_in_line_table_and_includes (pph_stream *stream)
> }
>
>
> +/* If FILENAME has already been read, return the stream associated with it.
> */
> +
> +static pph_stream *
> +pph_image_already_read (const char *filename)
> +{
> + pph_stream *include;
> + unsigned i;
> +
> + /* FIXME pph, implement a hash map to avoid this linear search. */
> + FOR_EACH_VEC_ELT (pph_stream_ptr, pph_read_images, i, include)
> + if (strcmp (include->name, filename) == 0)
> + return include;
> +
> + return NULL;
> +}
> +
> +
> /* Helper for pph_read_file. Read contents of PPH file in STREAM. */
>
> static void
> @@ -1605,6 +1620,11 @@ pph_read_file_1 (pph_stream *stream)
> read. */
> cpp_token_replay_loc = pph_in_line_table_and_includes (stream);
>
> + /* If we have read STREAM before, we do not need to re-read the rest
> + of its body. We only needed to read its line table. */
> + if (pph_image_already_read (stream->name))
> + return;
> +
> /* Read all the identifiers and pre-processor symbols in the global
> namespace. */
> pph_in_identifiers (stream, &idents_used);
> @@ -1650,13 +1670,22 @@ pph_read_file_1 (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_reading_includes)
> - pph_add_include (stream, true);
> + pph_add_include (stream);
> +}
> +
> +
> +/* Add STREAM to the list of read images. */
> +
> +static void
> +pph_add_read_image (pph_stream *stream)
> +{
> + VEC_safe_push (pph_stream_ptr, heap, pph_read_images, stream);
> }
>
>
> /* Read PPH file FILENAME. Return the in-memory pph_stream instance. */
>
> -pph_stream *
> +void
> pph_read_file (const char *filename)
> {
> pph_stream *stream;
> @@ -1667,7 +1696,7 @@ pph_read_file (const char *filename)
> else
> error ("Cannot open PPH file for reading: %s: %m", filename);
>
> - return stream;
> + pph_add_read_image (stream);
> }
>
>
> diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
> index 264294c..1a32814 100644
> --- a/gcc/cp/pph-streamer-out.c
> +++ b/gcc/cp/pph-streamer-out.c
> @@ -1845,18 +1845,13 @@ pph_add_decl_to_symtab (tree decl, enum
> pph_symtab_action action,
> }
>
>
> -/* Add INCLUDE to the list of files included by the main pph_out_stream
> - if IS_MAIN_STREAM_INCLUDE, as well as to the global list of all read
> - includes. */
> +/* Add INCLUDE to the list of files included by pph_out_stream. */
>
> void
> -pph_add_include (pph_stream *include, bool is_main_stream_include)
> +pph_add_include (pph_stream *include)
> {
> - if (is_main_stream_include)
> - VEC_safe_push (pph_stream_ptr, heap,
> - pph_out_stream->encoder.w.includes, include);
> -
> - VEC_safe_push (pph_stream_ptr, heap, pph_read_images, include);
> + VEC_safe_push (pph_stream_ptr, heap, pph_out_stream->encoder.w.includes,
> + include);
> }
>
>
> diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
> index 7205d64..7f98764 100644
> --- a/gcc/cp/pph-streamer.h
> +++ b/gcc/cp/pph-streamer.h
> @@ -255,7 +255,7 @@ 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, enum pph_symtab_action, bool, bool);
> -void pph_add_include (pph_stream *, bool);
> +void pph_add_include (pph_stream *);
> void pph_writer_init (void);
> void pph_writer_finish (void);
> void pph_write_location (struct output_block *, location_t);
> @@ -269,7 +269,7 @@ 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 *);
> -pph_stream *pph_read_file (const char *);
> +void pph_read_file (const char *);
> void pph_reader_finish (void);
>
> /* In pt.c. */
>
> --
> This patch is available for review at http://codereview.appspot.com/4983055
>
