[pph] Fix pph_read_tree_header.

[Lawrence Crowl, 2011-09-17 22:01:37]

Restructure pph_read_tree_header and pph_read_tree to do what their
names say they do.  In particular, generic tree handling from
pph_read_tree_header to pph_read_tree.

Add debugging routines pph_tree_code_text (to avoid out of bounds
indexing on a bad tree code) and pph_dump_min_decl (to dump the
minimal decl information).

Rename the preprocessor lookaside table field pth_debug_level to
debug_level (as PTH is gone).  Adjust thess debug levels to mean
0 == none, 1 == statistics, 2 == verbose.  Adjust the initialization
in pph_init to reflect that.

Tested on x64.


Index: gcc/cp/ChangeLog.pph

2011-09-17   Lawrence Crowl  <crowl@google.com>

	* pph.h (pph_tree_code_text): New.
	(pph_dump_min_decl): New.
	* pph.c (pph_tree_code_text): New.  Update users of tree_code_name
	to call this function instead, here and in other files.
	(pph_dump_min_decl): New.
	(pph_init): Adjust debug value to preprocessor's value.
	* pph-streamer-in.c (pph_read_tree_header): Move work unrelated to
	tree headers out of this function.
	* (pph_read_tree): Move that work here.
	* pt.c (pph_in_spec_entry_htab): Use pph_logfile instead of stderr.
	(pph_out_spec_entry_tables): Likewise.
	(pph_in_spec_entry_tables): Likewise.

Index: libcpp/ChangeLog.pph

2011-09-17  Lawrence Crowl  <crowl@google.com>

	* internal.h (struct cpp_lookaside): Rename pth_debug_level to
	debug_level.
	* symtab.c (cpp_lt_create): Likewise.
	(cpp_lt_capture): Likewise.
	(lt_statistics): Likewise, and change values to 0 == none,
	1 == statistics, 2 == verbose.
	(lt_query_macro): Likewise.


Index: gcc/cp/pph.c
===================================================================
--- gcc/cp/pph.c	(revision 178797)
+++ gcc/cp/pph.c	(working copy)
@@ -42,6 +42,25 @@ along with GCC; see the file COPYING3.  
    -fpph_logfile.  If this flag is not given, stdout is used.  */
 FILE *pph_logfile = NULL;
 
+/* Convert a checked tree_code CODE to a string.  */
+
+const char*
+pph_tree_code_text (enum tree_code code)
+{
+  gcc_assert (code <= TEMPLATE_INFO);
+  return tree_code_name[code];
+}
+
+/* Dump identifying information for a minimal DECL to FILE.  */
+
+void
+pph_dump_min_decl (FILE *file, tree decl)
+{
+  expanded_location xloc = expand_location (DECL_SOURCE_LOCATION (decl));
+  print_generic_expr (file, DECL_NAME (decl), 0);
+  print_generic_expr (file, DECL_CONTEXT (decl), 0);
+  fprintf (file, "%s:%d", xloc.file, xloc.line);
+}
 
 /* Dump a complicated name for tree T to FILE using FLAGS.
    See TDF_* in tree-pass.h for flags.  */
@@ -50,7 +69,7 @@ void
 pph_dump_tree_name (FILE *file, tree t, int flags)
 {
   enum tree_code code = TREE_CODE (t);
-  fprintf (file, "%s\t", tree_code_name[code]);
+  fprintf (file, "%s\t", pph_tree_code_text (code));
   if (code == FUNCTION_TYPE || code == METHOD_TYPE)
     {
       dump_function_to_file (t, file, flags);
@@ -182,7 +201,7 @@ pph_init (void)
   cb->include = pph_include_handler;
 
   table = cpp_lt_exchange (parse_in,
-                           cpp_lt_create (cpp_lt_order, flag_pph_debug));
+                           cpp_lt_create (cpp_lt_order, flag_pph_debug/2));
   gcc_assert (table == NULL);
 
   /* If we are generating a PPH file, initialize the writer.  */
Index: gcc/cp/pph.h
===================================================================
--- gcc/cp/pph.h	(revision 178797)
+++ gcc/cp/pph.h	(working copy)
@@ -72,6 +72,8 @@ extern FILE *pph_logfile;
 /* In pph.c  */
 extern void pph_init (void);
 extern void pph_finish (void);
+extern const char *pph_tree_code_text (enum tree_code code);
+extern void pph_dump_min_decl (FILE *file, tree decl);
 extern void pph_dump_tree_name (FILE *file, tree t, int flags);
 extern void pph_dump_namespace (FILE *, tree ns);
 
Index: gcc/cp/pph-streamer-in.c
===================================================================
--- gcc/cp/pph-streamer-in.c	(revision 178797)
+++ gcc/cp/pph-streamer-in.c	(working copy)
@@ -1957,13 +1957,13 @@ pph_read_tree_body (pph_stream *stream, 
     case OPTIMIZATION_NODE:
     case TARGET_OPTION_NODE:
       fatal_error ("PPH: unimplemented tree node '%s'",
-		   tree_code_name[TREE_CODE (expr)]);
+		   pph_tree_code_text (TREE_CODE (expr)));
       break;
 
     /* TREES UNRECOGNIZED */
     default:
       fatal_error ("PPH: unrecognized tree node '%s'",
-                   tree_code_name[TREE_CODE (expr)]);
+                   pph_tree_code_text (TREE_CODE (expr)));
     }
 }
 
@@ -2007,61 +2007,29 @@ pph_unpack_value_fields (struct bitpack_
 
 
 /* Read a tree header from STREAM and allocate a memory instance for it.
-   Store the new tree in *EXPR_P and write it into the pickle cache at
-   slot IX.
-
-   The return code will indicate whether the caller needs to keep
-   reading the body for *EXPR_P.  Some trees are simple enough that
-   they are completely contained in the header.  In these cases, no
-   more reading is necessary, so we return true.  Otherwise, return
-   false to indicate that the caller should read the body of the tree.  */
+   Return the new tree.  */
 
-static bool
-pph_read_tree_header (pph_stream *stream, tree *expr_p, unsigned ix)
+static tree
+pph_read_tree_header (pph_stream *stream, enum LTO_tags tag)
 {
-  enum LTO_tags tag;
-  bool fully_read_p;
+  /* Find data.  */
   struct lto_input_block *ib = stream->encoder.r.ib;
   struct data_in *data_in = stream->encoder.r.data_in;
 
-  tag = streamer_read_record_start (ib);
-  gcc_assert ((unsigned) tag < (unsigned) LTO_NUM_TAGS);
-
-  if (tag == LTO_builtin_decl)
-    {
-      /* If we are going to read a built-in function, all we need is
-	 the code and class.  */
-      *expr_p = streamer_get_builtin_tree (ib, data_in);
-      fully_read_p = true;
-    }
-  else if (tag == lto_tree_code_to_tag (INTEGER_CST))
-    {
-      /* For integer constants we only need the type and its hi/low
-	 words.  */
-      *expr_p = streamer_read_integer_cst (ib, data_in);
-      fully_read_p = true;
-    }
-  else
-    {
       struct bitpack_d bp;
 
-      /* Otherwise, materialize a new node from IB.  This will also read
-	 all the language-independent bitfields for the new tree.  */
-      *expr_p = streamer_alloc_tree (ib, data_in, tag);
+  /* Allocate the tree.  */
+  tree expr = streamer_alloc_tree (ib, data_in, tag);
 
-      /* Read the language-independent bitfields for *EXPR_P.  */
-      bp = streamer_read_tree_bitfields (ib, *expr_p);
+  /* Read the language-independent bitfields for expr.  */
+  bp = streamer_read_tree_bitfields (ib, expr);
 
       /* Unpack all language-dependent bitfields.  */
-      pph_unpack_value_fields (&bp, *expr_p);
+  pph_unpack_value_fields (&bp, expr);
 
-      fully_read_p = false;
-    }
 
-  /* Add *EXPR_P to the pickle cache at slot IX.  */
-  pph_cache_insert_at (&stream->cache, *expr_p, ix);
 
-  return fully_read_p;
+  return expr;
 }
 
 
@@ -2069,15 +2037,20 @@ pph_read_tree_header (pph_stream *stream
    new tree from the PPH stream in DATA_IN.  */
 
 tree
-pph_read_tree (struct lto_input_block *ib ATTRIBUTE_UNUSED,
-	       struct data_in *data_in)
+pph_read_tree (struct lto_input_block *ib_unused ATTRIBUTE_UNUSED,
+	       struct data_in *root_data_in)
 {
-  pph_stream *stream = (pph_stream *) data_in->sdata;
+  /* Find data.  */
+  pph_stream *stream = (pph_stream *) root_data_in->sdata;
+  struct lto_input_block *ib = stream->encoder.r.ib;
+  struct data_in *data_in = stream->encoder.r.data_in;
+
   tree expr;
-  bool fully_read_p;
   enum pph_record_marker marker;
   unsigned image_ix, ix;
+  enum LTO_tags tag;
 
+  /* Read record start and test cache.  */
   marker = pph_in_start_record (stream, &image_ix, &ix);
   if (marker == PPH_RECORD_END)
     return NULL;
@@ -2087,10 +2060,30 @@ pph_read_tree (struct lto_input_block *i
   /* We did not find the tree in the pickle cache, allocate the tree by
      reading the header fields (different tree nodes need to be
      allocated in different ways).  */
-  fully_read_p = pph_read_tree_header (stream, &expr, ix);
-  if (!fully_read_p)
-    pph_read_tree_body (stream, expr);
-
+  tag = streamer_read_record_start (ib);
+  gcc_assert ((unsigned) tag < (unsigned) LTO_NUM_TAGS);
+  if (tag == LTO_builtin_decl)
+    {
+      /* If we are going to read a built-in function, all we need is
+	 the code and class.  */
+      expr = streamer_get_builtin_tree (ib, data_in);
+      pph_cache_insert_at (&stream->cache, expr, ix);
+    }
+  else if (tag == lto_tree_code_to_tag (INTEGER_CST))
+    {
+      /* For integer constants we only need the type and its hi/low
+	 words.  */
+      expr = streamer_read_integer_cst (ib, data_in);
+      pph_cache_insert_at (&stream->cache, expr, ix);
+    }
+  else
+    {
+      /* Otherwise, materialize a new node from IB.  This will also read
+         all the language-independent bitfields for the new tree.  */
+      expr = pph_read_tree_header (stream, tag);
+      pph_cache_insert_at (&stream->cache, expr, ix);
+      pph_read_tree_body (stream, expr);
+    }
   return expr;
 }
 
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 178797)
+++ gcc/cp/pt.c	(working copy)
@@ -20304,7 +20304,7 @@ pph_in_spec_entry_htab (pph_stream *stre
 {
   unsigned count = pph_in_uint (stream);
   if (flag_pph_debug >= 2)
-    fprintf (stderr, "loading %d spec_entries\n", count );
+    fprintf (pph_logfile, "PPH: loading %d spec_entries\n", count );
   for (; count > 0; --count)
     {
       hashval_t hash;
@@ -20327,10 +20327,10 @@ pph_out_spec_entry_tables (pph_stream *s
 {
   pph_out_spec_entry_htab (stream, &decl_specializations);
   if (flag_pph_dump_tree)
-    pph_dump_spec_entry_htab (stderr, "decl", &decl_specializations);
+    pph_dump_spec_entry_htab (pph_logfile, "decl", &decl_specializations);
   pph_out_spec_entry_htab (stream, &type_specializations);
   if (flag_pph_dump_tree)
-    pph_dump_spec_entry_htab (stderr, "type", &type_specializations);
+    pph_dump_spec_entry_htab (pph_logfile, "type", &type_specializations);
 }
 
 /* Load and merge all spec_entry tables from STREAM.  */
@@ -20340,10 +20340,10 @@ pph_in_spec_entry_tables (pph_stream *st
 {
   pph_in_spec_entry_htab (stream, &decl_specializations);
   if (flag_pph_dump_tree)
-    pph_dump_spec_entry_htab (stderr, "decl", &decl_specializations);
+    pph_dump_spec_entry_htab (pph_logfile, "decl", &decl_specializations);
   pph_in_spec_entry_htab (stream, &type_specializations);
   if (flag_pph_dump_tree)
-    pph_dump_spec_entry_htab (stderr, "type", &type_specializations);
+    pph_dump_spec_entry_htab (pph_logfile, "type", &type_specializations);
 }
 
 
Index: gcc/cp/pph-streamer.c
===================================================================
--- gcc/cp/pph-streamer.c	(revision 178797)
+++ gcc/cp/pph-streamer.c	(working copy)
@@ -236,7 +236,8 @@ pph_trace (pph_stream *stream, const voi
 	const_tree t = (const_tree) data;
 	if (t)
           {
-            fprintf (pph_logfile, ", code=%s", tree_code_name[TREE_CODE (t)]);
+            enum tree_code code = TREE_CODE (t);
+            fprintf (pph_logfile, ", code=%s", pph_tree_code_text (code));
             if (DECL_P (t))
               {
                 fprintf (pph_logfile, ", value=");
Index: gcc/cp/pph-streamer-out.c
===================================================================
--- gcc/cp/pph-streamer-out.c	(revision 178797)
+++ gcc/cp/pph-streamer-out.c	(working copy)
@@ -1709,13 +1709,13 @@ pph_write_tree_body (pph_stream *stream,
     case OPTIMIZATION_NODE:
     case TARGET_OPTION_NODE:
       fatal_error ("PPH: unimplemented tree node '%s'",
-		   tree_code_name[TREE_CODE (expr)]);
+		   pph_tree_code_text (TREE_CODE (expr)));
       break;
 
     /* TREES UNRECOGNIZED */
     default:
       fatal_error ("PPH: unrecognized tree node '%s'",
-                   tree_code_name[TREE_CODE (expr)]);
+                   pph_tree_code_text (TREE_CODE (expr)));
     }
 }
 
Index: libcpp/symtab.c
===================================================================
--- libcpp/symtab.c	(revision 178797)
+++ libcpp/symtab.c	(working copy)
@@ -420,7 +420,7 @@ cpp_lt_create (unsigned int order, unsig
 		  (void (*) (void *)) free);
   obstack_alignment_mask (table->strings) = 0;
 
-  table->pth_debug_level = debug;
+  table->debug_level = debug;
   lt_clear_stats (table);
 
   return table;
@@ -452,7 +452,7 @@ lt_statistics (struct cpp_lookaside *tab
 void 
 cpp_lt_destroy (cpp_lookaside *table)
 {
-  if (table->pth_debug_level >= 2)
+  if (table->debug_level >= 1)
     lt_statistics (table);
   if (table->strings)
     {
@@ -541,8 +541,8 @@ lt_query_macro (cpp_reader *reader, cpp_
         }
     }
 
-  if (reader->lookaside_table->pth_debug_level >= 3)
-    fprintf (stderr, "PTH: macro %s is %s\n",
+  if (reader->lookaside_table->debug_level >= 2)
+    fprintf (stderr, "PPH: macro %s is %s\n",
                      (const char *)cpp_node->ident.str,
                      definition);
 
@@ -626,7 +626,7 @@ cpp_lt_capture (cpp_reader *reader)
     {
       /* Allocate a new table.  */
       reader->lookaside_table = cpp_lt_create (aside->sticky_order,
-                                              aside->pth_debug_level);
+					       aside->debug_level);
       cpp_lt_destroy (aside);  /* May also dump statistics.  */
     }
   else
@@ -634,7 +634,7 @@ cpp_lt_capture (cpp_reader *reader)
       /* Reuse the old table.  */
 
       /* Dump out the statistics.  */
-      if (aside->pth_debug_level >= 2)
+      if (aside->debug_level >= 2)
         {
           lt_statistics (aside);
           lt_clear_stats (aside);
Index: libcpp/internal.h
===================================================================
--- libcpp/internal.h	(revision 178797)
+++ libcpp/internal.h	(working copy)
@@ -358,7 +358,7 @@ struct cpp_lookaside {
   unsigned long long empties;		/* Number of table empty/capture.  */
 
   /* Table debugging.  */
-  unsigned int pth_debug_level;
+  unsigned int debug_level;
 };
 
 /* Lookup an identifer in the lookaside table,

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

[Diego Novillo, 2011-09-21 19:01:14]

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

http://codereview.appspot.com/5050045/diff/1/gcc/cp/pph-streamer-in.c#newcode...
gcc/cp/pph-streamer-in.c:2024: /* Read the language-independent bitfields for
expr.  */
s/expr/EXPR/

http://codereview.appspot.com/5050045/diff/1/gcc/cp/pph.c
File gcc/cp/pph.c (right):

http://codereview.appspot.com/5050045/diff/1/gcc/cp/pph.c#newcode50
gcc/cp/pph.c:50: gcc_assert (code <= TEMPLATE_INFO);
  48 pph_tree_code_text (enum tree_code code)
  49 {
  50   gcc_assert (code <= TEMPLATE_INFO);

gcc_assert (code < MAX_TREE_CODES);