[lto] Merge streamer hooks from pph branch.

[Diego Novillo, 2011-06-01 00:12:35]

This patch merges the LTO streamer hooks from the pph branch.

These hooks are meant to separate streaming work that is specific to
GIMPLE from other modules that may want to stream their own data
structures.  In the PPH branch, we are using the streamer to save
front end data structures, so there was a bunch of work and checks
that made no sense in the front end.

All this module-specific work can be moved to the hooks defined in
struct lto_streamer_hooks.

One of the concerns I had with the hooks is potential overhead when
streaming because of the new indirect calls added by the patch.  This
does not seem to be the case.

I did several timing runs using insn-emit.o (the biggest object file
we generate in a bootstrap) and using an LTO link of cc1.  The times
are slightly in favour of the hooks, actually (the patch removes some
checks from the core tree pickling routines), but the difference is
insignificant.

Over 5 runs on an idle system: Without this patch, we compile
insn-emit.o in 41.56 secs (wall time), with the patch the time is
41.34 secs (wall time).

The other change in the patch is to reserve enough tags in enum
LTO_tags to cover all the possible tree codes.  This exposes the bug
that I fixed yesterday in the pph branch (the sign of the shifts in
lto_output_int_in_range).  This means that we need to use
output_record_start (LTO_null) instead of output_zero to output
0-delimiters, which increases the size of the output by less than 1%
on average.

Tested with LTO profiledbootstrap on x86_64.  OK for trunk?


Diego.


	* Makefile.in (cgraphunit.o): Add dependency on LTO_STREAMER_H.
	* cgraphunit.c: Include lto-streamer.h
	(cgraph_finalize_compilation_unit): Call gimple_streamer_hooks_init
	if LTO is enabled.
	* lto-streamer-in.c (unpack_value_fields): Do not check
	for TS_SSA_NAME, TS_STATEMENT_LIST and TS_OMP_CLAUSE.
	Call lto_streamer_hooks.unpack_value_fields if set.
	(lto_materialize_tree): For unhandled nodes, first try to
	call lto_streamer_hooks.alloc_tree, if it exists.
	(lto_input_ts_decl_common_tree_pointers): Move reading of
	DECL_INITIAL to gimple_streamer_read_tree.
	(lto_input_tree_pointers): Do not handle TS_SSA_NAME,
	TS_STATEMENT_LIST, TS_OMP_CLAUSE and TS_OPTIMIZATION.
	(lto_read_tree): Call lto_streamer_hooks.read_tree if
	set.
	Only register symbols in symtab if
	lto_streamer_hooks.register_decls_in_symtab_p is set.
	(gimple_streamer_read_tree): New.
	(lto_reader_init): Rename from lto_init_reader.
	Move initialization code to gimple_streamer_reader_init.
	Call lto_streamer_hooks.reader_init if set.
	(gimple_streamer_reader_init): New.
	* lto-streamer-out.c (pack_value_fields): Do not handle
	TS_SSA_NAME, TS_STATEMENT_LIST and TS_OMP_CLAUSE.
	Call lto_streamer_hooks.pack_value_fields if set.
	(lto_output_tree_ref): For tree nodes that are not
	normally indexable, call
	lto_streamer_hooks.indexable_with_decls_p before giving
	up.
	(lto_output_ts_decl_common_tree_pointers): Move handling
	for FUNCTION_DECL and TRANSLATION_UNIT_DECL to
	gimple_streamer_write_tree.
	Move assertion for NULL DECL_SAVED_TREEs to
	gimple_streamer_write_tree.
	(lto_output_ts_decl_with_vis_tree_pointers): Call
	output_record_start with LTO_null instead of output_zero.
	(lto_output_ts_binfo_tree_pointers): Likewise.
	(lto_output_tree): Likewise.
	(output_eh_try_list): Likewise.
	(output_eh_region): Likewise.
	(output_eh_lp): Likewise.
	(output_eh_regions): Likewise.
	(output_bb): Likewise.
	(output_function): Likewise.
	(output_unreferenced_globals): Likewise.
	(lto_output_tree_pointers): Do not handle TS_SSA_NAME,
	TS_STATEMENT_LIST, TS_OMP_CLAUSE and TS_OPTIMIZATION.
	(lto_output_tree_header): Call
	lto_streamer_hooks.is_streamable instead of
	lto_is_streamable.
	Call lto_streamer_hooks.output_tree_header if set.
	(lto_write_tree): Call lto_streamer_hooks.write_tree if
	set.
	(gimple_streamer_write_tree): New.
	(lto_output_tree): If
	lto_streamer_hooks.has_unique_integer_csts_p is set,
	lookup the constant in the streamer cache first.
	* lto-streamer.c (lto_streamer_cache_create): Call
	lto_streamer_hooks.preload_common_nodes instead of
	lto_preload_common_nodes.
	(lto_is_streamable): Move from lto-streamer.h
	(gimple_streamer_hooks_init): New.
	(streamer_hooks): New.
	(streamer_hooks_): Declare.
	(streamer_hooks_init): New.
	* lto-streamer.h (struct output_block): Forward declare.
	(struct lto_input_block): Likewise.
	(struct data_in): Likewise.
	(struct bitpack_d): Likewise.
	(enum LTO_tags): Reserve MAX_TREE_CODES
	to cover all front end codes as well.
	(struct lto_streamer_hooks): Declare.
	(gimple_streamer_hooks_init): Declare.
	(gimple_streamer_write_tree): Declare.
	(gimple_streamer_read_tree): Declare.
	(streamer_hooks): Declare.
	(streamer_hooks_init): Declare.
	(lto_tag_is_tree_code_p): Check max value against MAX_TREE_CODES.
	(lto_is_streamable): Move to lto-streamer.c
	(lto_output_int_in_range): Change << to >> when shifting VAL.

lto/ChangeLog
2011-05-31  Diego Novillo  <dnovillo@google.com>

	* lto.c (lto_main): Call gimple_streamer_hooks_init.

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 834acb1..22fde67 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -140,6 +140,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "plugin.h"
 #include "ipa-inline.h"
 #include "ipa-utils.h"
+#include "lto-streamer.h"
 
 static void cgraph_expand_all_functions (void);
 static void cgraph_mark_functions_to_output (void);
@@ -1092,6 +1093,10 @@ cgraph_finalize_compilation_unit (void)
 {
   timevar_push (TV_CGRAPH);
 
+  /* If LTO is enabled, initialize the streamer hooks needed by GIMPLE.  */
+  if (flag_lto)
+    gimple_streamer_hooks_init ();
+
   /* If we're here there's no current function anymore.  Some frontends
      are lazy in clearing these.  */
   current_function_decl = NULL;
diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
index 88966f2..801fe6f 100644
--- a/gcc/lto-streamer-in.c
+++ b/gcc/lto-streamer-in.c
@@ -1833,6 +1833,7 @@ static void
 unpack_value_fields (struct bitpack_d *bp, tree expr)
 {
   enum tree_code code;
+  lto_streamer_hooks *h = streamer_hooks ();
 
   code = TREE_CODE (expr);
 
@@ -1864,26 +1865,11 @@ unpack_value_fields (struct bitpack_d *bp, tree expr)
   if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
     unpack_ts_block_value_fields (bp, expr);
 
-  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
-    {
-      /* We only stream the version number of SSA names.  */
-      gcc_unreachable ();
-    }
-
-  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
-    {
-      /* This is only used by GENERIC.  */
-      gcc_unreachable ();
-    }
-
-  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
-    {
-      /* This is only used by High GIMPLE.  */
-      gcc_unreachable ();
-    }
-
   if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
     unpack_ts_translation_unit_decl_value_fields (bp, expr);
+
+  if (h->unpack_value_fields)
+    h->unpack_value_fields (bp, expr);
 }
 
 
@@ -1935,8 +1921,17 @@ lto_materialize_tree (struct lto_input_block *ib, struct
data_in *data_in,
     }
   else
     {
-      /* All other nodes can be materialized with a raw make_node call.  */
-      result = make_node (code);
+      lto_streamer_hooks *h = streamer_hooks ();
+
+      /* For all other nodes, see if the streamer knows how to allocate
+	 it.  */
+      if (h->alloc_tree)
+	result = h->alloc_tree (code, ib, data_in);
+
+      /* If the hook did not handle it, materialize the tree with a raw
+	 make_node call.  */
+      if (result == NULL_TREE)
+	result = make_node (code);
     }
 
 #ifdef LTO_STREAMER_DEBUG
@@ -2031,12 +2026,8 @@ lto_input_ts_decl_common_tree_pointers (struct
lto_input_block *ib,
 {
   DECL_SIZE (expr) = lto_input_tree (ib, data_in);
   DECL_SIZE_UNIT (expr) = lto_input_tree (ib, data_in);
-
-  if (TREE_CODE (expr) != FUNCTION_DECL
-      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
-    DECL_INITIAL (expr) = lto_input_tree (ib, data_in);
-
   DECL_ATTRIBUTES (expr) = lto_input_tree (ib, data_in);
+
   /* Do not stream DECL_ABSTRACT_ORIGIN.  We cannot handle debug information
      for early inlining so drop it on the floor instead of ICEing in
      dwarf2out.c.  */
@@ -2433,38 +2424,15 @@ lto_input_tree_pointers (struct lto_input_block *ib,
struct data_in *data_in,
   if (CODE_CONTAINS_STRUCT (code, TS_EXP))
     lto_input_ts_exp_tree_pointers (ib, data_in, expr);
 
-  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
-    {
-      /* We only stream the version number of SSA names.  */
-      gcc_unreachable ();
-    }
-
   if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
     lto_input_ts_block_tree_pointers (ib, data_in, expr);
 
   if (CODE_CONTAINS_STRUCT (code, TS_BINFO))
     lto_input_ts_binfo_tree_pointers (ib, data_in, expr);
 
-  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
-    {
-      /* This should only appear in GENERIC.  */
-      gcc_unreachable ();
-    }
-
   if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
     lto_input_ts_constructor_tree_pointers (ib, data_in, expr);
 
-  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
-    {
-      /* This should only appear in High GIMPLE.  */
-      gcc_unreachable ();
-    }
-
-  if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
-    {
-      sorry ("optimization options not supported yet");
-    }
-
   if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
     lto_input_ts_target_option (ib, expr);
 
@@ -2655,20 +2623,29 @@ lto_read_tree (struct lto_input_block *ib, struct
data_in *data_in,
 	       enum LTO_tags tag)
 {
   tree result;
+  lto_streamer_hooks *h = streamer_hooks ();
 
   result = lto_materialize_tree (ib, data_in, tag);
 
   /* Read all the pointer fields in RESULT.  */
   lto_input_tree_pointers (ib, data_in, result);
 
+  /* Call back into the streaming module to read anything else it
+     may need.  */
+  if (h->read_tree)
+    h->read_tree (ib, data_in, result);
+
   /* We should never try to instantiate an MD or NORMAL builtin here.  */
   if (TREE_CODE (result) == FUNCTION_DECL)
     gcc_assert (!lto_stream_as_builtin_p (result));
 
-  if (TREE_CODE (result) == VAR_DECL)
-    lto_register_var_decl_in_symtab (data_in, result);
-  else if (TREE_CODE (result) == FUNCTION_DECL && !DECL_BUILT_IN (result))
-    lto_register_function_decl_in_symtab (data_in, result);
+  if (h->register_decls_in_symtab_p)
+    {
+      if (TREE_CODE (result) == VAR_DECL)
+	lto_register_var_decl_in_symtab (data_in, result);
+      else if (TREE_CODE (result) == FUNCTION_DECL && !DECL_BUILT_IN (result))
+	lto_register_function_decl_in_symtab (data_in, result);
+    }
 
   /* end_marker = */ lto_input_1_unsigned (ib);
 
@@ -2682,6 +2659,22 @@ lto_read_tree (struct lto_input_block *ib, struct data_in
*data_in,
 }
 
 
+/* LTO streamer hook for reading GIMPLE trees.  IB and DATA_IN are as in
+   lto_read_tree.  EXPR is the tree was materialized by lto_read_tree and
+   needs GIMPLE specific data to be filled in.  */
+
+void
+gimple_streamer_read_tree (struct lto_input_block *ib,
+			   struct data_in *data_in,
+			   tree expr)
+{
+  if (DECL_P (expr)
+      && TREE_CODE (expr) != FUNCTION_DECL
+      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
+    DECL_INITIAL (expr) = lto_input_tree (ib, data_in);
+}
+
+
 /* Read and INTEGER_CST node from input block IB using the per-file
    context in DATA_IN.  */
 
@@ -2763,16 +2756,23 @@ lto_input_tree (struct lto_input_block *ib, struct
data_in *data_in)
 /* Initialization for the LTO reader.  */
 
 void
-lto_init_reader (void)
+lto_reader_init (void)
 {
   lto_streamer_init ();
-
-  memset (&lto_stats, 0, sizeof (lto_stats));
-  bitmap_obstack_initialize (NULL);
-
   file_name_hash_table = htab_create (37, hash_string_slot_node,
 				      eq_string_slot_node, free);
+  if (streamer_hooks ()->reader_init)
+    streamer_hooks ()->reader_init ();
+}
+
 
+/* GIMPLE streamer hook for initializing the LTO reader.  */
+
+void
+gimple_streamer_reader_init (void)
+{
+  memset (&lto_stats, 0, sizeof (lto_stats));
+  bitmap_obstack_initialize (NULL);
   gimple_register_cfg_hooks ();
 }
 
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index b3b81bd..90e3252 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -558,6 +558,7 @@ static void
 pack_value_fields (struct bitpack_d *bp, tree expr)
 {
   enum tree_code code;
+  lto_streamer_hooks *h = streamer_hooks ();
 
   code = TREE_CODE (expr);
 
@@ -589,26 +590,11 @@ pack_value_fields (struct bitpack_d *bp, tree expr)
   if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
     pack_ts_block_value_fields (bp, expr);
 
-  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
-    {
-      /* We only stream the version number of SSA names.  */
-      gcc_unreachable ();
-    }
-
-  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
-    {
-      /* This is only used by GENERIC.  */
-      gcc_unreachable ();
-    }
-
-  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
-    {
-      /* This is only used by High GIMPLE.  */
-      gcc_unreachable ();
-    }
-
   if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
     pack_ts_translation_unit_decl_value_fields (bp, expr);
+
+  if (h->pack_value_fields)
+    h->pack_value_fields (bp, expr);
 }
 
 
@@ -772,9 +758,23 @@ lto_output_tree_ref (struct output_block *ob, tree expr)
       break;
 
     default:
-      /* No other node is indexable, so it should have been handled
-	 by lto_output_tree.  */
-      gcc_unreachable ();
+      {
+	lto_streamer_hooks *h = streamer_hooks ();
+
+	/* See if streamer hooks allows this node to be indexable with
+	   VAR_DECLs.  */
+	if (h->indexable_with_decls_p && h->indexable_with_decls_p (expr))
+	  {
+	    output_record_start (ob, LTO_global_decl_ref);
+	    lto_output_var_decl_index (ob->decl_state, ob->main_stream, expr);
+	  }
+	else
+	  {
+	    /* No other node is indexable, so it should have been
+	      handled by lto_output_tree.  */
+	    gcc_unreachable ();
+	  }
+      }
     }
 }
 
@@ -883,27 +883,10 @@ lto_output_ts_decl_common_tree_pointers (struct
output_block *ob, tree expr,
   lto_output_tree_or_ref (ob, DECL_SIZE (expr), ref_p);
   lto_output_tree_or_ref (ob, DECL_SIZE_UNIT (expr), ref_p);
 
-  if (TREE_CODE (expr) != FUNCTION_DECL
-      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
-    {
-      tree initial = DECL_INITIAL (expr);
-      if (TREE_CODE (expr) == VAR_DECL
-	  && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
-	  && initial)
-	{
-	  lto_varpool_encoder_t varpool_encoder =
ob->decl_state->varpool_node_encoder;
-	  struct varpool_node *vnode = varpool_get_node (expr);
-	  if (!vnode)
-	    initial = error_mark_node;
-	  else if (!lto_varpool_encoder_encode_initializer_p (varpool_encoder,
-							      vnode))
-	    initial = NULL;
-	}
-    
-      lto_output_tree_or_ref (ob, initial, ref_p);
-    }
+  /* Do not stream DECL_INITIAL.  */
 
   lto_output_tree_or_ref (ob, DECL_ATTRIBUTES (expr), ref_p);
+
   /* Do not stream DECL_ABSTRACT_ORIGIN.  We cannot handle debug information
      for early inlining so drop it on the floor instead of ICEing in
      dwarf2out.c.  */
@@ -931,11 +914,6 @@ lto_output_ts_decl_non_common_tree_pointers (struct
output_block *ob,
 {
   if (TREE_CODE (expr) == FUNCTION_DECL)
     {
-      /* DECL_SAVED_TREE holds the GENERIC representation for DECL.
-	 At this point, it should not exist.  Either because it was
-	 converted to gimple or because DECL didn't have a GENERIC
-	 representation in this TU.  */
-      gcc_assert (DECL_SAVED_TREE (expr) == NULL_TREE);
       lto_output_tree_or_ref (ob, DECL_ARGUMENTS (expr), ref_p);
       lto_output_tree_or_ref (ob, DECL_RESULT (expr), ref_p);
     }
@@ -955,7 +933,7 @@ lto_output_ts_decl_with_vis_tree_pointers (struct
output_block *ob, tree expr,
   if (DECL_ASSEMBLER_NAME_SET_P (expr))
     lto_output_tree_or_ref (ob, DECL_ASSEMBLER_NAME (expr), ref_p);
   else
-    output_zero (ob);
+    output_record_start (ob, LTO_null);
 
   lto_output_tree_or_ref (ob, DECL_SECTION_NAME (expr), ref_p);
   lto_output_tree_or_ref (ob, DECL_COMDAT_GROUP (expr), ref_p);
@@ -1136,7 +1114,7 @@ lto_output_ts_binfo_tree_pointers (struct output_block
*ob, tree expr,
      is needed to build the empty BINFO node on the reader side.  */
   FOR_EACH_VEC_ELT (tree, BINFO_BASE_BINFOS (expr), i, t)
     lto_output_tree_or_ref (ob, t, ref_p);
-  output_zero (ob);
+  output_record_start (ob, LTO_null);
 
   lto_output_tree_or_ref (ob, BINFO_OFFSET (expr), ref_p);
   lto_output_tree_or_ref (ob, BINFO_VTABLE (expr), ref_p);
@@ -1256,12 +1234,6 @@ lto_output_tree_pointers (struct output_block *ob, tree
expr, bool ref_p)
   if (CODE_CONTAINS_STRUCT (code, TS_EXP))
     lto_output_ts_exp_tree_pointers (ob, expr, ref_p);
 
-  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
-    {
-      /* We only stream the version number of SSA names.  */
-      gcc_unreachable ();
-    }
-
   if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
     lto_output_ts_block_tree_pointers (ob, expr, ref_p);
 
@@ -1271,21 +1243,6 @@ lto_output_tree_pointers (struct output_block *ob, tree
expr, bool ref_p)
   if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
     lto_output_ts_constructor_tree_pointers (ob, expr, ref_p);
 
-  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
-    {
-      /* This should only appear in GENERIC.  */
-      gcc_unreachable ();
-    }
-
-  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
-    {
-      /* This should only appear in High GIMPLE.  */
-      gcc_unreachable ();
-    }
-
-  if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
-    sorry ("gimple bytecode streams do not support the optimization
attribute");
-
   if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
     lto_output_ts_target_option (ob, expr);
 
@@ -1304,18 +1261,20 @@ lto_output_tree_header (struct output_block *ob, tree
expr)
 {
   enum LTO_tags tag;
   enum tree_code code;
+  lto_streamer_hooks *h = streamer_hooks ();
 
   /* We should not see any non-GIMPLE tree nodes here.  */
   code = TREE_CODE (expr);
-  if (!lto_is_streamable (expr))
-    internal_error ("tree code %qs is not supported in gimple streams",
-		    tree_code_name[code]);
+  if (h->is_streamable && !h->is_streamable (expr))
+    internal_error ("tree code %qs is not supported in %s streams",
+		    h->name, tree_code_name[code]);
 
   /* The header of a tree node consists of its tag, the size of
      the node, and any other information needed to instantiate
      EXPR on the reading side (such as the number of slots in
      variable sized nodes).  */
   tag = lto_tree_code_to_tag (code);
+  gcc_assert ((unsigned) tag < (unsigned) LTO_NUM_TAGS);
   output_record_start (ob, tag);
 
   /* The following will cause bootstrap miscomparisons.  Enable with care.  */
@@ -1338,6 +1297,11 @@ lto_output_tree_header (struct output_block *ob, tree
expr)
     output_sleb128 (ob, TREE_VEC_LENGTH (expr));
   else if (CODE_CONTAINS_STRUCT (code, TS_BINFO))
     output_uleb128 (ob, BINFO_N_BASE_BINFOS (expr));
+
+  /* Allow the streamer to write any streamer-specific information
+     needed to instantiate the node when reading.  */
+  if (h->output_tree_header)
+    h->output_tree_header (ob, expr);
 }
 
 
@@ -1385,6 +1349,7 @@ static void
 lto_write_tree (struct output_block *ob, tree expr, bool ref_p)
 {
   struct bitpack_d bp;
+  lto_streamer_hooks *h = streamer_hooks ();
 
   /* Write the header, containing everything needed to materialize
      EXPR on the reading side.  */
@@ -1399,11 +1364,58 @@ lto_write_tree (struct output_block *ob, tree expr, bool
ref_p)
   /* Write all the pointer fields in EXPR.  */
   lto_output_tree_pointers (ob, expr, ref_p);
 
+  /* Call back into the streaming module to see if it needs to write
+     anything that was not written by the common streamer.  */
+  if (h->write_tree)
+    h->write_tree (ob, expr, ref_p);
+
   /* Mark the end of EXPR.  */
   output_zero (ob);
 }
 
 
+/* GIMPLE hook for writing GIMPLE-specific parts of trees.  OB, EXPR
+   and REF_P are as in lto_write_tree.  */
+
+void
+gimple_streamer_write_tree (struct output_block *ob, tree expr, bool ref_p)
+{
+  if (TREE_CODE (expr) == FUNCTION_DECL)
+    {
+      /* DECL_SAVED_TREE holds the GENERIC representation for DECL.
+	 At this point, it should not exist.  Either because it was
+	 converted to gimple or because DECL didn't have a GENERIC
+	 representation in this TU.  */
+      gcc_assert (DECL_SAVED_TREE (expr) == NULL_TREE);
+    }
+
+  if (DECL_P (expr)
+      && TREE_CODE (expr) != FUNCTION_DECL
+      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
+    {
+      /* Handle DECL_INITIAL for symbols.  */
+      tree initial = DECL_INITIAL (expr);
+      if (TREE_CODE (expr) == VAR_DECL
+	  && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
+	  && initial)
+	{
+	  lto_varpool_encoder_t varpool_encoder;
+	  struct varpool_node *vnode;
+
+	  varpool_encoder = ob->decl_state->varpool_node_encoder;
+	  vnode = varpool_get_node (expr);
+	  if (!vnode)
+	    initial = error_mark_node;
+	  else if (!lto_varpool_encoder_encode_initializer_p (varpool_encoder,
+							      vnode))
+	    initial = NULL;
+	}
+
+      lto_output_tree_or_ref (ob, initial, ref_p);
+    }
+}
+
+
 /* Emit the integer constant CST to output block OB.  If REF_P is true,
    CST's type will be emitted as a reference.  */
 
@@ -1430,7 +1442,7 @@ lto_output_tree (struct output_block *ob, tree expr, bool
ref_p)
 
   if (expr == NULL_TREE)
     {
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
       return;
     }
 
@@ -1438,8 +1450,27 @@ lto_output_tree (struct output_block *ob, tree expr, bool
ref_p)
      to be materialized by the reader (to implement TYPE_CACHED_VALUES).  */
   if (TREE_CODE (expr) == INTEGER_CST)
     {
-      lto_output_integer_cst (ob, expr, ref_p);
-      return;
+      bool is_special;
+
+     /* There are some constants that are special to the streamer
+	(e.g., void_zero_node, truthvalue_false_node).
+	These constants cannot be rematerialized with
+	build_int_cst_wide because they may actually lack a type (like
+	void_zero_node) and they need to be pointer-identical to trees
+	materialized by the compiler tables like global_trees or
+	c_global_trees.
+
+	If the streamer told us that it has special constants, they
+	will be preloaded in the streamer cache.  If we find a match,
+	then stream the constant as a reference so the reader can
+	re-materialize it from the cache.  */
+      is_special = streamer_hooks ()->has_unique_integer_csts_p
+		   && lto_streamer_cache_lookup (ob->writer_cache, expr, NULL);
+      if (!is_special)
+	{
+	  lto_output_integer_cst (ob, expr, ref_p);
+	  return;
+	}
     }
 
   existed_p = lto_streamer_cache_insert (ob->writer_cache, expr, &ix);
@@ -1486,7 +1517,7 @@ output_eh_try_list (struct output_block *ob, eh_catch
first)
       lto_output_tree_ref (ob, n->label);
     }
 
-  output_zero (ob);
+  output_record_start (ob, LTO_null);
 }
 
 
@@ -1501,7 +1532,7 @@ output_eh_region (struct output_block *ob, eh_region r)
 
   if (r == NULL)
     {
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
       return;
     }
 
@@ -1564,7 +1595,7 @@ output_eh_lp (struct output_block *ob, eh_landing_pad lp)
 {
   if (lp == NULL)
     {
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
       return;
     }
 
@@ -1633,9 +1664,9 @@ output_eh_regions (struct output_block *ob, struct
function *fn)
 	}
     }
 
-  /* The 0 either terminates the record or indicates that there are no
-     eh_records at all.  */
-  output_zero (ob);
+  /* The LTO_null either terminates the record or indicates that there
+     are no eh_records at all.  */
+  output_record_start (ob, LTO_null);
 }
 
 
@@ -1880,10 +1911,10 @@ output_bb (struct output_block *ob, basic_block bb,
struct function *fn)
 	      output_sleb128 (ob, region);
 	    }
 	  else
-	    output_zero (ob);
+	    output_record_start (ob, LTO_null);
 	}
 
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
 
       for (bsi = gsi_start_phis (bb); !gsi_end_p (bsi); gsi_next (&bsi))
 	{
@@ -1896,7 +1927,7 @@ output_bb (struct output_block *ob, basic_block bb, struct
function *fn)
 	    output_phi (ob, phi);
 	}
 
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
     }
 }
 
@@ -2053,7 +2084,7 @@ output_function (struct cgraph_node *node)
     output_bb (ob, bb, fn);
 
   /* The terminator for this function.  */
-  output_zero (ob);
+  output_record_start (ob, LTO_null);
 
   output_cfg (ob, fn);
 
@@ -2167,7 +2198,7 @@ output_unreferenced_globals (cgraph_node_set set,
varpool_node_set vset)
       }
   symbol_alias_set_destroy (defined);
 
-  output_zero (ob);
+  output_record_start (ob, LTO_null);
 
   produce_asm (ob, NULL);
   destroy_output_block (ob);
@@ -2238,6 +2269,8 @@ static void
 lto_writer_init (void)
 {
   lto_streamer_init ();
+  if (streamer_hooks ()->writer_init)
+    streamer_hooks ()->writer_init ();
 }
 
 
diff --git a/gcc/lto-streamer.c b/gcc/lto-streamer.c
index abccd10..4281bd9 100644
--- a/gcc/lto-streamer.c
+++ b/gcc/lto-streamer.c
@@ -579,7 +579,7 @@ lto_streamer_cache_create (void)
   /* Load all the well-known tree nodes that are always created by
      the compiler on startup.  This prevents writing them out
      unnecessarily.  */
-  lto_preload_common_nodes (cache);
+  streamer_hooks ()->preload_common_nodes (cache);
 
   return cache;
 }
@@ -724,3 +724,70 @@ lto_check_version (int major, int minor)
 		 major, minor,
 		 LTO_major_version, LTO_minor_version);
 }
+
+
+/* Return true if EXPR is a tree node that can be written to disk.  */
+
+static inline bool
+lto_is_streamable (tree expr)
+{
+  enum tree_code code = TREE_CODE (expr);
+
+  /* Notice that we reject SSA_NAMEs as well.  We only emit the SSA
+     name version in lto_output_tree_ref (see output_ssa_names).  */
+  return !is_lang_specific (expr)
+	 && code != SSA_NAME
+	 && code != CALL_EXPR
+	 && code != LANG_TYPE
+	 && code != MODIFY_EXPR
+	 && code != INIT_EXPR
+	 && code != TARGET_EXPR
+	 && code != BIND_EXPR
+	 && code != WITH_CLEANUP_EXPR
+	 && code != STATEMENT_LIST
+	 && code != OMP_CLAUSE
+	 && code != OPTIMIZATION_NODE
+	 && (code == CASE_LABEL_EXPR
+	     || code == DECL_EXPR
+	     || TREE_CODE_CLASS (code) != tcc_statement);
+}
+
+
+/* Initialize all the streamer hooks used for streaming GIMPLE.  */
+
+void
+gimple_streamer_hooks_init (void)
+{
+  lto_streamer_hooks *h = streamer_hooks_init ();
+  h->name = "gimple";
+  h->reader_init = gimple_streamer_reader_init;
+  h->writer_init = NULL;
+  h->preload_common_nodes = lto_preload_common_nodes;
+  h->is_streamable = lto_is_streamable;
+  h->write_tree = gimple_streamer_write_tree;
+  h->read_tree = gimple_streamer_read_tree;
+  h->register_decls_in_symtab_p = true;
+}
+
+
+/* Return the current set of streamer hooks.  Note that only one set
+   of streamer hooks can be active at a time.  */
+
+static lto_streamer_hooks streamer_hooks_;
+
+lto_streamer_hooks *
+streamer_hooks (void)
+{
+  return &streamer_hooks_;
+}
+
+
+/* Initialize and return the current set of streamer hooks.  */
+
+lto_streamer_hooks *
+streamer_hooks_init (void)
+{
+  lto_streamer_hooks *h = streamer_hooks ();
+  memset (h, 0, sizeof (lto_streamer_hooks));
+  return h;
+}
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index e8410d4..035aa5e 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -33,6 +33,12 @@ along with GCC; see the file COPYING3.  If not see
 #include "alloc-pool.h"
 #include "gcov-io.h"
 
+/* Forward declarations to avoid including unnecessary headers.  */
+struct output_block;
+struct lto_input_block;
+struct data_in;
+struct bitpack_d;
+
 /* Define when debugging the LTO streamer.  This causes the writer
    to output the numeric value for the memory address of the tree node
    being emitted.  When debugging a problem in the reader, check the
@@ -186,7 +192,7 @@ enum LTO_tags
 
      Conversely, to map between LTO tags and tree/gimple codes, the
      reverse operation must be applied.  */
-  LTO_bb0 = 1 + NUM_TREE_CODES + LAST_AND_UNUSED_GIMPLE_CODE,
+  LTO_bb0 = 1 + MAX_TREE_CODES + LAST_AND_UNUSED_GIMPLE_CODE,
   LTO_bb1,
 
   /* EH region holding the previous statement.  */
@@ -741,6 +747,98 @@ struct data_in
 };
 
 
+/* Streamer hooks.  These functions do additional processing as
+   needed by the module.  There are two types of callbacks, those that
+   replace the default behavior and those that supplement it.  Any
+   or all of these hooks can be NULL.  */
+typedef struct lto_streamer_hooks {
+  /* A string identifying this streamer.  */
+  const char *name;
+
+  /* Called by lto_streamer_cache_create to instantiate a cache of
+     well-known nodes.  These are tree nodes that are always
+     instantiated by the compiler on startup.  Additionally, these
+     nodes need to be shared.  This function should call
+     lto_streamer_cache_append on every tree node that it wishes to
+     preload in the streamer cache.  This way, the writer will only
+     write out a reference to the tree and the reader will instantiate
+     the tree out of this pre-populated cache.  */
+  void (*preload_common_nodes) (struct lto_streamer_cache_d *);
+
+  /* Called by lto_reader_init after it does basic initialization.  */
+  void (*reader_init) (void);
+
+  /* Called by lto_writer_init after it does basic initalization.  */
+  void (*writer_init) (void);
+
+  /* Return true if the given tree is supported by this streamer.  */
+  bool (*is_streamable) (tree);
+
+  /* Called by lto_write_tree after writing all the common parts of
+     a tree.  If defined, the callback is in charge of writing all
+     the fields that lto_write_tree did not write out.  Arguments
+     are as in lto_write_tree.
+
+     The following tree fields are not handled by common code:
+
+	DECL_ABSTRACT_ORIGIN
+	DECL_INITIAL
+	DECL_SAVED_TREE
+
+     Callbacks may choose to ignore or handle them.  If handled,
+     the reader should read them in the exact same sequence written
+     by the writer.  */
+  void (*write_tree) (struct output_block *, tree, bool);
+
+  /* Called by lto_read_tree after reading all the common parts of
+     a tree.  If defined, the callback is in charge of reading all
+     the fields that lto_read_tree did not read in.  Arguments
+     are as in lto_read_tree.  */
+  void (*read_tree) (struct lto_input_block *, struct data_in *, tree);
+
+  /* Called by lto_output_tree_ref to determine if the given tree node
+     should be emitted as a reference to the table of declarations
+     (the same table that holds VAR_DECLs).  */
+  bool (*indexable_with_decls_p) (tree);
+
+  /* Called by pack_value_fields to store any non-pointer fields
+     in the tree structure.  The arguments are as in pack_value_fields.  */
+  void (*pack_value_fields) (struct bitpack_d *, tree);
+
+  /* Called by unpack_value_fields to retrieve any non-pointer fields
+     in the tree structure.  The arguments are as in unpack_value_fields.  */
+  void (*unpack_value_fields) (struct bitpack_d *, tree);
+
+  /* Non-zero if the streamer should register decls in the LTO
+     global symbol tables.  */
+  unsigned register_decls_in_symtab_p : 1;
+
+  /* Non-zero if the streamer has special constants that cannot be
+     shared and are used in pointer-equality tests (e.g., void_zero_node,
+     truthvalue_false_node, etc).  These constants will be present in
+     the streamer cache and should be streamed as references.  */
+  unsigned has_unique_integer_csts_p : 1;
+
+  /* Called by lto_materialize_tree for tree nodes that it does not
+     know how to allocate memory for.  If defined, this hook should
+     return a new tree node of the given code.  The data_in and
+     input_block arguments are passed in case the hook needs to
+     read more data from the stream to allocate the node.
+     If this hook returns NULL, then lto_materialize_tree will attempt
+     to allocate the tree by calling make_node directly.  */
+  tree (*alloc_tree) (enum tree_code, struct lto_input_block *,
+                      struct data_in *);
+
+  /* Called by lto_output_tree_header to write any streamer-specific
+     information needed to allocate the tree.  This hook may assume
+     that the basic header data (tree code, etc) has already been
+     written.  It should only write any extra data needed to allocate
+     the node (e.g., in the case of CALL_EXPR, this hook would write
+     the number of arguments to the CALL_EXPR).  */
+  void (*output_tree_header) (struct output_block *, tree);
+} lto_streamer_hooks;
+
+
 /* In lto-section-in.c  */
 extern struct lto_input_block * lto_create_simple_input_block (
 			       struct lto_file_decl_data *,
@@ -851,17 +949,21 @@ extern intptr_t lto_orig_address_get (tree);
 extern void lto_orig_address_remove (tree);
 #endif
 extern void lto_check_version (int, int);
-
+extern void gimple_streamer_hooks_init (void);
+extern void gimple_streamer_write_tree (struct output_block *, tree, bool);
+extern void gimple_streamer_read_tree (struct lto_input_block *,
+				       struct data_in *, tree);
+extern lto_streamer_hooks *streamer_hooks (void);
+extern lto_streamer_hooks *streamer_hooks_init (void);
 
 /* In lto-streamer-in.c */
 extern void lto_input_cgraph (struct lto_file_decl_data *, const char *);
-extern void lto_init_reader (void);
+extern void lto_reader_init (void);
 extern tree lto_input_tree (struct lto_input_block *, struct data_in *);
 extern void lto_input_function_body (struct lto_file_decl_data *, tree,
 				     const char *);
 extern void lto_input_constructors_and_inits (struct lto_file_decl_data *,
 					      const char *);
-extern void lto_init_reader (void);
 extern struct data_in *lto_data_in_create (struct lto_file_decl_data *,
 				    const char *, unsigned,
 				    VEC(ld_plugin_symbol_resolution_t,heap) *);
@@ -869,6 +971,7 @@ extern void lto_data_in_delete (struct data_in *);
 extern const char *lto_input_string (struct data_in *,
 				     struct lto_input_block *);
 extern void lto_input_data_block (struct lto_input_block *, void *, size_t);
+extern void gimple_streamer_reader_init (void);
 
 
 /* In lto-streamer-out.c  */
@@ -957,7 +1060,7 @@ extern VEC(lto_out_decl_state_ptr, heap)
*lto_function_decl_states;
 static inline bool
 lto_tag_is_tree_code_p (enum LTO_tags tag)
 {
-  return tag > LTO_null && (unsigned) tag <= NUM_TREE_CODES;
+  return tag > LTO_null && (unsigned) tag <= MAX_TREE_CODES;
 }
 
 
@@ -1063,29 +1166,6 @@ lto_stream_as_builtin_p (tree expr)
 	      || DECL_BUILT_IN_CLASS (expr) == BUILT_IN_MD));
 }
 
-/* Return true if EXPR is a tree node that can be written to disk.  */
-static inline bool
-lto_is_streamable (tree expr)
-{
-  enum tree_code code = TREE_CODE (expr);
-
-  /* Notice that we reject SSA_NAMEs as well.  We only emit the SSA
-     name version in lto_output_tree_ref (see output_ssa_names).  */
-  return !is_lang_specific (expr)
-	 && code != SSA_NAME
-	 && code != CALL_EXPR
-	 && code != LANG_TYPE
-	 && code != MODIFY_EXPR
-	 && code != INIT_EXPR
-	 && code != TARGET_EXPR
-	 && code != BIND_EXPR
-	 && code != WITH_CLEANUP_EXPR
-	 && code != STATEMENT_LIST
-	 && (code == CASE_LABEL_EXPR
-	     || code == DECL_EXPR
-	     || TREE_CODE_CLASS (code) != tcc_statement);
-}
-
 DEFINE_DECL_STREAM_FUNCS (TYPE, type)
 DEFINE_DECL_STREAM_FUNCS (FIELD_DECL, field_decl)
 DEFINE_DECL_STREAM_FUNCS (FN_DECL, fn_decl)
@@ -1226,11 +1306,11 @@ lto_output_int_in_range (struct lto_output_stream *obs,
   val -= min;
   lto_output_1_stream (obs, val & 255);
   if (range >= 0xff)
-    lto_output_1_stream (obs, (val << 8) & 255);
+    lto_output_1_stream (obs, (val >> 8) & 255);
   if (range >= 0xffff)
-    lto_output_1_stream (obs, (val << 16) & 255);
+    lto_output_1_stream (obs, (val >> 16) & 255);
   if (range >= 0xffffff)
-    lto_output_1_stream (obs, (val << 24) & 255);
+    lto_output_1_stream (obs, (val >> 24) & 255);
 }
 
 /* Input VAL into OBS and verify it is in range MIN...MAX that is supposed
diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
index 9d4e2ed..657d667 100644
--- a/gcc/lto/lto.c
+++ b/gcc/lto/lto.c
@@ -2615,8 +2615,8 @@ void
 lto_main (void)
 {
   lto_process_name ();
-
-  lto_init_reader ();
+  gimple_streamer_hooks_init ();
+  lto_reader_init ();
 
   /* Read all the symbols and call graph from all the files in the
      command line.  */

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

[rguenther_suse.de, 2011-06-01 12:07:45]

On Tue, 31 May 2011, Diego Novillo wrote:

> 
> This patch merges the LTO streamer hooks from the pph branch.
> 
> These hooks are meant to separate streaming work that is specific to
> GIMPLE from other modules that may want to stream their own data
> structures.  In the PPH branch, we are using the streamer to save
> front end data structures, so there was a bunch of work and checks
> that made no sense in the front end.
> 
> All this module-specific work can be moved to the hooks defined in
> struct lto_streamer_hooks.
> 
> One of the concerns I had with the hooks is potential overhead when
> streaming because of the new indirect calls added by the patch.  This
> does not seem to be the case.
> 
> I did several timing runs using insn-emit.o (the biggest object file
> we generate in a bootstrap) and using an LTO link of cc1.  The times
> are slightly in favour of the hooks, actually (the patch removes some
> checks from the core tree pickling routines), but the difference is
> insignificant.
> 
> Over 5 runs on an idle system: Without this patch, we compile
> insn-emit.o in 41.56 secs (wall time), with the patch the time is
> 41.34 secs (wall time).
> 
> The other change in the patch is to reserve enough tags in enum
> LTO_tags to cover all the possible tree codes.  This exposes the bug
> that I fixed yesterday in the pph branch (the sign of the shifts in
> lto_output_int_in_range).  This means that we need to use
> output_record_start (LTO_null) instead of output_zero to output
> 0-delimiters, which increases the size of the output by less than 1%
> on average.

Can you split this fix out from this merge?

> Tested with LTO profiledbootstrap on x86_64.  OK for trunk?

Comments inline

> 
> Diego.
> 
> 
> 	* Makefile.in (cgraphunit.o): Add dependency on LTO_STREAMER_H.
> 	* cgraphunit.c: Include lto-streamer.h
> 	(cgraph_finalize_compilation_unit): Call gimple_streamer_hooks_init
> 	if LTO is enabled.
> 	* lto-streamer-in.c (unpack_value_fields): Do not check
> 	for TS_SSA_NAME, TS_STATEMENT_LIST and TS_OMP_CLAUSE.
> 	Call lto_streamer_hooks.unpack_value_fields if set.
> 	(lto_materialize_tree): For unhandled nodes, first try to
> 	call lto_streamer_hooks.alloc_tree, if it exists.
> 	(lto_input_ts_decl_common_tree_pointers): Move reading of
> 	DECL_INITIAL to gimple_streamer_read_tree.
> 	(lto_input_tree_pointers): Do not handle TS_SSA_NAME,
> 	TS_STATEMENT_LIST, TS_OMP_CLAUSE and TS_OPTIMIZATION.
> 	(lto_read_tree): Call lto_streamer_hooks.read_tree if
> 	set.
> 	Only register symbols in symtab if
> 	lto_streamer_hooks.register_decls_in_symtab_p is set.
> 	(gimple_streamer_read_tree): New.
> 	(lto_reader_init): Rename from lto_init_reader.
> 	Move initialization code to gimple_streamer_reader_init.
> 	Call lto_streamer_hooks.reader_init if set.
> 	(gimple_streamer_reader_init): New.
> 	* lto-streamer-out.c (pack_value_fields): Do not handle
> 	TS_SSA_NAME, TS_STATEMENT_LIST and TS_OMP_CLAUSE.
> 	Call lto_streamer_hooks.pack_value_fields if set.
> 	(lto_output_tree_ref): For tree nodes that are not
> 	normally indexable, call
> 	lto_streamer_hooks.indexable_with_decls_p before giving
> 	up.
> 	(lto_output_ts_decl_common_tree_pointers): Move handling
> 	for FUNCTION_DECL and TRANSLATION_UNIT_DECL to
> 	gimple_streamer_write_tree.
> 	Move assertion for NULL DECL_SAVED_TREEs to
> 	gimple_streamer_write_tree.
> 	(lto_output_ts_decl_with_vis_tree_pointers): Call
> 	output_record_start with LTO_null instead of output_zero.
> 	(lto_output_ts_binfo_tree_pointers): Likewise.
> 	(lto_output_tree): Likewise.
> 	(output_eh_try_list): Likewise.
> 	(output_eh_region): Likewise.
> 	(output_eh_lp): Likewise.
> 	(output_eh_regions): Likewise.
> 	(output_bb): Likewise.
> 	(output_function): Likewise.
> 	(output_unreferenced_globals): Likewise.
> 	(lto_output_tree_pointers): Do not handle TS_SSA_NAME,
> 	TS_STATEMENT_LIST, TS_OMP_CLAUSE and TS_OPTIMIZATION.
> 	(lto_output_tree_header): Call
> 	lto_streamer_hooks.is_streamable instead of
> 	lto_is_streamable.
> 	Call lto_streamer_hooks.output_tree_header if set.
> 	(lto_write_tree): Call lto_streamer_hooks.write_tree if
> 	set.
> 	(gimple_streamer_write_tree): New.
> 	(lto_output_tree): If
> 	lto_streamer_hooks.has_unique_integer_csts_p is set,
> 	lookup the constant in the streamer cache first.
> 	* lto-streamer.c (lto_streamer_cache_create): Call
> 	lto_streamer_hooks.preload_common_nodes instead of
> 	lto_preload_common_nodes.
> 	(lto_is_streamable): Move from lto-streamer.h
> 	(gimple_streamer_hooks_init): New.
> 	(streamer_hooks): New.
> 	(streamer_hooks_): Declare.
> 	(streamer_hooks_init): New.
> 	* lto-streamer.h (struct output_block): Forward declare.
> 	(struct lto_input_block): Likewise.
> 	(struct data_in): Likewise.
> 	(struct bitpack_d): Likewise.
> 	(enum LTO_tags): Reserve MAX_TREE_CODES
> 	to cover all front end codes as well.
> 	(struct lto_streamer_hooks): Declare.
> 	(gimple_streamer_hooks_init): Declare.
> 	(gimple_streamer_write_tree): Declare.
> 	(gimple_streamer_read_tree): Declare.
> 	(streamer_hooks): Declare.
> 	(streamer_hooks_init): Declare.
> 	(lto_tag_is_tree_code_p): Check max value against MAX_TREE_CODES.
> 	(lto_is_streamable): Move to lto-streamer.c
> 	(lto_output_int_in_range): Change << to >> when shifting VAL.
> 
> lto/ChangeLog
> 2011-05-31  Diego Novillo  <dnovillo@google.com>
> 
> 	* lto.c (lto_main): Call gimple_streamer_hooks_init.
> 
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index 834acb1..22fde67 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -140,6 +140,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "plugin.h"
>  #include "ipa-inline.h"
>  #include "ipa-utils.h"
> +#include "lto-streamer.h"
>  
>  static void cgraph_expand_all_functions (void);
>  static void cgraph_mark_functions_to_output (void);
> @@ -1092,6 +1093,10 @@ cgraph_finalize_compilation_unit (void)
>  {
>    timevar_push (TV_CGRAPH);
>  
> +  /* If LTO is enabled, initialize the streamer hooks needed by GIMPLE.  */
> +  if (flag_lto)
> +    gimple_streamer_hooks_init ();

Ugh.  Isn't there a better entry for this?  Are you going to add

  if (flag_pph)
    init_hooks_some_other_way ();

here?  It looks it rather belongs to opts.c or toplev.c if the hooks
are really initialized dependent on compiler flags.

>    /* If we're here there's no current function anymore.  Some frontends
>       are lazy in clearing these.  */
>    current_function_decl = NULL;
> diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
> index 88966f2..801fe6f 100644
> --- a/gcc/lto-streamer-in.c
> +++ b/gcc/lto-streamer-in.c
> @@ -1833,6 +1833,7 @@ static void
>  unpack_value_fields (struct bitpack_d *bp, tree expr)
>  {
>    enum tree_code code;
> +  lto_streamer_hooks *h = streamer_hooks ();

A function to access a global ... we have lang_hooks and targetm,
so please simply use streamer_hooks as a variable.
streamer_hooks ()->preload_common_nodes (cache) looks super-ugly.

>    code = TREE_CODE (expr);
>  
> @@ -1864,26 +1865,11 @@ unpack_value_fields (struct bitpack_d *bp, tree expr)
>    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
>      unpack_ts_block_value_fields (bp, expr);
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
> -    {
> -      /* We only stream the version number of SSA names.  */
> -      gcc_unreachable ();
> -    }
> -
> -  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
> -    {
> -      /* This is only used by GENERIC.  */
> -      gcc_unreachable ();
> -    }
> -
> -  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
> -    {
> -      /* This is only used by High GIMPLE.  */
> -      gcc_unreachable ();
> -    }
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
>      unpack_ts_translation_unit_decl_value_fields (bp, expr);
> +
> +  if (h->unpack_value_fields)
> +    h->unpack_value_fields (bp, expr);

I suppose the LTO implementation has a gcc_unreachable () for
the cases we do not handle here?

>  }
>  
>  
> @@ -1935,8 +1921,17 @@ lto_materialize_tree (struct lto_input_block *ib,
struct data_in *data_in,
>      }
>    else
>      {
> -      /* All other nodes can be materialized with a raw make_node call.  */
> -      result = make_node (code);
> +      lto_streamer_hooks *h = streamer_hooks ();
> +
> +      /* For all other nodes, see if the streamer knows how to allocate
> +	 it.  */
> +      if (h->alloc_tree)
> +	result = h->alloc_tree (code, ib, data_in);
> +
> +      /* If the hook did not handle it, materialize the tree with a raw
> +	 make_node call.  */
> +      if (result == NULL_TREE)
> +	result = make_node (code);
>      }
>  
>  #ifdef LTO_STREAMER_DEBUG
> @@ -2031,12 +2026,8 @@ lto_input_ts_decl_common_tree_pointers (struct
lto_input_block *ib,
>  {
>    DECL_SIZE (expr) = lto_input_tree (ib, data_in);
>    DECL_SIZE_UNIT (expr) = lto_input_tree (ib, data_in);
> -
> -  if (TREE_CODE (expr) != FUNCTION_DECL
> -      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
> -    DECL_INITIAL (expr) = lto_input_tree (ib, data_in);
> -

Why move those?  DECL_INITIAL _is_ in decl_common.

>    DECL_ATTRIBUTES (expr) = lto_input_tree (ib, data_in);
> +
>    /* Do not stream DECL_ABSTRACT_ORIGIN.  We cannot handle debug information
>       for early inlining so drop it on the floor instead of ICEing in
>       dwarf2out.c.  */
> @@ -2433,38 +2424,15 @@ lto_input_tree_pointers (struct lto_input_block *ib,
struct data_in *data_in,
>    if (CODE_CONTAINS_STRUCT (code, TS_EXP))
>      lto_input_ts_exp_tree_pointers (ib, data_in, expr);
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
> -    {
> -      /* We only stream the version number of SSA names.  */
> -      gcc_unreachable ();
> -    }
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
>      lto_input_ts_block_tree_pointers (ib, data_in, expr);
>  
>    if (CODE_CONTAINS_STRUCT (code, TS_BINFO))
>      lto_input_ts_binfo_tree_pointers (ib, data_in, expr);
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
> -    {
> -      /* This should only appear in GENERIC.  */
> -      gcc_unreachable ();
> -    }
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
>      lto_input_ts_constructor_tree_pointers (ib, data_in, expr);
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
> -    {
> -      /* This should only appear in High GIMPLE.  */
> -      gcc_unreachable ();
> -    }
> -
> -  if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
> -    {
> -      sorry ("optimization options not supported yet");
> -    }
> -

Where do those checks go?  Or do we simply lose them?

>    if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
>      lto_input_ts_target_option (ib, expr);
>  
> @@ -2655,20 +2623,29 @@ lto_read_tree (struct lto_input_block *ib, struct
data_in *data_in,
>  	       enum LTO_tags tag)
>  {
>    tree result;
> +  lto_streamer_hooks *h = streamer_hooks ();
>  
>    result = lto_materialize_tree (ib, data_in, tag);
>  
>    /* Read all the pointer fields in RESULT.  */
>    lto_input_tree_pointers (ib, data_in, result);
>  
> +  /* Call back into the streaming module to read anything else it
> +     may need.  */
> +  if (h->read_tree)
> +    h->read_tree (ib, data_in, result);
> +
>    /* We should never try to instantiate an MD or NORMAL builtin here.  */
>    if (TREE_CODE (result) == FUNCTION_DECL)
>      gcc_assert (!lto_stream_as_builtin_p (result));
>  
> -  if (TREE_CODE (result) == VAR_DECL)
> -    lto_register_var_decl_in_symtab (data_in, result);
> -  else if (TREE_CODE (result) == FUNCTION_DECL && !DECL_BUILT_IN (result))
> -    lto_register_function_decl_in_symtab (data_in, result);
> +  if (h->register_decls_in_symtab_p)
> +    {
> +      if (TREE_CODE (result) == VAR_DECL)
> +	lto_register_var_decl_in_symtab (data_in, result);
> +      else if (TREE_CODE (result) == FUNCTION_DECL && !DECL_BUILT_IN
(result))
> +	lto_register_function_decl_in_symtab (data_in, result);
> +    }

I would say we should defer symtab registering to uniquify_nodes time,
when we are sure we completely streamed in the tree we want to register
(then we don't have to deal with partially read trees in those functions).

Can you work on a preparatory patch for this please?  That would get
rid for the need of this hook and clean up things at the same time.

>  
>    /* end_marker = */ lto_input_1_unsigned (ib);
>  
> @@ -2682,6 +2659,22 @@ lto_read_tree (struct lto_input_block *ib, struct
data_in *data_in,
>  }
>  
>  
> +/* LTO streamer hook for reading GIMPLE trees.  IB and DATA_IN are as in
> +   lto_read_tree.  EXPR is the tree was materialized by lto_read_tree and
> +   needs GIMPLE specific data to be filled in.  */
> +
> +void
> +gimple_streamer_read_tree (struct lto_input_block *ib,
> +			   struct data_in *data_in,
> +			   tree expr)
> +{
> +  if (DECL_P (expr)
> +      && TREE_CODE (expr) != FUNCTION_DECL
> +      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
> +    DECL_INITIAL (expr) = lto_input_tree (ib, data_in);

What's wrong with doing this in the decl-common path?

> +}
> +
> +
>  /* Read and INTEGER_CST node from input block IB using the per-file
>     context in DATA_IN.  */
>  
> @@ -2763,16 +2756,23 @@ lto_input_tree (struct lto_input_block *ib, struct
data_in *data_in)
>  /* Initialization for the LTO reader.  */
>  
>  void
> -lto_init_reader (void)
> +lto_reader_init (void)
>  {
>    lto_streamer_init ();
> -
> -  memset (&lto_stats, 0, sizeof (lto_stats));
> -  bitmap_obstack_initialize (NULL);
> -
>    file_name_hash_table = htab_create (37, hash_string_slot_node,
>  				      eq_string_slot_node, free);
> +  if (streamer_hooks ()->reader_init)
> +    streamer_hooks ()->reader_init ();
> +}
> +
>  
> +/* GIMPLE streamer hook for initializing the LTO reader.  */
> +
> +void
> +gimple_streamer_reader_init (void)
> +{
> +  memset (&lto_stats, 0, sizeof (lto_stats));
> +  bitmap_obstack_initialize (NULL);
>    gimple_register_cfg_hooks ();
>  }
>  
> diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
> index b3b81bd..90e3252 100644
> --- a/gcc/lto-streamer-out.c
> +++ b/gcc/lto-streamer-out.c
> @@ -558,6 +558,7 @@ static void
>  pack_value_fields (struct bitpack_d *bp, tree expr)
>  {
>    enum tree_code code;
> +  lto_streamer_hooks *h = streamer_hooks ();
>  
>    code = TREE_CODE (expr);
>  
> @@ -589,26 +590,11 @@ pack_value_fields (struct bitpack_d *bp, tree expr)
>    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
>      pack_ts_block_value_fields (bp, expr);
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
> -    {
> -      /* We only stream the version number of SSA names.  */
> -      gcc_unreachable ();
> -    }
> -
> -  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
> -    {
> -      /* This is only used by GENERIC.  */
> -      gcc_unreachable ();
> -    }
> -
> -  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
> -    {
> -      /* This is only used by High GIMPLE.  */
> -      gcc_unreachable ();
> -    }
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
>      pack_ts_translation_unit_decl_value_fields (bp, expr);
> +
> +  if (h->pack_value_fields)
> +    h->pack_value_fields (bp, expr);
>  }
>  
>  
> @@ -772,9 +758,23 @@ lto_output_tree_ref (struct output_block *ob, tree expr)
>        break;
>  
>      default:
> -      /* No other node is indexable, so it should have been handled
> -	 by lto_output_tree.  */
> -      gcc_unreachable ();
> +      {
> +	lto_streamer_hooks *h = streamer_hooks ();
> +
> +	/* See if streamer hooks allows this node to be indexable with
> +	   VAR_DECLs.  */

with VAR_DECLs?  More like "similar to global decls."?

> +	if (h->indexable_with_decls_p && h->indexable_with_decls_p (expr))
> +	  {
> +	    output_record_start (ob, LTO_global_decl_ref);
> +	    lto_output_var_decl_index (ob->decl_state, ob->main_stream, expr);

Why hook it this way and not

             if (h->output_tree_ref
                 && h->output_tree_ref (...))
               break;
             gcc_unreachable (); 

I find the flag vs. function hook stuff somewhat odd.

> +	  }
> +	else
> +	  {
> +	    /* No other node is indexable, so it should have been
> +	      handled by lto_output_tree.  */
> +	    gcc_unreachable ();
> +	  }
> +      }
>      }
>  }
>  
> @@ -883,27 +883,10 @@ lto_output_ts_decl_common_tree_pointers (struct
output_block *ob, tree expr,
>    lto_output_tree_or_ref (ob, DECL_SIZE (expr), ref_p);
>    lto_output_tree_or_ref (ob, DECL_SIZE_UNIT (expr), ref_p);
>  
> -  if (TREE_CODE (expr) != FUNCTION_DECL
> -      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
> -    {
> -      tree initial = DECL_INITIAL (expr);
> -      if (TREE_CODE (expr) == VAR_DECL
> -	  && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
> -	  && initial)
> -	{
> -	  lto_varpool_encoder_t varpool_encoder =
ob->decl_state->varpool_node_encoder;
> -	  struct varpool_node *vnode = varpool_get_node (expr);
> -	  if (!vnode)
> -	    initial = error_mark_node;
> -	  else if (!lto_varpool_encoder_encode_initializer_p (varpool_encoder,
> -							      vnode))
> -	    initial = NULL;
> -	}
> -    
> -      lto_output_tree_or_ref (ob, initial, ref_p);
> -    }
> +  /* Do not stream DECL_INITIAL.  */

That's a gross comment ;)  We _do_ stream it, but now in a hook.
I suppose all the streamer functions should lose their lto_ prefix -
reading them with such comments will get confusing ...

>  
>    lto_output_tree_or_ref (ob, DECL_ATTRIBUTES (expr), ref_p);
> +
>    /* Do not stream DECL_ABSTRACT_ORIGIN.  We cannot handle debug information
>       for early inlining so drop it on the floor instead of ICEing in
>       dwarf2out.c.  */
> @@ -931,11 +914,6 @@ lto_output_ts_decl_non_common_tree_pointers (struct
output_block *ob,
>  {
>    if (TREE_CODE (expr) == FUNCTION_DECL)
>      {
> -      /* DECL_SAVED_TREE holds the GENERIC representation for DECL.
> -	 At this point, it should not exist.  Either because it was
> -	 converted to gimple or because DECL didn't have a GENERIC
> -	 representation in this TU.  */
> -      gcc_assert (DECL_SAVED_TREE (expr) == NULL_TREE);
>        lto_output_tree_or_ref (ob, DECL_ARGUMENTS (expr), ref_p);
>        lto_output_tree_or_ref (ob, DECL_RESULT (expr), ref_p);
>      }
> @@ -955,7 +933,7 @@ lto_output_ts_decl_with_vis_tree_pointers (struct
output_block *ob, tree expr,
>    if (DECL_ASSEMBLER_NAME_SET_P (expr))
>      lto_output_tree_or_ref (ob, DECL_ASSEMBLER_NAME (expr), ref_p);
>    else
> -    output_zero (ob);
> +    output_record_start (ob, LTO_null);
>  
>    lto_output_tree_or_ref (ob, DECL_SECTION_NAME (expr), ref_p);
>    lto_output_tree_or_ref (ob, DECL_COMDAT_GROUP (expr), ref_p);
> @@ -1136,7 +1114,7 @@ lto_output_ts_binfo_tree_pointers (struct output_block
*ob, tree expr,
>       is needed to build the empty BINFO node on the reader side.  */
>    FOR_EACH_VEC_ELT (tree, BINFO_BASE_BINFOS (expr), i, t)
>      lto_output_tree_or_ref (ob, t, ref_p);
> -  output_zero (ob);
> +  output_record_start (ob, LTO_null);
>  
>    lto_output_tree_or_ref (ob, BINFO_OFFSET (expr), ref_p);
>    lto_output_tree_or_ref (ob, BINFO_VTABLE (expr), ref_p);
> @@ -1256,12 +1234,6 @@ lto_output_tree_pointers (struct output_block *ob, tree
expr, bool ref_p)
>    if (CODE_CONTAINS_STRUCT (code, TS_EXP))
>      lto_output_ts_exp_tree_pointers (ob, expr, ref_p);
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
> -    {
> -      /* We only stream the version number of SSA names.  */
> -      gcc_unreachable ();
> -    }
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
>      lto_output_ts_block_tree_pointers (ob, expr, ref_p);
>  
> @@ -1271,21 +1243,6 @@ lto_output_tree_pointers (struct output_block *ob, tree
expr, bool ref_p)
>    if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
>      lto_output_ts_constructor_tree_pointers (ob, expr, ref_p);
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
> -    {
> -      /* This should only appear in GENERIC.  */
> -      gcc_unreachable ();
> -    }
> -
> -  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
> -    {
> -      /* This should only appear in High GIMPLE.  */
> -      gcc_unreachable ();
> -    }
> -
> -  if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
> -    sorry ("gimple bytecode streams do not support the optimization
attribute");
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
>      lto_output_ts_target_option (ob, expr);
>  
> @@ -1304,18 +1261,20 @@ lto_output_tree_header (struct output_block *ob, tree
expr)
>  {
>    enum LTO_tags tag;
>    enum tree_code code;
> +  lto_streamer_hooks *h = streamer_hooks ();
>  
>    /* We should not see any non-GIMPLE tree nodes here.  */
>    code = TREE_CODE (expr);
> -  if (!lto_is_streamable (expr))
> -    internal_error ("tree code %qs is not supported in gimple streams",
> -		    tree_code_name[code]);
> +  if (h->is_streamable && !h->is_streamable (expr))
> +    internal_error ("tree code %qs is not supported in %s streams",
> +		    h->name, tree_code_name[code]);

I'd say this hook should always exist.

>    /* The header of a tree node consists of its tag, the size of
>       the node, and any other information needed to instantiate
>       EXPR on the reading side (such as the number of slots in
>       variable sized nodes).  */
>    tag = lto_tree_code_to_tag (code);
> +  gcc_assert ((unsigned) tag < (unsigned) LTO_NUM_TAGS);

Doesn't Honzas streaming this as enum already assert this?

>    output_record_start (ob, tag);
>  
>    /* The following will cause bootstrap miscomparisons.  Enable with care. 
*/
> @@ -1338,6 +1297,11 @@ lto_output_tree_header (struct output_block *ob, tree
expr)
>      output_sleb128 (ob, TREE_VEC_LENGTH (expr));
>    else if (CODE_CONTAINS_STRUCT (code, TS_BINFO))
>      output_uleb128 (ob, BINFO_N_BASE_BINFOS (expr));
> +
> +  /* Allow the streamer to write any streamer-specific information
> +     needed to instantiate the node when reading.  */
> +  if (h->output_tree_header)
> +    h->output_tree_header (ob, expr);
>  }
>  
>  
> @@ -1385,6 +1349,7 @@ static void
>  lto_write_tree (struct output_block *ob, tree expr, bool ref_p)
>  {
>    struct bitpack_d bp;
> +  lto_streamer_hooks *h = streamer_hooks ();
>  
>    /* Write the header, containing everything needed to materialize
>       EXPR on the reading side.  */
> @@ -1399,11 +1364,58 @@ lto_write_tree (struct output_block *ob, tree expr,
bool ref_p)
>    /* Write all the pointer fields in EXPR.  */
>    lto_output_tree_pointers (ob, expr, ref_p);
>  
> +  /* Call back into the streaming module to see if it needs to write
> +     anything that was not written by the common streamer.  */
> +  if (h->write_tree)
> +    h->write_tree (ob, expr, ref_p);
> +
>    /* Mark the end of EXPR.  */
>    output_zero (ob);
>  }
>  
>  
> +/* GIMPLE hook for writing GIMPLE-specific parts of trees.  OB, EXPR
> +   and REF_P are as in lto_write_tree.  */
> +
> +void
> +gimple_streamer_write_tree (struct output_block *ob, tree expr, bool ref_p)
> +{
> +  if (TREE_CODE (expr) == FUNCTION_DECL)
> +    {
> +      /* DECL_SAVED_TREE holds the GENERIC representation for DECL.
> +	 At this point, it should not exist.  Either because it was
> +	 converted to gimple or because DECL didn't have a GENERIC
> +	 representation in this TU.  */
> +      gcc_assert (DECL_SAVED_TREE (expr) == NULL_TREE);
> +    }

I think we can drop this check.

> +  if (DECL_P (expr)
> +      && TREE_CODE (expr) != FUNCTION_DECL
> +      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
> +    {
> +      /* Handle DECL_INITIAL for symbols.  */
> +      tree initial = DECL_INITIAL (expr);
> +      if (TREE_CODE (expr) == VAR_DECL
> +	  && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
> +	  && initial)
> +	{
> +	  lto_varpool_encoder_t varpool_encoder;
> +	  struct varpool_node *vnode;
> +
> +	  varpool_encoder = ob->decl_state->varpool_node_encoder;
> +	  vnode = varpool_get_node (expr);
> +	  if (!vnode)
> +	    initial = error_mark_node;
> +	  else if (!lto_varpool_encoder_encode_initializer_p (varpool_encoder,
> +							      vnode))
> +	    initial = NULL;
> +	}
> +
> +      lto_output_tree_or_ref (ob, initial, ref_p);
> +    }
> +}
> +
> +
>  /* Emit the integer constant CST to output block OB.  If REF_P is true,
>     CST's type will be emitted as a reference.  */
>  
> @@ -1430,7 +1442,7 @@ lto_output_tree (struct output_block *ob, tree expr,
bool ref_p)
>  
>    if (expr == NULL_TREE)
>      {
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>        return;
>      }
>  
> @@ -1438,8 +1450,27 @@ lto_output_tree (struct output_block *ob, tree expr,
bool ref_p)
>       to be materialized by the reader (to implement TYPE_CACHED_VALUES).  */
>    if (TREE_CODE (expr) == INTEGER_CST)
>      {
> -      lto_output_integer_cst (ob, expr, ref_p);
> -      return;
> +      bool is_special;
> +
> +     /* There are some constants that are special to the streamer
> +	(e.g., void_zero_node, truthvalue_false_node).
> +	These constants cannot be rematerialized with
> +	build_int_cst_wide because they may actually lack a type (like
> +	void_zero_node) and they need to be pointer-identical to trees
> +	materialized by the compiler tables like global_trees or
> +	c_global_trees.
> +
> +	If the streamer told us that it has special constants, they
> +	will be preloaded in the streamer cache.  If we find a match,
> +	then stream the constant as a reference so the reader can
> +	re-materialize it from the cache.  */
> +      is_special = streamer_hooks ()->has_unique_integer_csts_p
> +		   && lto_streamer_cache_lookup (ob->writer_cache, expr, NULL);
> +      if (!is_special)
> +	{
> +	  lto_output_integer_cst (ob, expr, ref_p);
> +	  return;
> +	}

???  We should not arrive here for such global trees.  Please do not
merge this part of the patch as part of the hook introducing (keep
patches simple, make them do a single thing ...)

>      }
>  
>    existed_p = lto_streamer_cache_insert (ob->writer_cache, expr, &ix);
> @@ -1486,7 +1517,7 @@ output_eh_try_list (struct output_block *ob, eh_catch
first)
>        lto_output_tree_ref (ob, n->label);
>      }
>  
> -  output_zero (ob);
> +  output_record_start (ob, LTO_null);
>  }
>  
>  
> @@ -1501,7 +1532,7 @@ output_eh_region (struct output_block *ob, eh_region r)
>  
>    if (r == NULL)
>      {
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>        return;
>      }
>  
> @@ -1564,7 +1595,7 @@ output_eh_lp (struct output_block *ob, eh_landing_pad
lp)
>  {
>    if (lp == NULL)
>      {
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>        return;
>      }
>  
> @@ -1633,9 +1664,9 @@ output_eh_regions (struct output_block *ob, struct
function *fn)
>  	}
>      }
>  
> -  /* The 0 either terminates the record or indicates that there are no
> -     eh_records at all.  */
> -  output_zero (ob);
> +  /* The LTO_null either terminates the record or indicates that there
> +     are no eh_records at all.  */
> +  output_record_start (ob, LTO_null);
>  }
>  
>  
> @@ -1880,10 +1911,10 @@ output_bb (struct output_block *ob, basic_block bb,
struct function *fn)
>  	      output_sleb128 (ob, region);
>  	    }
>  	  else
> -	    output_zero (ob);
> +	    output_record_start (ob, LTO_null);
>  	}
>  
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>  
>        for (bsi = gsi_start_phis (bb); !gsi_end_p (bsi); gsi_next (&bsi))
>  	{
> @@ -1896,7 +1927,7 @@ output_bb (struct output_block *ob, basic_block bb,
struct function *fn)
>  	    output_phi (ob, phi);
>  	}
>  
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>      }
>  }
>  
> @@ -2053,7 +2084,7 @@ output_function (struct cgraph_node *node)
>      output_bb (ob, bb, fn);
>  
>    /* The terminator for this function.  */
> -  output_zero (ob);
> +  output_record_start (ob, LTO_null);
>  
>    output_cfg (ob, fn);
>  
> @@ -2167,7 +2198,7 @@ output_unreferenced_globals (cgraph_node_set set,
varpool_node_set vset)
>        }
>    symbol_alias_set_destroy (defined);
>  
> -  output_zero (ob);
> +  output_record_start (ob, LTO_null);
>  
>    produce_asm (ob, NULL);
>    destroy_output_block (ob);
> @@ -2238,6 +2269,8 @@ static void
>  lto_writer_init (void)
>  {
>    lto_streamer_init ();
> +  if (streamer_hooks ()->writer_init)
> +    streamer_hooks ()->writer_init ();

This hook should always exist.  Why is this called in a context with
lto_*?

>  }
>  
>  
> diff --git a/gcc/lto-streamer.c b/gcc/lto-streamer.c
> index abccd10..4281bd9 100644
> --- a/gcc/lto-streamer.c
> +++ b/gcc/lto-streamer.c
> @@ -579,7 +579,7 @@ lto_streamer_cache_create (void)
>    /* Load all the well-known tree nodes that are always created by
>       the compiler on startup.  This prevents writing them out
>       unnecessarily.  */
> -  lto_preload_common_nodes (cache);
> +  streamer_hooks ()->preload_common_nodes (cache);
>  
>    return cache;
>  }
> @@ -724,3 +724,70 @@ lto_check_version (int major, int minor)
>  		 major, minor,
>  		 LTO_major_version, LTO_minor_version);
>  }
> +
> +
> +/* Return true if EXPR is a tree node that can be written to disk.  */
> +
> +static inline bool
> +lto_is_streamable (tree expr)
> +{
> +  enum tree_code code = TREE_CODE (expr);
> +
> +  /* Notice that we reject SSA_NAMEs as well.  We only emit the SSA
> +     name version in lto_output_tree_ref (see output_ssa_names).  */
> +  return !is_lang_specific (expr)
> +	 && code != SSA_NAME
> +	 && code != CALL_EXPR
> +	 && code != LANG_TYPE
> +	 && code != MODIFY_EXPR
> +	 && code != INIT_EXPR
> +	 && code != TARGET_EXPR
> +	 && code != BIND_EXPR
> +	 && code != WITH_CLEANUP_EXPR
> +	 && code != STATEMENT_LIST
> +	 && code != OMP_CLAUSE
> +	 && code != OPTIMIZATION_NODE
> +	 && (code == CASE_LABEL_EXPR
> +	     || code == DECL_EXPR
> +	     || TREE_CODE_CLASS (code) != tcc_statement);
> +}

This change (with the removal of the cases with the unreachable()s)
could be made separately (directly calling it instead of hooking it).

> +
> +/* Initialize all the streamer hooks used for streaming GIMPLE.  */
> +
> +void
> +gimple_streamer_hooks_init (void)

It's more like lto_streamer_hooks_init, no?  You are basically
turning lto_streamer_* to tree_streamer_* and make lto_streamer_*
tree_streamer_* + lto_streamer_hooks, no?  It would be really
nice to also reflect this in the function naming (and eventually
moving the tree_streamer_* stuff to separate files).  It really
confuses me ;)

> +{
> +  lto_streamer_hooks *h = streamer_hooks_init ();

Heh, and the type is called lto_streamer_hooks ...

> +  h->name = "gimple";
> +  h->reader_init = gimple_streamer_reader_init;
> +  h->writer_init = NULL;
> +  h->preload_common_nodes = lto_preload_common_nodes;
> +  h->is_streamable = lto_is_streamable;
> +  h->write_tree = gimple_streamer_write_tree;
> +  h->read_tree = gimple_streamer_read_tree;
> +  h->register_decls_in_symtab_p = true;
> +}
> +
> +
> +/* Return the current set of streamer hooks.  Note that only one set
> +   of streamer hooks can be active at a time.  */
> +
> +static lto_streamer_hooks streamer_hooks_;
> +
> +lto_streamer_hooks *
> +streamer_hooks (void)
> +{
> +  return &streamer_hooks_;
> +}
> +
> +
> +/* Initialize and return the current set of streamer hooks.  */
> +
> +lto_streamer_hooks *
> +streamer_hooks_init (void)
> +{
> +  lto_streamer_hooks *h = streamer_hooks ();
> +  memset (h, 0, sizeof (lto_streamer_hooks));
> +  return h;
> +}
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index e8410d4..035aa5e 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -33,6 +33,12 @@ along with GCC; see the file COPYING3.  If not see
>  #include "alloc-pool.h"
>  #include "gcov-io.h"
>  
> +/* Forward declarations to avoid including unnecessary headers.  */
> +struct output_block;
> +struct lto_input_block;
> +struct data_in;
> +struct bitpack_d;
> +
>  /* Define when debugging the LTO streamer.  This causes the writer
>     to output the numeric value for the memory address of the tree node
>     being emitted.  When debugging a problem in the reader, check the
> @@ -186,7 +192,7 @@ enum LTO_tags
>  
>       Conversely, to map between LTO tags and tree/gimple codes, the
>       reverse operation must be applied.  */
> -  LTO_bb0 = 1 + NUM_TREE_CODES + LAST_AND_UNUSED_GIMPLE_CODE,
> +  LTO_bb0 = 1 + MAX_TREE_CODES + LAST_AND_UNUSED_GIMPLE_CODE,
>    LTO_bb1,

This (and related) change can be merged separately.

>    /* EH region holding the previous statement.  */
> @@ -741,6 +747,98 @@ struct data_in
>  };
>  
>  
> +/* Streamer hooks.  These functions do additional processing as
> +   needed by the module.  There are two types of callbacks, those that
> +   replace the default behavior and those that supplement it.  Any
> +   or all of these hooks can be NULL.  */
> +typedef struct lto_streamer_hooks {
> +  /* A string identifying this streamer.  */
> +  const char *name;
> +
> +  /* Called by lto_streamer_cache_create to instantiate a cache of
> +     well-known nodes.  These are tree nodes that are always
> +     instantiated by the compiler on startup.  Additionally, these
> +     nodes need to be shared.  This function should call
> +     lto_streamer_cache_append on every tree node that it wishes to
> +     preload in the streamer cache.  This way, the writer will only
> +     write out a reference to the tree and the reader will instantiate
> +     the tree out of this pre-populated cache.  */
> +  void (*preload_common_nodes) (struct lto_streamer_cache_d *);
> +
> +  /* Called by lto_reader_init after it does basic initialization.  */
> +  void (*reader_init) (void);
> +
> +  /* Called by lto_writer_init after it does basic initalization.  */
> +  void (*writer_init) (void);
> +
> +  /* Return true if the given tree is supported by this streamer.  */
> +  bool (*is_streamable) (tree);
> +
> +  /* Called by lto_write_tree after writing all the common parts of
> +     a tree.  If defined, the callback is in charge of writing all
> +     the fields that lto_write_tree did not write out.  Arguments
> +     are as in lto_write_tree.
> +
> +     The following tree fields are not handled by common code:
> +
> +	DECL_ABSTRACT_ORIGIN
> +	DECL_INITIAL
> +	DECL_SAVED_TREE
> +
> +     Callbacks may choose to ignore or handle them.  If handled,
> +     the reader should read them in the exact same sequence written
> +     by the writer.  */
> +  void (*write_tree) (struct output_block *, tree, bool);
> +
> +  /* Called by lto_read_tree after reading all the common parts of
> +     a tree.  If defined, the callback is in charge of reading all
> +     the fields that lto_read_tree did not read in.  Arguments
> +     are as in lto_read_tree.  */
> +  void (*read_tree) (struct lto_input_block *, struct data_in *, tree);
> +
> +  /* Called by lto_output_tree_ref to determine if the given tree node
> +     should be emitted as a reference to the table of declarations
> +     (the same table that holds VAR_DECLs).  */
> +  bool (*indexable_with_decls_p) (tree);
> +
> +  /* Called by pack_value_fields to store any non-pointer fields
> +     in the tree structure.  The arguments are as in pack_value_fields.  */
> +  void (*pack_value_fields) (struct bitpack_d *, tree);
> +
> +  /* Called by unpack_value_fields to retrieve any non-pointer fields
> +     in the tree structure.  The arguments are as in unpack_value_fields.  */
> +  void (*unpack_value_fields) (struct bitpack_d *, tree);
> +
> +  /* Non-zero if the streamer should register decls in the LTO
> +     global symbol tables.  */
> +  unsigned register_decls_in_symtab_p : 1;
> +
> +  /* Non-zero if the streamer has special constants that cannot be
> +     shared and are used in pointer-equality tests (e.g., void_zero_node,
> +     truthvalue_false_node, etc).  These constants will be present in
> +     the streamer cache and should be streamed as references.  */
> +  unsigned has_unique_integer_csts_p : 1;
> +
> +  /* Called by lto_materialize_tree for tree nodes that it does not
> +     know how to allocate memory for.  If defined, this hook should
> +     return a new tree node of the given code.  The data_in and
> +     input_block arguments are passed in case the hook needs to
> +     read more data from the stream to allocate the node.
> +     If this hook returns NULL, then lto_materialize_tree will attempt
> +     to allocate the tree by calling make_node directly.  */
> +  tree (*alloc_tree) (enum tree_code, struct lto_input_block *,
> +                      struct data_in *);
> +
> +  /* Called by lto_output_tree_header to write any streamer-specific
> +     information needed to allocate the tree.  This hook may assume
> +     that the basic header data (tree code, etc) has already been
> +     written.  It should only write any extra data needed to allocate
> +     the node (e.g., in the case of CALL_EXPR, this hook would write
> +     the number of arguments to the CALL_EXPR).  */
> +  void (*output_tree_header) (struct output_block *, tree);
> +} lto_streamer_hooks;

Rename to tree_streamer_hooks, start a tree-streamer.h file.

> +
>  /* In lto-section-in.c  */
>  extern struct lto_input_block * lto_create_simple_input_block (
>  			       struct lto_file_decl_data *,
> @@ -851,17 +949,21 @@ extern intptr_t lto_orig_address_get (tree);
>  extern void lto_orig_address_remove (tree);
>  #endif
>  extern void lto_check_version (int, int);
> -
> +extern void gimple_streamer_hooks_init (void);
> +extern void gimple_streamer_write_tree (struct output_block *, tree, bool);
> +extern void gimple_streamer_read_tree (struct lto_input_block *,
> +				       struct data_in *, tree);
> +extern lto_streamer_hooks *streamer_hooks (void);
> +extern lto_streamer_hooks *streamer_hooks_init (void);
>  
>  /* In lto-streamer-in.c */
>  extern void lto_input_cgraph (struct lto_file_decl_data *, const char *);
> -extern void lto_init_reader (void);
> +extern void lto_reader_init (void);
>  extern tree lto_input_tree (struct lto_input_block *, struct data_in *);
>  extern void lto_input_function_body (struct lto_file_decl_data *, tree,
>  				     const char *);
>  extern void lto_input_constructors_and_inits (struct lto_file_decl_data *,
>  					      const char *);
> -extern void lto_init_reader (void);
>  extern struct data_in *lto_data_in_create (struct lto_file_decl_data *,
>  				    const char *, unsigned,
>  				    VEC(ld_plugin_symbol_resolution_t,heap) *);
> @@ -869,6 +971,7 @@ extern void lto_data_in_delete (struct data_in *);
>  extern const char *lto_input_string (struct data_in *,
>  				     struct lto_input_block *);
>  extern void lto_input_data_block (struct lto_input_block *, void *, size_t);
> +extern void gimple_streamer_reader_init (void);
>  
>  
>  /* In lto-streamer-out.c  */
> @@ -957,7 +1060,7 @@ extern VEC(lto_out_decl_state_ptr, heap)
*lto_function_decl_states;
>  static inline bool
>  lto_tag_is_tree_code_p (enum LTO_tags tag)
>  {
> -  return tag > LTO_null && (unsigned) tag <= NUM_TREE_CODES;
> +  return tag > LTO_null && (unsigned) tag <= MAX_TREE_CODES;
>  }
>  
>  
> @@ -1063,29 +1166,6 @@ lto_stream_as_builtin_p (tree expr)
>  	      || DECL_BUILT_IN_CLASS (expr) == BUILT_IN_MD));
>  }
>  
> -/* Return true if EXPR is a tree node that can be written to disk.  */
> -static inline bool
> -lto_is_streamable (tree expr)
> -{
> -  enum tree_code code = TREE_CODE (expr);
> -
> -  /* Notice that we reject SSA_NAMEs as well.  We only emit the SSA
> -     name version in lto_output_tree_ref (see output_ssa_names).  */
> -  return !is_lang_specific (expr)
> -	 && code != SSA_NAME
> -	 && code != CALL_EXPR
> -	 && code != LANG_TYPE
> -	 && code != MODIFY_EXPR
> -	 && code != INIT_EXPR
> -	 && code != TARGET_EXPR
> -	 && code != BIND_EXPR
> -	 && code != WITH_CLEANUP_EXPR
> -	 && code != STATEMENT_LIST
> -	 && (code == CASE_LABEL_EXPR
> -	     || code == DECL_EXPR
> -	     || TREE_CODE_CLASS (code) != tcc_statement);
> -}
> -
>  DEFINE_DECL_STREAM_FUNCS (TYPE, type)
>  DEFINE_DECL_STREAM_FUNCS (FIELD_DECL, field_decl)
>  DEFINE_DECL_STREAM_FUNCS (FN_DECL, fn_decl)
> @@ -1226,11 +1306,11 @@ lto_output_int_in_range (struct lto_output_stream
*obs,
>    val -= min;
>    lto_output_1_stream (obs, val & 255);
>    if (range >= 0xff)
> -    lto_output_1_stream (obs, (val << 8) & 255);
> +    lto_output_1_stream (obs, (val >> 8) & 255);
>    if (range >= 0xffff)
> -    lto_output_1_stream (obs, (val << 16) & 255);
> +    lto_output_1_stream (obs, (val >> 16) & 255);
>    if (range >= 0xffffff)
> -    lto_output_1_stream (obs, (val << 24) & 255);
> +    lto_output_1_stream (obs, (val >> 24) & 255);
>  }
>  
>  /* Input VAL into OBS and verify it is in range MIN...MAX that is supposed
> diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
> index 9d4e2ed..657d667 100644
> --- a/gcc/lto/lto.c
> +++ b/gcc/lto/lto.c
> @@ -2615,8 +2615,8 @@ void
>  lto_main (void)
>  {
>    lto_process_name ();
> -
> -  lto_init_reader ();
> +  gimple_streamer_hooks_init ();
> +  lto_reader_init ();

Thanks,
Richard.

[Diego Novillo, 2011-06-01 16:03:04]

On Wed, Jun 1, 2011 at 08:07, Richard Guenther <rguenther@suse.de> wrote:

>>  static void cgraph_expand_all_functions (void);
>>  static void cgraph_mark_functions_to_output (void);
>> @@ -1092,6 +1093,10 @@ cgraph_finalize_compilation_unit (void)
>>  {
>>    timevar_push (TV_CGRAPH);
>>
>> +  /* If LTO is enabled, initialize the streamer hooks needed by GIMPLE.  */
>> +  if (flag_lto)
>> +    gimple_streamer_hooks_init ();
>
> Ugh.  Isn't there a better entry for this?  Are you going to add
>
>  if (flag_pph)
>    init_hooks_some_other_way ();
>
> here?  It looks it rather belongs to opts.c or toplev.c if the hooks
> are really initialized dependent on compiler flags.

Not at all, this is for gimple, specifically.  The front end
initializes hooks in its own way.  The problem here is that the gimple
hooks are needed by the middle end.  If we initialize gimple hooks too
early, the FE will override them.  So we need to initialize them after
the front end is done (hence the location for this call).

I'm happy to move this somewhere else, but it needs to happen right
before the middle end starts calling LTO pickling routines.

>
>>    /* If we're here there's no current function anymore.  Some frontends
>>       are lazy in clearing these.  */
>>    current_function_decl = NULL;
>> diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
>> index 88966f2..801fe6f 100644
>> --- a/gcc/lto-streamer-in.c
>> +++ b/gcc/lto-streamer-in.c
>> @@ -1833,6 +1833,7 @@ static void
>>  unpack_value_fields (struct bitpack_d *bp, tree expr)
>>  {
>>    enum tree_code code;
>> +  lto_streamer_hooks *h = streamer_hooks ();
>
> A function to access a global ... we have lang_hooks and targetm,
> so please simply use streamer_hooks as a variable.
> streamer_hooks ()->preload_common_nodes (cache) looks super-ugly.

I did not want to add yet another global.  I don't feel too strong
about this one, given the presence of lang_hooks and targetm.  So, you
prefer the direct global access?

>> @@ -1864,26 +1865,11 @@ unpack_value_fields (struct bitpack_d *bp, tree expr)
>>    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
>>      unpack_ts_block_value_fields (bp, expr);
>>
>> -  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
>> -    {
>> -      /* We only stream the version number of SSA names.  */
>> -      gcc_unreachable ();
>> -    }
>> -
>> -  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
>> -    {
>> -      /* This is only used by GENERIC.  */
>> -      gcc_unreachable ();
>> -    }
>> -
>> -  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
>> -    {
>> -      /* This is only used by High GIMPLE.  */
>> -      gcc_unreachable ();
>> -    }
>> -
>>    if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
>>      unpack_ts_translation_unit_decl_value_fields (bp, expr);
>> +
>> +  if (h->unpack_value_fields)
>> +    h->unpack_value_fields (bp, expr);
>
> I suppose the LTO implementation has a gcc_unreachable () for
> the cases we do not handle here?

Right.  This was already superfluous.  It's tested already by
lto_is_streamable().

>
>>  }
>>
>>
>> @@ -1935,8 +1921,17 @@ lto_materialize_tree (struct lto_input_block *ib,
struct data_in *data_in,
>>      }
>>    else
>>      {
>> -      /* All other nodes can be materialized with a raw make_node call.  */
>> -      result = make_node (code);
>> +      lto_streamer_hooks *h = streamer_hooks ();
>> +
>> +      /* For all other nodes, see if the streamer knows how to allocate
>> +      it.  */
>> +      if (h->alloc_tree)
>> +     result = h->alloc_tree (code, ib, data_in);
>> +
>> +      /* If the hook did not handle it, materialize the tree with a raw
>> +      make_node call.  */
>> +      if (result == NULL_TREE)
>> +     result = make_node (code);
>>      }
>>
>>  #ifdef LTO_STREAMER_DEBUG
>> @@ -2031,12 +2026,8 @@ lto_input_ts_decl_common_tree_pointers (struct
lto_input_block *ib,
>>  {
>>    DECL_SIZE (expr) = lto_input_tree (ib, data_in);
>>    DECL_SIZE_UNIT (expr) = lto_input_tree (ib, data_in);
>> -
>> -  if (TREE_CODE (expr) != FUNCTION_DECL
>> -      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
>> -    DECL_INITIAL (expr) = lto_input_tree (ib, data_in);
>> -
>
> Why move those?  DECL_INITIAL _is_ in decl_common.

I needed to move the handling of DECL_INITIAL in the writer.  This
forces us to move the handling in the reader.  Otherwise, reader and
writer will be out of sync (DECL_INITIAL is now written last).

> Where do those checks go?  Or do we simply lose them?

They already are in lto_is_streamable.  See above.

>> -  if (TREE_CODE (result) == VAR_DECL)
>> -    lto_register_var_decl_in_symtab (data_in, result);
>> -  else if (TREE_CODE (result) == FUNCTION_DECL && !DECL_BUILT_IN (result))
>> -    lto_register_function_decl_in_symtab (data_in, result);
>> +  if (h->register_decls_in_symtab_p)
>> +    {
>> +      if (TREE_CODE (result) == VAR_DECL)
>> +     lto_register_var_decl_in_symtab (data_in, result);
>> +      else if (TREE_CODE (result) == FUNCTION_DECL && !DECL_BUILT_IN
(result))
>> +     lto_register_function_decl_in_symtab (data_in, result);
>> +    }
>
> I would say we should defer symtab registering to uniquify_nodes time,
> when we are sure we completely streamed in the tree we want to register
> (then we don't have to deal with partially read trees in those functions).
>
> Can you work on a preparatory patch for this please?  That would get
> rid for the need of this hook and clean up things at the same time.

That's a much better idea.  Will do.

>
>>
>>    /* end_marker = */ lto_input_1_unsigned (ib);
>>
>> @@ -2682,6 +2659,22 @@ lto_read_tree (struct lto_input_block *ib, struct
data_in *data_in,
>>  }
>>
>>
>> +/* LTO streamer hook for reading GIMPLE trees.  IB and DATA_IN are as in
>> +   lto_read_tree.  EXPR is the tree was materialized by lto_read_tree and
>> +   needs GIMPLE specific data to be filled in.  */
>> +
>> +void
>> +gimple_streamer_read_tree (struct lto_input_block *ib,
>> +                        struct data_in *data_in,
>> +                        tree expr)
>> +{
>> +  if (DECL_P (expr)
>> +      && TREE_CODE (expr) != FUNCTION_DECL
>> +      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
>> +    DECL_INITIAL (expr) = lto_input_tree (ib, data_in);
>
> What's wrong with doing this in the decl-common path?

Just that if the writer code moves, the reader needs to move as well
to avoid out-of-sync problems.

>> @@ -772,9 +758,23 @@ lto_output_tree_ref (struct output_block *ob, tree expr)
>>        break;
>>
>>      default:
>> -      /* No other node is indexable, so it should have been handled
>> -      by lto_output_tree.  */
>> -      gcc_unreachable ();
>> +      {
>> +     lto_streamer_hooks *h = streamer_hooks ();
>> +
>> +     /* See if streamer hooks allows this node to be indexable with
>> +        VAR_DECLs.  */
>
> with VAR_DECLs?  More like "similar to global decls."?

Changed.

>
>> +     if (h->indexable_with_decls_p && h->indexable_with_decls_p (expr))
>> +       {
>> +         output_record_start (ob, LTO_global_decl_ref);
>> +         lto_output_var_decl_index (ob->decl_state, ob->main_stream, expr);
>
> Why hook it this way and not
>
>             if (h->output_tree_ref
>                 && h->output_tree_ref (...))
>               break;
>             gcc_unreachable ();
>
> I find the flag vs. function hook stuff somewhat odd.

Sure.  It's

>
>> +       }
>> +     else
>> +       {
>> +         /* No other node is indexable, so it should have been
>> +           handled by lto_output_tree.  */
>> +         gcc_unreachable ();
>> +       }
>> +      }
>>      }
>>  }
>>
>> @@ -883,27 +883,10 @@ lto_output_ts_decl_common_tree_pointers (struct
output_block *ob, tree expr,
>>    lto_output_tree_or_ref (ob, DECL_SIZE (expr), ref_p);
>>    lto_output_tree_or_ref (ob, DECL_SIZE_UNIT (expr), ref_p);
>>
>> -  if (TREE_CODE (expr) != FUNCTION_DECL
>> -      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
>> -    {
>> -      tree initial = DECL_INITIAL (expr);
>> -      if (TREE_CODE (expr) == VAR_DECL
>> -       && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
>> -       && initial)
>> -     {
>> -       lto_varpool_encoder_t varpool_encoder =
ob->decl_state->varpool_node_encoder;
>> -       struct varpool_node *vnode = varpool_get_node (expr);
>> -       if (!vnode)
>> -         initial = error_mark_node;
>> -       else if (!lto_varpool_encoder_encode_initializer_p (varpool_encoder,
>> -                                                           vnode))
>> -         initial = NULL;
>> -     }
>> -
>> -      lto_output_tree_or_ref (ob, initial, ref_p);
>> -    }
>> +  /* Do not stream DECL_INITIAL.  */
>
> That's a gross comment ;)  We _do_ stream it, but now in a hook.

Fair enough.

> I suppose all the streamer functions should lose their lto_ prefix -
> reading them with such comments will get confusing ...

Yeah, good point.  I'll send a rename patch.

>>    /* We should not see any non-GIMPLE tree nodes here.  */
>>    code = TREE_CODE (expr);
>> -  if (!lto_is_streamable (expr))
>> -    internal_error ("tree code %qs is not supported in gimple streams",
>> -                 tree_code_name[code]);
>> +  if (h->is_streamable && !h->is_streamable (expr))
>> +    internal_error ("tree code %qs is not supported in %s streams",
>> +                 h->name, tree_code_name[code]);
>
> I'd say this hook should always exist.

Sure.

>>    /* The header of a tree node consists of its tag, the size of
>>       the node, and any other information needed to instantiate
>>       EXPR on the reading side (such as the number of slots in
>>       variable sized nodes).  */
>>    tag = lto_tree_code_to_tag (code);
>> +  gcc_assert ((unsigned) tag < (unsigned) LTO_NUM_TAGS);
>
> Doesn't Honzas streaming this as enum already assert this?

Yeah.  I had removed this before testing, but I sent out the original patch.

>> +/* GIMPLE hook for writing GIMPLE-specific parts of trees.  OB, EXPR
>> +   and REF_P are as in lto_write_tree.  */
>> +
>> +void
>> +gimple_streamer_write_tree (struct output_block *ob, tree expr, bool ref_p)
>> +{
>> +  if (TREE_CODE (expr) == FUNCTION_DECL)
>> +    {
>> +      /* DECL_SAVED_TREE holds the GENERIC representation for DECL.
>> +      At this point, it should not exist.  Either because it was
>> +      converted to gimple or because DECL didn't have a GENERIC
>> +      representation in this TU.  */
>> +      gcc_assert (DECL_SAVED_TREE (expr) == NULL_TREE);
>> +    }
>
> I think we can drop this check.

Done.

>> @@ -1438,8 +1450,27 @@ lto_output_tree (struct output_block *ob, tree expr,
bool ref_p)
>>       to be materialized by the reader (to implement TYPE_CACHED_VALUES).  */
>>    if (TREE_CODE (expr) == INTEGER_CST)
>>      {
>> -      lto_output_integer_cst (ob, expr, ref_p);
>> -      return;
>> +      bool is_special;
>> +
>> +     /* There are some constants that are special to the streamer
>> +     (e.g., void_zero_node, truthvalue_false_node).
>> +     These constants cannot be rematerialized with
>> +     build_int_cst_wide because they may actually lack a type (like
>> +     void_zero_node) and they need to be pointer-identical to trees
>> +     materialized by the compiler tables like global_trees or
>> +     c_global_trees.
>> +
>> +     If the streamer told us that it has special constants, they
>> +     will be preloaded in the streamer cache.  If we find a match,
>> +     then stream the constant as a reference so the reader can
>> +     re-materialize it from the cache.  */
>> +      is_special = streamer_hooks ()->has_unique_integer_csts_p
>> +                && lto_streamer_cache_lookup (ob->writer_cache, expr, NULL);
>> +      if (!is_special)
>> +     {
>> +       lto_output_integer_cst (ob, expr, ref_p);
>> +       return;
>> +     }
>
> ???  We should not arrive here for such global trees.  Please do not
> merge this part of the patch as part of the hook introducing (keep
> patches simple, make them do a single thing ...)

Not sure what you are objecting to.  We do execute this for global
trees in the C++ FE (as described in the comment).  Are you objecting
to never handling unique constants or to merging this handling until
the pph bits are in?

>> @@ -2238,6 +2269,8 @@ static void
>>  lto_writer_init (void)
>>  {
>>    lto_streamer_init ();
>> +  if (streamer_hooks ()->writer_init)
>> +    streamer_hooks ()->writer_init ();
>
> This hook should always exist.  Why is this called in a context with
> lto_*?

There is common streaming code that both the gimple and c++ streamers
use.  I suppose both could call lto_streamer_init and then call their
own initializer routines instead of doing a hook.

>> +
>> +
>> +/* Return true if EXPR is a tree node that can be written to disk.  */
>> +
>> +static inline bool
>> +lto_is_streamable (tree expr)
>> +{
>> +  enum tree_code code = TREE_CODE (expr);
>> +
>> +  /* Notice that we reject SSA_NAMEs as well.  We only emit the SSA
>> +     name version in lto_output_tree_ref (see output_ssa_names).  */
>> +  return !is_lang_specific (expr)
>> +      && code != SSA_NAME
>> +      && code != CALL_EXPR
>> +      && code != LANG_TYPE
>> +      && code != MODIFY_EXPR
>> +      && code != INIT_EXPR
>> +      && code != TARGET_EXPR
>> +      && code != BIND_EXPR
>> +      && code != WITH_CLEANUP_EXPR
>> +      && code != STATEMENT_LIST
>> +      && code != OMP_CLAUSE
>> +      && code != OPTIMIZATION_NODE
>> +      && (code == CASE_LABEL_EXPR
>> +          || code == DECL_EXPR
>> +          || TREE_CODE_CLASS (code) != tcc_statement);
>> +}
>
> This change (with the removal of the cases with the unreachable()s)
> could be made separately (directly calling it instead of hooking it).

I still need to hook this because pph_is_streamable is much more lenient.

>
>> +
>> +/* Initialize all the streamer hooks used for streaming GIMPLE.  */
>> +
>> +void
>> +gimple_streamer_hooks_init (void)
>
> It's more like lto_streamer_hooks_init, no?  You are basically
> turning lto_streamer_* to tree_streamer_* and make lto_streamer_*
> tree_streamer_* + lto_streamer_hooks, no?

I was about to call them gimple_streamer_hooks, but lto_streamer_hooks
is also fine with me.  Subsequent patch.  So:

1- Call the generic implementation and streamer hooks tree_streamer_*
2- Rename the lto-specific parts of streaming to lto_streamer_*
3- Move generic streaming implementation to tree-streamer.[ch]

Does that sound OK?


>> @@ -186,7 +192,7 @@ enum LTO_tags
>>
>>       Conversely, to map between LTO tags and tree/gimple codes, the
>>       reverse operation must be applied.  */
>> -  LTO_bb0 = 1 + NUM_TREE_CODES + LAST_AND_UNUSED_GIMPLE_CODE,
>> +  LTO_bb0 = 1 + MAX_TREE_CODES + LAST_AND_UNUSED_GIMPLE_CODE,
>>    LTO_bb1,
>
> This (and related) change can be merged separately.

Yes.


Thanks for the feedback.


Diego.

[rguenther_suse.de, 2011-06-01 19:19:32]

On Wed, 1 Jun 2011, Diego Novillo wrote:

> On Wed, Jun 1, 2011 at 08:07, Richard Guenther <rguenther@suse.de> wrote:
> 
> >>  static void cgraph_expand_all_functions (void);
> >>  static void cgraph_mark_functions_to_output (void);
> >> @@ -1092,6 +1093,10 @@ cgraph_finalize_compilation_unit (void)
> >>  {
> >>    timevar_push (TV_CGRAPH);
> >>
> >> +  /* If LTO is enabled, initialize the streamer hooks needed by GIMPLE.
 */
> >> +  if (flag_lto)
> >> +    gimple_streamer_hooks_init ();
> >
> > Ugh.  Isn't there a better entry for this?  Are you going to add
> >
> >  if (flag_pph)
> >    init_hooks_some_other_way ();
> >
> > here?  It looks it rather belongs to opts.c or toplev.c if the hooks
> > are really initialized dependent on compiler flags.
> 
> Not at all, this is for gimple, specifically.  The front end
> initializes hooks in its own way.  The problem here is that the gimple
> hooks are needed by the middle end.  If we initialize gimple hooks too
> early, the FE will override them.  So we need to initialize them after
> the front end is done (hence the location for this call).
> 
> I'm happy to move this somewhere else, but it needs to happen right
> before the middle end starts calling LTO pickling routines.
> 
> >
> >>    /* If we're here there's no current function anymore.  Some frontends
> >>       are lazy in clearing these.  */
> >>    current_function_decl = NULL;
> >> diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
> >> index 88966f2..801fe6f 100644
> >> --- a/gcc/lto-streamer-in.c
> >> +++ b/gcc/lto-streamer-in.c
> >> @@ -1833,6 +1833,7 @@ static void
> >>  unpack_value_fields (struct bitpack_d *bp, tree expr)
> >>  {
> >>    enum tree_code code;
> >> +  lto_streamer_hooks *h = streamer_hooks ();
> >
> > A function to access a global ... we have lang_hooks and targetm,
> > so please simply use streamer_hooks as a variable.
> > streamer_hooks ()->preload_common_nodes (cache) looks super-ugly.
> 
> I did not want to add yet another global.  I don't feel too strong
> about this one, given the presence of lang_hooks and targetm.  So, you
> prefer the direct global access?

Yes, I see no benefit of using a global function to get access
to the address of a global variable.

> >> @@ -1864,26 +1865,11 @@ unpack_value_fields (struct bitpack_d *bp, tree
expr)
> >>    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
> >>      unpack_ts_block_value_fields (bp, expr);
> >>
> >> -  if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME))
> >> -    {
> >> -      /* We only stream the version number of SSA names.  */
> >> -      gcc_unreachable ();
> >> -    }
> >> -
> >> -  if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST))
> >> -    {
> >> -      /* This is only used by GENERIC.  */
> >> -      gcc_unreachable ();
> >> -    }
> >> -
> >> -  if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE))
> >> -    {
> >> -      /* This is only used by High GIMPLE.  */
> >> -      gcc_unreachable ();
> >> -    }
> >> -
> >>    if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
> >>      unpack_ts_translation_unit_decl_value_fields (bp, expr);
> >> +
> >> +  if (h->unpack_value_fields)
> >> +    h->unpack_value_fields (bp, expr);
> >
> > I suppose the LTO implementation has a gcc_unreachable () for
> > the cases we do not handle here?
> 
> Right.  This was already superfluous.  It's tested already by
> lto_is_streamable().
> 
> >
> >>  }
> >>
> >>
> >> @@ -1935,8 +1921,17 @@ lto_materialize_tree (struct lto_input_block *ib,
struct data_in *data_in,
> >>      }
> >>    else
> >>      {
> >> -      /* All other nodes can be materialized with a raw make_node call.
 */
> >> -      result = make_node (code);
> >> +      lto_streamer_hooks *h = streamer_hooks ();
> >> +
> >> +      /* For all other nodes, see if the streamer knows how to allocate
> >> +      it.  */
> >> +      if (h->alloc_tree)
> >> +     result = h->alloc_tree (code, ib, data_in);
> >> +
> >> +      /* If the hook did not handle it, materialize the tree with a raw
> >> +      make_node call.  */
> >> +      if (result == NULL_TREE)
> >> +     result = make_node (code);
> >>      }
> >>
> >>  #ifdef LTO_STREAMER_DEBUG
> >> @@ -2031,12 +2026,8 @@ lto_input_ts_decl_common_tree_pointers (struct
lto_input_block *ib,
> >>  {
> >>    DECL_SIZE (expr) = lto_input_tree (ib, data_in);
> >>    DECL_SIZE_UNIT (expr) = lto_input_tree (ib, data_in);
> >> -
> >> -  if (TREE_CODE (expr) != FUNCTION_DECL
> >> -      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
> >> -    DECL_INITIAL (expr) = lto_input_tree (ib, data_in);
> >> -
> >
> > Why move those?  DECL_INITIAL _is_ in decl_common.
> 
> I needed to move the handling of DECL_INITIAL in the writer.  This
> forces us to move the handling in the reader.  Otherwise, reader and
> writer will be out of sync (DECL_INITIAL is now written last).
> 
> > Where do those checks go?  Or do we simply lose them?
> 
> They already are in lto_is_streamable.  See above.
> 
> >> -  if (TREE_CODE (result) == VAR_DECL)
> >> -    lto_register_var_decl_in_symtab (data_in, result);
> >> -  else if (TREE_CODE (result) == FUNCTION_DECL && !DECL_BUILT_IN (result))
> >> -    lto_register_function_decl_in_symtab (data_in, result);
> >> +  if (h->register_decls_in_symtab_p)
> >> +    {
> >> +      if (TREE_CODE (result) == VAR_DECL)
> >> +     lto_register_var_decl_in_symtab (data_in, result);
> >> +      else if (TREE_CODE (result) == FUNCTION_DECL && !DECL_BUILT_IN
(result))
> >> +     lto_register_function_decl_in_symtab (data_in, result);
> >> +    }
> >
> > I would say we should defer symtab registering to uniquify_nodes time,
> > when we are sure we completely streamed in the tree we want to register
> > (then we don't have to deal with partially read trees in those functions).
> >
> > Can you work on a preparatory patch for this please?  That would get
> > rid for the need of this hook and clean up things at the same time.
> 
> That's a much better idea.  Will do.

Thanks.

> >
> >>
> >>    /* end_marker = */ lto_input_1_unsigned (ib);
> >>
> >> @@ -2682,6 +2659,22 @@ lto_read_tree (struct lto_input_block *ib, struct
data_in *data_in,
> >>  }
> >>
> >>
> >> +/* LTO streamer hook for reading GIMPLE trees.  IB and DATA_IN are as in
> >> +   lto_read_tree.  EXPR is the tree was materialized by lto_read_tree and
> >> +   needs GIMPLE specific data to be filled in.  */
> >> +
> >> +void
> >> +gimple_streamer_read_tree (struct lto_input_block *ib,
> >> +                        struct data_in *data_in,
> >> +                        tree expr)
> >> +{
> >> +  if (DECL_P (expr)
> >> +      && TREE_CODE (expr) != FUNCTION_DECL
> >> +      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
> >> +    DECL_INITIAL (expr) = lto_input_tree (ib, data_in);
> >
> > What's wrong with doing this in the decl-common path?
> 
> Just that if the writer code moves, the reader needs to move as well
> to avoid out-of-sync problems.
> 
> >> @@ -772,9 +758,23 @@ lto_output_tree_ref (struct output_block *ob, tree
expr)
> >>        break;
> >>
> >>      default:
> >> -      /* No other node is indexable, so it should have been handled
> >> -      by lto_output_tree.  */
> >> -      gcc_unreachable ();
> >> +      {
> >> +     lto_streamer_hooks *h = streamer_hooks ();
> >> +
> >> +     /* See if streamer hooks allows this node to be indexable with
> >> +        VAR_DECLs.  */
> >
> > with VAR_DECLs?  More like "similar to global decls."?
> 
> Changed.
> 
> >
> >> +     if (h->indexable_with_decls_p && h->indexable_with_decls_p (expr))
> >> +       {
> >> +         output_record_start (ob, LTO_global_decl_ref);
> >> +         lto_output_var_decl_index (ob->decl_state, ob->main_stream,
expr);
> >
> > Why hook it this way and not
> >
> >             if (h->output_tree_ref
> >                 && h->output_tree_ref (...))
> >               break;
> >             gcc_unreachable ();
> >
> > I find the flag vs. function hook stuff somewhat odd.
> 
> Sure.  It's

... missing words? ;)

> >
> >> +       }
> >> +     else
> >> +       {
> >> +         /* No other node is indexable, so it should have been
> >> +           handled by lto_output_tree.  */
> >> +         gcc_unreachable ();
> >> +       }
> >> +      }
> >>      }
> >>  }
> >>
> >> @@ -883,27 +883,10 @@ lto_output_ts_decl_common_tree_pointers (struct
output_block *ob, tree expr,
> >>    lto_output_tree_or_ref (ob, DECL_SIZE (expr), ref_p);
> >>    lto_output_tree_or_ref (ob, DECL_SIZE_UNIT (expr), ref_p);
> >>
> >> -  if (TREE_CODE (expr) != FUNCTION_DECL
> >> -      && TREE_CODE (expr) != TRANSLATION_UNIT_DECL)
> >> -    {
> >> -      tree initial = DECL_INITIAL (expr);
> >> -      if (TREE_CODE (expr) == VAR_DECL
> >> -       && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
> >> -       && initial)
> >> -     {
> >> -       lto_varpool_encoder_t varpool_encoder =
ob->decl_state->varpool_node_encoder;
> >> -       struct varpool_node *vnode = varpool_get_node (expr);
> >> -       if (!vnode)
> >> -         initial = error_mark_node;
> >> -       else if (!lto_varpool_encoder_encode_initializer_p
(varpool_encoder,
> >> -                                                           vnode))
> >> -         initial = NULL;
> >> -     }
> >> -
> >> -      lto_output_tree_or_ref (ob, initial, ref_p);
> >> -    }
> >> +  /* Do not stream DECL_INITIAL.  */
> >
> > That's a gross comment ;)  We _do_ stream it, but now in a hook.
> 
> Fair enough.
> 
> > I suppose all the streamer functions should lose their lto_ prefix -
> > reading them with such comments will get confusing ...
> 
> Yeah, good point.  I'll send a rename patch.
> 
> >>    /* We should not see any non-GIMPLE tree nodes here.  */
> >>    code = TREE_CODE (expr);
> >> -  if (!lto_is_streamable (expr))
> >> -    internal_error ("tree code %qs is not supported in gimple streams",
> >> -                 tree_code_name[code]);
> >> +  if (h->is_streamable && !h->is_streamable (expr))
> >> +    internal_error ("tree code %qs is not supported in %s streams",
> >> +                 h->name, tree_code_name[code]);
> >
> > I'd say this hook should always exist.
> 
> Sure.
> 
> >>    /* The header of a tree node consists of its tag, the size of
> >>       the node, and any other information needed to instantiate
> >>       EXPR on the reading side (such as the number of slots in
> >>       variable sized nodes).  */
> >>    tag = lto_tree_code_to_tag (code);
> >> +  gcc_assert ((unsigned) tag < (unsigned) LTO_NUM_TAGS);
> >
> > Doesn't Honzas streaming this as enum already assert this?
> 
> Yeah.  I had removed this before testing, but I sent out the original patch.
> 
> >> +/* GIMPLE hook for writing GIMPLE-specific parts of trees.  OB, EXPR
> >> +   and REF_P are as in lto_write_tree.  */
> >> +
> >> +void
> >> +gimple_streamer_write_tree (struct output_block *ob, tree expr, bool
ref_p)
> >> +{
> >> +  if (TREE_CODE (expr) == FUNCTION_DECL)
> >> +    {
> >> +      /* DECL_SAVED_TREE holds the GENERIC representation for DECL.
> >> +      At this point, it should not exist.  Either because it was
> >> +      converted to gimple or because DECL didn't have a GENERIC
> >> +      representation in this TU.  */
> >> +      gcc_assert (DECL_SAVED_TREE (expr) == NULL_TREE);
> >> +    }
> >
> > I think we can drop this check.
> 
> Done.
> 
> >> @@ -1438,8 +1450,27 @@ lto_output_tree (struct output_block *ob, tree expr,
bool ref_p)
> >>       to be materialized by the reader (to implement TYPE_CACHED_VALUES).
 */
> >>    if (TREE_CODE (expr) == INTEGER_CST)
> >>      {
> >> -      lto_output_integer_cst (ob, expr, ref_p);
> >> -      return;
> >> +      bool is_special;
> >> +
> >> +     /* There are some constants that are special to the streamer
> >> +     (e.g., void_zero_node, truthvalue_false_node).
> >> +     These constants cannot be rematerialized with
> >> +     build_int_cst_wide because they may actually lack a type (like
> >> +     void_zero_node) and they need to be pointer-identical to trees
> >> +     materialized by the compiler tables like global_trees or
> >> +     c_global_trees.
> >> +
> >> +     If the streamer told us that it has special constants, they
> >> +     will be preloaded in the streamer cache.  If we find a match,
> >> +     then stream the constant as a reference so the reader can
> >> +     re-materialize it from the cache.  */
> >> +      is_special = streamer_hooks ()->has_unique_integer_csts_p
> >> +                && lto_streamer_cache_lookup (ob->writer_cache, expr,
NULL);
> >> +      if (!is_special)
> >> +     {
> >> +       lto_output_integer_cst (ob, expr, ref_p);
> >> +       return;
> >> +     }
> >
> > ???  We should not arrive here for such global trees.  Please do not
> > merge this part of the patch as part of the hook introducing (keep
> > patches simple, make them do a single thing ...)
> 
> Not sure what you are objecting to.  We do execute this for global
> trees in the C++ FE (as described in the comment).  Are you objecting
> to never handling unique constants or to merging this handling until
> the pph bits are in?

Are you not pre-loading those global trees then?  Yes, I think this isn't
the time to merge this piece.  Ah, I think I get it - we don't stream
integer constants as trees.  But it's odd that you only handle this
for integer-csts and not other trees we don't stream as-is (and
thus do not enter into the cache) - I think this should be moved
up a level and made generic to handle all trees.  Or we should
handle integer-csts similar to builtins - always enter them in the cache,
or only handle all pre-loaded nodes that way.

> >> @@ -2238,6 +2269,8 @@ static void
> >>  lto_writer_init (void)
> >>  {
> >>    lto_streamer_init ();
> >> +  if (streamer_hooks ()->writer_init)
> >> +    streamer_hooks ()->writer_init ();
> >
> > This hook should always exist.  Why is this called in a context with
> > lto_*?
> 
> There is common streaming code that both the gimple and c++ streamers
> use.  I suppose both could call lto_streamer_init and then call their
> own initializer routines instead of doing a hook.

Yeah, I'd prefer that.  I don't have a clear picture yet on what
piece of the streamer is actually shared.
 
> >> +
> >> +
> >> +/* Return true if EXPR is a tree node that can be written to disk.  */
> >> +
> >> +static inline bool
> >> +lto_is_streamable (tree expr)
> >> +{
> >> +  enum tree_code code = TREE_CODE (expr);
> >> +
> >> +  /* Notice that we reject SSA_NAMEs as well.  We only emit the SSA
> >> +     name version in lto_output_tree_ref (see output_ssa_names).  */
> >> +  return !is_lang_specific (expr)
> >> +      && code != SSA_NAME
> >> +      && code != CALL_EXPR
> >> +      && code != LANG_TYPE
> >> +      && code != MODIFY_EXPR
> >> +      && code != INIT_EXPR
> >> +      && code != TARGET_EXPR
> >> +      && code != BIND_EXPR
> >> +      && code != WITH_CLEANUP_EXPR
> >> +      && code != STATEMENT_LIST
> >> +      && code != OMP_CLAUSE
> >> +      && code != OPTIMIZATION_NODE
> >> +      && (code == CASE_LABEL_EXPR
> >> +          || code == DECL_EXPR
> >> +          || TREE_CODE_CLASS (code) != tcc_statement);
> >> +}
> >
> > This change (with the removal of the cases with the unreachable()s)
> > could be made separately (directly calling it instead of hooking it).
> 
> I still need to hook this because pph_is_streamable is much more lenient.

Of course, you can do that in the hookization patch.  The above is just
to make that smaller.

> >
> >> +
> >> +/* Initialize all the streamer hooks used for streaming GIMPLE.  */
> >> +
> >> +void
> >> +gimple_streamer_hooks_init (void)
> >
> > It's more like lto_streamer_hooks_init, no?  You are basically
> > turning lto_streamer_* to tree_streamer_* and make lto_streamer_*
> > tree_streamer_* + lto_streamer_hooks, no?
> 
> I was about to call them gimple_streamer_hooks, but lto_streamer_hooks
> is also fine with me.  Subsequent patch.  So:
> 
> 1- Call the generic implementation and streamer hooks tree_streamer_*
> 2- Rename the lto-specific parts of streaming to lto_streamer_*
> 3- Move generic streaming implementation to tree-streamer.[ch]
> 
> Does that sound OK?

That sounds ok.  You are only sharing the code streaming trees, right?

Richard.

[Diego Novillo, 2011-06-04 19:30:34]

On Wed, Jun 1, 2011 at 15:19, Richard Guenther <rguenther@suse.de> wrote:

> Yes, I see no benefit of using a global function to get access
> to the address of a global variable.

There is the minor benefit of being able to control  access to it, but
I don't have a really convincing reason to give you, so I changed it
to a global.

>> >> +     if (h->indexable_with_decls_p && h->indexable_with_decls_p (expr))
>> >> +       {
>> >> +         output_record_start (ob, LTO_global_decl_ref);
>> >> +         lto_output_var_decl_index (ob->decl_state, ob->main_stream,
expr);
>> >
>> > Why hook it this way and not
>> >
>> >             if (h->output_tree_ref
>> >                 && h->output_tree_ref (...))
>> >               break;
>> >             gcc_unreachable ();
>> >
>> > I find the flag vs. function hook stuff somewhat odd.
>>
>> Sure.  It's
>
> ... missing words? ;)

Sorry.  I meant to continue with "It's just that this particular hook
is simpler if it only needs to decide whether the node can be written
as a decl reference.  The code to write the node will be the same
everywhere."  It would lead to duplication and the hooks would need to
know more internal details of the generic streamer (they need to write
the reference in exactly the way that lto_input_tree is expecting).

This is not a flag, actually.  It's a predicate function called on a
node.  If the node passes the predicate, then it is written in the
decl index table.


>> >> @@ -1438,8 +1450,27 @@ lto_output_tree (struct output_block *ob, tree
expr, bool ref_p)
>> >>       to be materialized by the reader (to implement TYPE_CACHED_VALUES).
 */
>> >>    if (TREE_CODE (expr) == INTEGER_CST)
>> >>      {
>> >> -      lto_output_integer_cst (ob, expr, ref_p);
>> >> -      return;
>> >> +      bool is_special;
>> >> +
>> >> +     /* There are some constants that are special to the streamer
>> >> +     (e.g., void_zero_node, truthvalue_false_node).
>> >> +     These constants cannot be rematerialized with
>> >> +     build_int_cst_wide because they may actually lack a type (like
>> >> +     void_zero_node) and they need to be pointer-identical to trees
>> >> +     materialized by the compiler tables like global_trees or
>> >> +     c_global_trees.
>> >> +
>> >> +     If the streamer told us that it has special constants, they
>> >> +     will be preloaded in the streamer cache.  If we find a match,
>> >> +     then stream the constant as a reference so the reader can
>> >> +     re-materialize it from the cache.  */
>> >> +      is_special = streamer_hooks ()->has_unique_integer_csts_p
>> >> +                && lto_streamer_cache_lookup (ob->writer_cache, expr,
NULL);
>> >> +      if (!is_special)
>> >> +     {
>> >> +       lto_output_integer_cst (ob, expr, ref_p);
>> >> +       return;
>> >> +     }
>> >
>> > ???  We should not arrive here for such global trees.  Please do not
>> > merge this part of the patch as part of the hook introducing (keep
>> > patches simple, make them do a single thing ...)
>>
>> Not sure what you are objecting to.  We do execute this for global
>> trees in the C++ FE (as described in the comment).  Are you objecting
>> to never handling unique constants or to merging this handling until
>> the pph bits are in?
>
> Are you not pre-loading those global trees then?

I am, but since the streamer always wanted to stream INTEGER_CSTs
separately, it wasn't getting a chance to check the cache first.

> Yes, I think this isn't the time to merge this piece.

No problem.  I'll keep this part in the branch.

> Ah, I think I get it - we don't stream integer constants as trees.

Right.

> But it's odd that you only handle this
> for integer-csts and not other trees we don't stream as-is (and
> thus do not enter into the cache)

Because constants are the only ones that are handled right before the
cache is consulted.  Every other pre-built tree can be cached
(regardless of whether it's handled by the streamer).

> - I think this should be moved up a level and made generic to handle all
trees.  Or we should
> handle integer-csts similar to builtins - always enter them in the cache,

I tried this, but the result was sub-optimal
(http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00563.html)
Putting constants in the cache, caused various failures which I never
fully debugged because I noticed an increase in the object size (I
remember it was noticeable, but not by how much).

I didn't insist too much with this approach, so maybe I could try it again.


> or only handle all pre-loaded nodes that way.

That is what we do.  Pre-loaded nodes always go through the cache.
The problem were pre-loaded constants, since they *never* go through
the cache.

>> >> @@ -2238,6 +2269,8 @@ static void
>> >>  lto_writer_init (void)
>> >>  {
>> >>    lto_streamer_init ();
>> >> +  if (streamer_hooks ()->writer_init)
>> >> +    streamer_hooks ()->writer_init ();
>> >
>> > This hook should always exist.  Why is this called in a context with
>> > lto_*?
>>
>> There is common streaming code that both the gimple and c++ streamers
>> use.  I suppose both could call lto_streamer_init and then call their
>> own initializer routines instead of doing a hook.
>
> Yeah, I'd prefer that.  I don't have a clear picture yet on what
> piece of the streamer is actually shared.

Sure.  Done.

>> >> +
>> >> +/* Initialize all the streamer hooks used for streaming GIMPLE.  */
>> >> +
>> >> +void
>> >> +gimple_streamer_hooks_init (void)
>> >
>> > It's more like lto_streamer_hooks_init, no?  You are basically
>> > turning lto_streamer_* to tree_streamer_* and make lto_streamer_*
>> > tree_streamer_* + lto_streamer_hooks, no?
>>
>> I was about to call them gimple_streamer_hooks, but lto_streamer_hooks
>> is also fine with me.  Subsequent patch.  So:
>>
>> 1- Call the generic implementation and streamer hooks tree_streamer_*
>> 2- Rename the lto-specific parts of streaming to lto_streamer_*
>> 3- Move generic streaming implementation to tree-streamer.[ch]
>>
>> Does that sound OK?
>
> That sounds ok.  You are only sharing the code streaming trees, right?

Right.  Patch coming up on top of the revised patch for streamer hooks.

The attached revision of the patch has been tested again with an LTO
profiledbootstrap on x86_64.  OK for trunk?

	* Makefile.in (lto-compress.o): Add dependency on LTO_STREAMER_H.
	(cgraph.o): Likewise.
	(cgraphunit.o): Likewise.
	* cgraphunit.c: Include lto-streamer.h
	(cgraph_finalize_compilation_unit): Call lto_streamer_hooks_init
	if LTO is enabled.
	* lto-streamer-in.c (unpack_value_fields): Call
	streamer_hooks.unpack_value_fields if set.
	(lto_materialize_tree): For unhandled nodes, first try to
	call lto_streamer_hooks.alloc_tree, if it exists.
	(lto_input_ts_decl_common_tree_pointers): Move reading of
	DECL_INITIAL to lto_streamer_read_tree.
	(lto_read_tree): Call lto_streamer_hooks.read_tree if set.
	(lto_streamer_read_tree): New.
	(lto_reader_init): Rename from lto_init_reader.
	Move initialization code to lto/lto.c.
	* lto-streamer-out.c (pack_value_fields): Call
	streamer_hooks.pack_value_fields if set.
	(lto_output_tree_ref): For tree nodes that are not
	normally indexable, call streamer_hooks.indexable_with_decls_p
	before giving up.
	(lto_output_ts_decl_common_tree_pointers): Move handling
	for FUNCTION_DECL and TRANSLATION_UNIT_DECL to
	lto_streamer_write_tree.
	(lto_output_tree_header): Call streamer_hooks.is_streamable
	instead of lto_is_streamable.
	Call lto_streamer_hooks.output_tree_header if set.
	(lto_write_tree): Call lto_streamer_hooks.write_tree if
	set.
	(lto_streamer_write_tree): New.
	(lto_output): Call lto_streamer_init directly.
	(lto_writer_init): Remove.
	* lto-streamer.c (streamer_hooks): New.
	(lto_streamer_cache_create): Call streamer_hooks.preload_common_nodes
	instead of lto_preload_common_nodes.
	(lto_is_streamable): Move from lto-streamer.h
	(lto_streamer_hooks_init): New.
	(streamer_hooks): New.
	(streamer_hooks_init): New.
	* lto-streamer.h (struct output_block): Forward declare.
	(struct lto_input_block): Likewise.
	(struct data_in): Likewise.
	(struct bitpack_d): Likewise.
	(struct streamer_hooks): Declare.
	(streamer_hooks): Declare.
	(lto_streamer_hooks_init): Declare.
	(lto_streamer_write_tree): Declare.
	(lto_streamer_read_tree): Declare.
	(streamer_hooks_init): Declare.
	(lto_is_streamable): Move to lto-streamer.c

lto/ChangeLog

	* lto.c (lto_init): New.
	(lto_main): Call it.


Diego.

[rguenther_suse.de, 2011-06-06 14:51:00]

On Sat, 4 Jun 2011, Diego Novillo wrote:

> On Wed, Jun 1, 2011 at 15:19, Richard Guenther <rguenther@suse.de> wrote:
> 
> > Yes, I see no benefit of using a global function to get access
> > to the address of a global variable.
> 
> There is the minor benefit of being able to control  access to it, but
> I don't have a really convincing reason to give you, so I changed it
> to a global.

Thanks.  It feels more consistent this way.

> >> >> +     if (h->indexable_with_decls_p && h->indexable_with_decls_p (expr))
> >> >> +       {
> >> >> +         output_record_start (ob, LTO_global_decl_ref);
> >> >> +         lto_output_var_decl_index (ob->decl_state, ob->main_stream,
expr);
> >> >
> >> > Why hook it this way and not
> >> >
> >> >             if (h->output_tree_ref
> >> >                 && h->output_tree_ref (...))
> >> >               break;
> >> >             gcc_unreachable ();
> >> >
> >> > I find the flag vs. function hook stuff somewhat odd.
> >>
> >> Sure.  It's
> >
> > ... missing words? ;)
> 
> Sorry.  I meant to continue with "It's just that this particular hook
> is simpler if it only needs to decide whether the node can be written
> as a decl reference.  The code to write the node will be the same
> everywhere."  It would lead to duplication and the hooks would need to
> know more internal details of the generic streamer (they need to write
> the reference in exactly the way that lto_input_tree is expecting).
> 
> This is not a flag, actually.  It's a predicate function called on a
> node.  If the node passes the predicate, then it is written in the
> decl index table.

Hm, ok.

> 
> >> >> @@ -1438,8 +1450,27 @@ lto_output_tree (struct output_block *ob, tree
expr, bool ref_p)
> >> >>       to be materialized by the reader (to implement
TYPE_CACHED_VALUES).  */
> >> >>    if (TREE_CODE (expr) == INTEGER_CST)
> >> >>      {
> >> >> -      lto_output_integer_cst (ob, expr, ref_p);
> >> >> -      return;
> >> >> +      bool is_special;
> >> >> +
> >> >> +     /* There are some constants that are special to the streamer
> >> >> +     (e.g., void_zero_node, truthvalue_false_node).
> >> >> +     These constants cannot be rematerialized with
> >> >> +     build_int_cst_wide because they may actually lack a type (like
> >> >> +     void_zero_node) and they need to be pointer-identical to trees
> >> >> +     materialized by the compiler tables like global_trees or
> >> >> +     c_global_trees.
> >> >> +
> >> >> +     If the streamer told us that it has special constants, they
> >> >> +     will be preloaded in the streamer cache.  If we find a match,
> >> >> +     then stream the constant as a reference so the reader can
> >> >> +     re-materialize it from the cache.  */
> >> >> +      is_special = streamer_hooks ()->has_unique_integer_csts_p
> >> >> +                && lto_streamer_cache_lookup (ob->writer_cache, expr,
NULL);
> >> >> +      if (!is_special)
> >> >> +     {
> >> >> +       lto_output_integer_cst (ob, expr, ref_p);
> >> >> +       return;
> >> >> +     }
> >> >
> >> > ???  We should not arrive here for such global trees.  Please do not
> >> > merge this part of the patch as part of the hook introducing (keep
> >> > patches simple, make them do a single thing ...)
> >>
> >> Not sure what you are objecting to.  We do execute this for global
> >> trees in the C++ FE (as described in the comment).  Are you objecting
> >> to never handling unique constants or to merging this handling until
> >> the pph bits are in?
> >
> > Are you not pre-loading those global trees then?
> 
> I am, but since the streamer always wanted to stream INTEGER_CSTs
> separately, it wasn't getting a chance to check the cache first.
> 
> > Yes, I think this isn't the time to merge this piece.
> 
> No problem.  I'll keep this part in the branch.
> 
> > Ah, I think I get it - we don't stream integer constants as trees.
> 
> Right.
> 
> > But it's odd that you only handle this
> > for integer-csts and not other trees we don't stream as-is (and
> > thus do not enter into the cache)
> 
> Because constants are the only ones that are handled right before the
> cache is consulted.  Every other pre-built tree can be cached
> (regardless of whether it's handled by the streamer).
> 
> > - I think this should be moved up a level and made generic to handle all
trees.  Or we should
> > handle integer-csts similar to builtins - always enter them in the cache,
> 
> I tried this, but the result was sub-optimal
> (http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00563.html)
> Putting constants in the cache, caused various failures which I never
> fully debugged because I noticed an increase in the object size (I
> remember it was noticeable, but not by how much).
> 
> I didn't insist too much with this approach, so maybe I could try it again.
> 
> 
> > or only handle all pre-loaded nodes that way.
> 
> That is what we do.  Pre-loaded nodes always go through the cache.
> The problem were pre-loaded constants, since they *never* go through
> the cache.

Do you remember if it was only void_zero_node that causes problems?
We could just special-case it in 
lto_input_integer_cst/lto_output_integer_cst.  Or even fix the C family
frontends to not create or use this strange node.  It seems to be
a special tree for "empty valid something" which could as well be
a new tree code with a singleton object.

> >> >> @@ -2238,6 +2269,8 @@ static void
> >> >>  lto_writer_init (void)
> >> >>  {
> >> >>    lto_streamer_init ();
> >> >> +  if (streamer_hooks ()->writer_init)
> >> >> +    streamer_hooks ()->writer_init ();
> >> >
> >> > This hook should always exist.  Why is this called in a context with
> >> > lto_*?
> >>
> >> There is common streaming code that both the gimple and c++ streamers
> >> use.  I suppose both could call lto_streamer_init and then call their
> >> own initializer routines instead of doing a hook.
> >
> > Yeah, I'd prefer that.  I don't have a clear picture yet on what
> > piece of the streamer is actually shared.
> 
> Sure.  Done.
> 
> >> >> +
> >> >> +/* Initialize all the streamer hooks used for streaming GIMPLE.  */
> >> >> +
> >> >> +void
> >> >> +gimple_streamer_hooks_init (void)
> >> >
> >> > It's more like lto_streamer_hooks_init, no?  You are basically
> >> > turning lto_streamer_* to tree_streamer_* and make lto_streamer_*
> >> > tree_streamer_* + lto_streamer_hooks, no?
> >>
> >> I was about to call them gimple_streamer_hooks, but lto_streamer_hooks
> >> is also fine with me.  Subsequent patch.  So:
> >>
> >> 1- Call the generic implementation and streamer hooks tree_streamer_*
> >> 2- Rename the lto-specific parts of streaming to lto_streamer_*
> >> 3- Move generic streaming implementation to tree-streamer.[ch]
> >>
> >> Does that sound OK?
> >
> > That sounds ok.  You are only sharing the code streaming trees, right?
> 
> Right.  Patch coming up on top of the revised patch for streamer hooks.
> 
> The attached revision of the patch has been tested again with an LTO
> profiledbootstrap on x86_64.  OK for trunk?

Looks good to me.

Thanks,
Richard.

> 	* Makefile.in (lto-compress.o): Add dependency on LTO_STREAMER_H.
> 	(cgraph.o): Likewise.
> 	(cgraphunit.o): Likewise.
> 	* cgraphunit.c: Include lto-streamer.h
> 	(cgraph_finalize_compilation_unit): Call lto_streamer_hooks_init
> 	if LTO is enabled.
> 	* lto-streamer-in.c (unpack_value_fields): Call
> 	streamer_hooks.unpack_value_fields if set.
> 	(lto_materialize_tree): For unhandled nodes, first try to
> 	call lto_streamer_hooks.alloc_tree, if it exists.
> 	(lto_input_ts_decl_common_tree_pointers): Move reading of
> 	DECL_INITIAL to lto_streamer_read_tree.
> 	(lto_read_tree): Call lto_streamer_hooks.read_tree if set.
> 	(lto_streamer_read_tree): New.
> 	(lto_reader_init): Rename from lto_init_reader.
> 	Move initialization code to lto/lto.c.
> 	* lto-streamer-out.c (pack_value_fields): Call
> 	streamer_hooks.pack_value_fields if set.
> 	(lto_output_tree_ref): For tree nodes that are not
> 	normally indexable, call streamer_hooks.indexable_with_decls_p
> 	before giving up.
> 	(lto_output_ts_decl_common_tree_pointers): Move handling
> 	for FUNCTION_DECL and TRANSLATION_UNIT_DECL to
> 	lto_streamer_write_tree.
> 	(lto_output_tree_header): Call streamer_hooks.is_streamable
> 	instead of lto_is_streamable.
> 	Call lto_streamer_hooks.output_tree_header if set.
> 	(lto_write_tree): Call lto_streamer_hooks.write_tree if
> 	set.
> 	(lto_streamer_write_tree): New.
> 	(lto_output): Call lto_streamer_init directly.
> 	(lto_writer_init): Remove.
> 	* lto-streamer.c (streamer_hooks): New.
> 	(lto_streamer_cache_create): Call streamer_hooks.preload_common_nodes
> 	instead of lto_preload_common_nodes.
> 	(lto_is_streamable): Move from lto-streamer.h
> 	(lto_streamer_hooks_init): New.
> 	(streamer_hooks): New.
> 	(streamer_hooks_init): New.
> 	* lto-streamer.h (struct output_block): Forward declare.
> 	(struct lto_input_block): Likewise.
> 	(struct data_in): Likewise.
> 	(struct bitpack_d): Likewise.
> 	(struct streamer_hooks): Declare.
> 	(streamer_hooks): Declare.
> 	(lto_streamer_hooks_init): Declare.
> 	(lto_streamer_write_tree): Declare.
> 	(lto_streamer_read_tree): Declare.
> 	(streamer_hooks_init): Declare.
> 	(lto_is_streamable): Move to lto-streamer.c
> 
> lto/ChangeLog
> 
> 	* lto.c (lto_init): New.
> 	(lto_main): Call it.
> 
> 
> Diego.
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

[Diego Novillo, 2011-06-06 15:33:08]

On Mon, Jun 6, 2011 at 10:50, Richard Guenther <rguenther@suse.de> wrote:

> Do you remember if it was only void_zero_node that causes problems?
> We could just special-case it in
> lto_input_integer_cst/lto_output_integer_cst.  Or even fix the C family
> frontends to not create or use this strange node.  It seems to be
> a special tree for "empty valid something" which could as well be
> a new tree code with a singleton object.

void_zero_node was the only one causing problems because it couldn't
be built with build_int_cst_wide.  However, the other well-known
constants where causing problem when the parser tried to
pointer-compare them.  This sounded a bit strange to me, since I was
expecting the constant cache in the type to give back shared
constants, but I did not investigate it further.

Making void_zero_node a new singleton object sounds like a good cleanup.


Diego.