[pph] Allocate string tables separately.

[Diego Novillo, 2011-08-04 17:12:29]

This patch separates the string tables from input streams to allow the
string tables to remain allocated throughout the compilation process.

Gab, I think this addresses the problem you were seeing with your
line number changes.  Let me know if it doesn't.

Tested on x86_64.  Committed to branch.


	* pph-streamer-in.c (string_tables): Declare.
	(pph_init_read): Create a new entry in string_tables and copy
	the string table from STREAM into it.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 15c5032..e57e1e7 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -33,6 +33,19 @@ along with GCC; see the file COPYING3.  If not see
 #include "cppbuiltin.h"
 #include "toplev.h"
 
+typedef char *char_p;
+DEF_VEC_P(char_p);
+DEF_VEC_ALLOC_P(char_p,heap);
+
+/* String tables for all input streams.  These are allocated separately
+  from streams because they cannot be deallocated after the streams
+  have been read (string streaming works by pointing into these
+  tables).
+
+  Each stream will create a new entry in this table of tables.  The
+  memory will remain allocated until the end of compilation.  */
+static VEC(char_p,heap) *string_tables = NULL;
+
 /* Wrapper for memory allocation calls that should have their results
    registered in the PPH streamer cache.  DATA is the pointer returned
    by the memory allocation call in ALLOC_EXPR.  IX is the cache slot 
@@ -106,6 +119,7 @@ pph_init_read (pph_stream *stream)
   int retcode;
   pph_file_header *header;
   const char *strtab, *body;
+  char *new_strtab;
 
   lto_reader_init ();
 
@@ -134,16 +148,25 @@ pph_init_read (pph_stream *stream)
   body_size = stream->encoder.r.file_size
 	      - strtab_size - sizeof (pph_file_header);
 
+  /* Create a new string table for STREAM.  This table is not part of
+     STREAM because it needs to remain around until the end of
+     compilation (all the string streaming routines work by pointing
+     into the string table, so we cannot deallocate it after reading
+     STREAM).  */
+  new_strtab = XNEWVEC (char, strtab_size);
+  memcpy (new_strtab, strtab, strtab_size);
+  VEC_safe_push (char_p, heap, string_tables, new_strtab);
+
   /* Create an input block structure pointing right after the string
      table.  */
   stream->encoder.r.ib = XCNEW (struct lto_input_block);
   LTO_INIT_INPUT_BLOCK_PTR (stream->encoder.r.ib, body, 0, body_size);
   stream->encoder.r.data_in
-      = lto_data_in_create (stream->encoder.r.pph_sections[0], strtab,
-			    strtab_size, NULL);
+      = lto_data_in_create (stream->encoder.r.pph_sections[0],
+			    new_strtab, strtab_size, NULL);
 
-  /* Associate STREAM with STREAM->ENCODER.R.DATA_IN so we can recover it from
-     the streamer hooks.  */
+  /* Associate STREAM with STREAM->ENCODER.R.DATA_IN so we can recover
+     it from the streamer hooks.  */
   stream->encoder.r.data_in->sdata = (void *) stream;
 }
 

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

[Gabriel Charette, 2011-08-05 17:42:11]

Yep this fixes the problem I had with the strings being freed early.

See comments inline below.

http://codereview.appspot.com/4843044/diff/1/gcc/cp/pph-streamer-in.c
File gcc/cp/pph-streamer-in.c (right):

http://codereview.appspot.com/4843044/diff/1/gcc/cp/pph-streamer-in.c#newcode157
gcc/cp/pph-streamer-in.c:157: memcpy (new_strtab, strtab, strtab_size);
I don't think we need to memcpy, to be more efficient instead we can just change
the way stream->encoder.r.file_data is read to only read the pph_file_header +
body into it, and fread the strtab straight into new_strtab.

This implies more logic when reading, but is potentially more efficient.

[Diego Novillo, 2011-08-05 17:51:12]

On Fri, Aug 5, 2011 at 13:42,  <gchare@google.com> wrote:

> Yep this fixes the problem I had with the strings being freed early.

Great.

>
http://codereview.appspot.com/4843044/diff/1/gcc/cp/pph-streamer-in.c#newcode157
> gcc/cp/pph-streamer-in.c:157: memcpy (new_strtab, strtab, strtab_size);
> I don't think we need to memcpy, to be more efficient instead we can
> just change the way stream->encoder.r.file_data is read to only read the
> pph_file_header + body into it, and fread the strtab straight into
> new_strtab.
>
> This implies more logic when reading, but is potentially more efficient.

Yeah, I thought about that, but I didn't want to make the file reading
logic more convoluted than it already is.  Maybe we can do that at
some later point.  And I think that the multiple read operations may
in fact be slower than memcpy'ing, but I didn't really try that.


Diego.