[pph] Cleanup line_table and includes streaming (issue4921052)

[pph] Cleanup line_table and includes streaming (issue4921052)

[Gabriel Charette]

This is a refactoring patch, it doesn't change/fix anything in pph itself.

I extracted out/in logic for includes into their own functions.

I removed the LINETAB parameter from in/out functions for the line_table as there is only one line_table and that's the only one we stream, the main/global one.

I wanted to move pph_loc_offset to pph_stream.encoder.r, but it's not possible while locations are called by the streamer hook as in the callback we do not have a pph_stream* parameter, added a FIXME to do it later if we do get rid of the hook for locations.

Tested with bootstrap and pph regression testing on x64.

Cheers,
Gab

2011-08-22  Gabriel Charette  <gchare@google.com>

	* pph-streamer-in.c (pph_loc_offset): Add FIXME to move this variable
	to pph_stream.encoder.r
	(pph_in_include): New.
	(pph_in_line_table_and_includes): Remove LINETAB parameter. Update
	all users to use global LINE_TABLE instead.
	Remove code moved to new function pph_in_include. And call it.
	(pph_read_file_1): Remove extra parameter in call to
	pph_in_line_table_and_includes.
	* pph-streamer-out.c (pph_out_include): New.
	(pph_get_next_include): Fix comment.
	(pph_out_line_table_and_includes): Remove LINETAB parameter. Update
	all users to use global LINE_TABLE instead.
	Remove code moved to new function pph_out_include. And call it.
	* gcc/cp/pph-streamer.h (pph_stream): Fix indenting.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 07e1135..2b7dc2d 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -75,7 +75,11 @@ static int pph_reading_includes = 0;
     } while (0)
 
 /* Set in pph_in_and_merge_line_table. Represents the source_location offset
-   which every streamed in token must add to it's serialized source_location. */
+   which every streamed in token must add to it's serialized source_location.
+
+   FIXME pph: Ideally this would be in pph_stream.encoder.r, but for that we
+   first need to get rid of the dependency to the streamer_hook for locations.
+   */
 static int pph_loc_offset;
 
 
@@ -1336,9 +1340,37 @@ pph_in_line_map (pph_stream *stream, struct line_map *lm)
 }
 
 
-/* Read the line_table from STREAM and merge it in LINETAB.  At the same time
-   load includes in the order they were originally included by loading them at
-   the point they were referenced in the line_table.
+/* Read in from STREAM and merge a referenced include into the current parsing
+   context.  */
+
+static void
+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
+     compilation.  */
+  line_table->highest_location = (prev_start_loc - 1) + pph_loc_offset;
+
+  /* FIXME pph: If we move pph_loc_offset to pph_stream.encoder.r, we could
+     have an independent offset for each stream and not have to save and
+     restore the state of a global pph_loc_offset as we are doing here.  */
+  old_loc_offset = pph_loc_offset;
+
+  include_name = pph_in_string (stream);
+  include = pph_read_file (include_name);
+  pph_add_include (include, false);
+
+  pph_loc_offset = old_loc_offset;
+}
+
+
+/* Read the line_table from STREAM and merge it in the current line_table.  At
+   the same time load includes in the order they were originally included by
+   loading them at the point they were referenced in the line_table.
 
    Returns the source_location of line 1 / col 0 for this include.
 
@@ -1348,13 +1380,13 @@ pph_in_line_map (pph_stream *stream, struct line_map *lm)
    a known current issue, so I didn't bother working around it here for now.  */
 
 static source_location
-pph_in_line_table_and_includes (pph_stream *stream, struct line_maps *linetab)
+pph_in_line_table_and_includes (pph_stream *stream)
 {
   unsigned int old_depth;
   bool first;
   int includer_ix = -1;
-  unsigned int used_before = linetab->used;
-  int entries_offset = linetab->used - PPH_NUM_IGNORED_LINE_TABLE_ENTRIES;
+  unsigned int used_before = line_table->used;
+  int entries_offset = line_table->used - PPH_NUM_IGNORED_LINE_TABLE_ENTRIES;
   enum pph_linetable_marker next_lt_marker = pph_in_linetable_marker (stream);
 
   pph_reading_includes++;
@@ -1364,29 +1396,16 @@ pph_in_line_table_and_includes (pph_stream *stream, struct line_maps *linetab)
     {
       if (next_lt_marker == PPH_LINETABLE_REFERENCE)
 	{
-	  int old_loc_offset;
-	  const char *include_name = pph_in_string (stream);
-	  source_location prev_start_loc = pph_in_source_location (stream);
-	  pph_stream *include;
-
 	  gcc_assert (!first);
-
-	  linetab->highest_location = (prev_start_loc - 1) + pph_loc_offset;
-
-	  old_loc_offset = pph_loc_offset;
-
-	  include = pph_read_file (include_name);
-	  pph_add_include (include, false);
-
-	  pph_loc_offset = old_loc_offset;
+	  pph_in_include (stream);
 	}
       else
 	{
 	  struct line_map *lm;
 
-	  linemap_ensure_extra_space_available (linetab);
+	  linemap_ensure_extra_space_available (line_table);
 
-	  lm = &linetab->maps[linetab->used];
+	  lm = &line_table->maps[line_table->used];
 
 	  pph_in_line_map (stream, lm);
 
@@ -1394,10 +1413,10 @@ pph_in_line_table_and_includes (pph_stream *stream, struct line_maps *linetab)
 	    {
 	      first = false;
 
-	      pph_loc_offset = (linetab->highest_location + 1)
+	      pph_loc_offset = (line_table->highest_location + 1)
 		               - lm->start_location;
 
-	      includer_ix = linetab->used - 1;
+	      includer_ix = line_table->used - 1;
 
 	      gcc_assert (lm->included_from == -1);
 	    }
@@ -1415,11 +1434,11 @@ pph_in_line_table_and_includes (pph_stream *stream, struct line_maps *linetab)
 	  else
 	    lm->included_from += entries_offset;
 
-	  gcc_assert (lm->included_from < (int) linetab->used);
+	  gcc_assert (lm->included_from < (int) line_table->used);
 
 	  lm->start_location += pph_loc_offset;
 
-	  linetab->used++;
+	  line_table->used++;
 	}
     }
 
@@ -1427,14 +1446,14 @@ pph_in_line_table_and_includes (pph_stream *stream, struct line_maps *linetab)
 
   {
     unsigned int expected_in = pph_in_uint (stream);
-    gcc_assert (linetab->used - used_before == expected_in);
+    gcc_assert (line_table->used - used_before == expected_in);
   }
 
-  linetab->highest_location = pph_loc_offset + pph_in_uint (stream);
-  linetab->highest_line = pph_loc_offset + pph_in_uint (stream);
+  line_table->highest_location = pph_loc_offset + pph_in_uint (stream);
+  line_table->highest_line = pph_loc_offset + pph_in_uint (stream);
 
   /* The MAX_COLUMN_HINT can be directly overwritten.  */
-  linetab->max_column_hint = pph_in_uint (stream);
+  line_table->max_column_hint = pph_in_uint (stream);
 
   /* The line_table doesn't store the last LC_LEAVE in any given compilation;
      thus we need to replay the LC_LEAVE for the header now.  For that same
@@ -1442,11 +1461,11 @@ pph_in_line_table_and_includes (pph_stream *stream, struct line_maps *linetab)
      one include deeper then the depth at which this pph was included.  The
      LC_LEAVE replay will then bring the depth back to what it was before
      calling this function.  */
-  old_depth = linetab->depth++;
-  linemap_add (linetab, LC_LEAVE, 0, NULL, 0);
-  gcc_assert (linetab->depth == old_depth);
+  old_depth = line_table->depth++;
+  linemap_add (line_table, LC_LEAVE, 0, NULL, 0);
+  gcc_assert (line_table->depth == old_depth);
 
-  return linetab->maps[used_before].start_location;
+  return line_table->maps[used_before].start_location;
 }
 
 
@@ -1470,7 +1489,7 @@ pph_read_file_1 (pph_stream *stream)
   /* Read in STREAM's line table and merge it in the current line table.
      At the same time, read in includes in the order they were originally
      read.  */
-  cpp_token_replay_loc = pph_in_line_table_and_includes (stream, line_table);
+  cpp_token_replay_loc = pph_in_line_table_and_includes (stream);
 
   /* Read all the identifiers and pre-processor symbols in the global
      namespace.  */
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index 024de38..bb13cb6 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -1192,6 +1192,18 @@ pph_out_line_map (pph_stream *stream, struct line_map *lm)
 }
 
 
+/* Write a reference of INCLUDE to STREAM.  Also write the START_LOCATION of
+   this include in the current line_table.  */
+
+static void
+pph_out_include (pph_stream *stream, pph_stream *include,
+                 source_location start_location)
+{
+  pph_out_source_location (stream, start_location);
+  pph_out_string (stream, include->name);
+}
+
+
 /* Compare filenames of a header and it's potentially corresponding pph file,
    stripping the path passed in and the extension. Returns true if HEADER_PATH
    and PPH_PATH end with the same filename. We expect HEADER_PATH to end in .h
@@ -1232,7 +1244,8 @@ pph_filename_eq_ignoring_path (const char *header_path, const char *pph_path)
 
 
 /* Return the *NEXT_INCLUDE_IX'th pph_stream in STREAM's list of includes.
-   Returns NULL if we have read all includes.  */
+   Returns NULL if we have read all includes.  Increments *NEXT_INCLUDE_IX
+   when sucessful.  */
 
 static inline pph_stream *
 pph_get_next_include (pph_stream *stream, unsigned int *next_incl_ix)
@@ -1246,27 +1259,27 @@ pph_get_next_include (pph_stream *stream, unsigned int *next_incl_ix)
 
 
 /* Emit the required line_map entry (those directly related to this include)
-   and some properties in LINETAB to STREAM, ignoring builtin and command-line
-   entries.  We will write references to our direct includes children and skip
-   their actual line_map entries (unless they are non-pph children in which case
-   we have to write out their line_map entries as well).  We assume
-   stream->encoder.w.includes contains the pph headers included in the same
-   order they are seen in the line_table.  */
+   and some properties in the line_table to STREAM, ignoring builtin and
+   command-line entries.  We will write references to our direct includes
+   children and skip their actual line_map entries (unless they are non-pph
+   children in which case we have to write out their line_map entries as well).
+   We assume stream->encoder.w.includes contains the pph headers included in the
+   same order they are seen in the line_table.  */
 
 static void
-pph_out_line_table_and_includes (pph_stream *stream, struct line_maps *linetab)
+pph_out_line_table_and_includes (pph_stream *stream)
 {
   unsigned int ix, next_incl_ix = 0;
   pph_stream *current_include;
 
   /* Any #include should have been fully parsed and exited at this point.  */
-  gcc_assert (linetab->depth == 0);
+  gcc_assert (line_table->depth == 0);
 
   current_include = pph_get_next_include (stream, &next_incl_ix);
 
-  for (ix = PPH_NUM_IGNORED_LINE_TABLE_ENTRIES; ix < linetab->used; ix++)
+  for (ix = PPH_NUM_IGNORED_LINE_TABLE_ENTRIES; ix < line_table->used; ix++)
     {
-      struct line_map *lm = &linetab->maps[ix];
+      struct line_map *lm = &line_table->maps[ix];
 
       if (ix == PPH_NUM_IGNORED_LINE_TABLE_ENTRIES)
         {
@@ -1289,23 +1302,20 @@ pph_out_line_table_and_includes (pph_stream *stream, struct line_maps *linetab)
 	  gcc_assert (lm->included_from != -1);
 
 	  pph_out_linetable_marker (stream, PPH_LINETABLE_REFERENCE);
-	  pph_out_string (stream, current_include->name);
 
-	  /* We also need to output the start_location to simulate the correct
-	      highest_location on the way in.  */
-	  pph_out_source_location (stream, lm->start_location);
+	  pph_out_include (stream, current_include, lm->start_location);
 
 	  /* Potentially lm could be included from a header other then the main
 	      one if a textual include includes a pph header (i.e. we can't
 	      simply rely on going back to included_from == -1).  */
-	  includer_level = INCLUDED_FROM (linetab, lm)->included_from;
+	  includer_level = INCLUDED_FROM (line_table, lm)->included_from;
 
 	  /* Skip all other linemap entries up to and including the LC_LEAVE
 	      from the referenced header back to the one including it.  */
-	  while (linetab->maps[++ix].included_from != includer_level)
+	  while (line_table->maps[++ix].included_from != includer_level)
 	    /* We should always leave this loop before the end of the
 		current line_table entries.  */
-	    gcc_assert (ix < linetab->used);
+	    gcc_assert (ix < line_table->used);
 
 	  current_include = pph_get_next_include (stream, &next_incl_ix);
 	}
@@ -1323,17 +1333,17 @@ pph_out_line_table_and_includes (pph_stream *stream, struct line_maps *linetab)
   pph_out_linetable_marker (stream, PPH_LINETABLE_END);
 
   /* Output the number of entries written to validate on input.  */
-  pph_out_uint (stream, linetab->used - PPH_NUM_IGNORED_LINE_TABLE_ENTRIES);
+  pph_out_uint (stream, line_table->used - PPH_NUM_IGNORED_LINE_TABLE_ENTRIES);
 
   /* Every pph header included should have been seen and skipped in the
      line_table streaming above.  */
   gcc_assert (next_incl_ix == VEC_length (pph_stream_ptr,
 					  stream->encoder.w.includes));
 
-  pph_out_source_location (stream, linetab->highest_location);
-  pph_out_source_location (stream, linetab->highest_line);
+  pph_out_source_location (stream, line_table->highest_location);
+  pph_out_source_location (stream, line_table->highest_line);
 
-  pph_out_uint (stream, linetab->max_column_hint);
+  pph_out_uint (stream, line_table->max_column_hint);
 }
 
 /* Write all the contents of STREAM.  */
@@ -1347,7 +1357,7 @@ pph_write_file (pph_stream *stream)
     fprintf (pph_logfile, "PPH: Writing %s\n", pph_out_file);
 
   /* Emit the line table entries and references to our direct includes.   */
-  pph_out_line_table_and_includes (stream, line_table);
+  pph_out_line_table_and_includes (stream);
 
   /* Emit all the identifiers and pre-processor symbols in the global
      namespace.  */
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index 4a1f9eb..e191a16 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -173,23 +173,23 @@ typedef struct pph_stream {
   union {
     /* Encoding tables and buffers used to write trees to a file.  */
     struct {
-	struct output_block *ob;
-	struct lto_out_decl_state *out_state;
-	struct lto_output_stream *decl_state_stream;
-
-	/* List of PPH files included by the PPH file that we are currently
-	  generating.  Note that this list only contains PPH files, not
-	  regular text headers.  Those are embedded in this stream.  */
-	VEC(pph_stream_ptr,heap) *includes;
+      struct output_block *ob;
+      struct lto_out_decl_state *out_state;
+      struct lto_output_stream *decl_state_stream;
+
+      /* List of PPH files included by the PPH file that we are currently
+        generating.  Note that this list only contains PPH files, not
+        regular text headers.  Those are embedded in this stream.  */
+      VEC(pph_stream_ptr,heap) *includes;
     } w;
 
     /* Decoding tables and buffers used to read trees from a file.  */
     struct {
-	struct lto_input_block *ib;
-	struct data_in *data_in;
-	struct lto_file_decl_data **pph_sections;
-	char *file_data;
-	size_t file_size;
+      struct lto_input_block *ib;
+      struct data_in *data_in;
+      struct lto_file_decl_data **pph_sections;
+      char *file_data;
+      size_t file_size;
     } r;
   } encoder;
 

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

Re: [pph] Cleanup line_table and includes streaming (issue4921052)

[Diego Novillo]

On 11-08-22 14:20 , Gabriel Charette wrote:

> 2011-08-22  Gabriel Charette<gchare@google.com>
>
> 	* pph-streamer-in.c (pph_loc_offset): Add FIXME to move this variable
> 	to pph_stream.encoder.r
> 	(pph_in_include): New.
> 	(pph_in_line_table_and_includes): Remove LINETAB parameter. Update
> 	all users to use global LINE_TABLE instead.
> 	Remove code moved to new function pph_in_include. And call it.
> 	(pph_read_file_1): Remove extra parameter in call to
> 	pph_in_line_table_and_includes.
> 	* pph-streamer-out.c (pph_out_include): New.
> 	(pph_get_next_include): Fix comment.
> 	(pph_out_line_table_and_includes): Remove LINETAB parameter. Update
> 	all users to use global LINE_TABLE instead.
> 	Remove code moved to new function pph_out_include. And call it.
> 	* gcc/cp/pph-streamer.h (pph_stream): Fix indenting.

OK.


Diego.