[pph] Stream out chains backwards

[Gabriel Charette, 2011-07-11 18:24:37]

**This patch goes on top of patches in issues 4672055 and 4675069 (which have
yet to be committed)**

Some things are built as soon as a tree is streamed in, and since the chains are
backwards, flipping them after streaming them in is not sufficient as some
things (e.g. unique numbers given to functions) have already been allocated.

The solution is to stream out the chain backwards to begin with.

This fixes the assembly diffs in which the LFB# were different in the pph and
non-pph assembly.

As noted by a FIXME comment, we probably want to do this for usings and
using_directives as well, but I didn't for this patch as we don't handle those
yet, and I'm not sure whether their chain is flipped or not.

Fixed tests x1functions and c1pr36533.
The c1pr44948-1a test changed from an ICE in lto_streamer_cache_get to an ICE in
lto_get_pickled_tree, but lto_get_pickled_tree calls lto_streamer_cache_get and
I'm pretty sure this is the same bug, not a new one introduced by this patch.

Tested with bootstrap build and pph regression testing.

2011-07-11  Gabriel Charette  <gchare@google.com>

	* pph-streamer-in.c (pph_add_bindings_to_namespace): Don't reverse 
	names and namespaces chains. Reverse names and namespaces
	only for the binding levels of namespaces streamed in as is.
	* pph-streamer-out.c (pph_out_chained_tree): New.
	(pph_out_chain_filtered): Add REVERSE parameter.
	(pph_out_binding_level): Use REVERSE parameter of
	pph_out_chain_filtered.
	* g++.dg/pph/c1pr36533.cc: Expect no asm difference.
	* g++.dg/pph/c1pr44948-1a.cc: Adjust XFAIL pattern.
	* g++.dg/pph/x1functions.cc: Expect no asm difference.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 55f7e12..fde1b93 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -1146,11 +1146,6 @@ pph_add_bindings_to_namespace (struct cp_binding_level
*bl, tree ns)
 {
   tree t, chain;
 
-  /* The chains are built backwards (ref: add_decl_to_level),
-     reverse them before putting them back in.  */
-  bl->names = nreverse (bl->names);
-  bl->namespaces = nreverse (bl->namespaces);
-
   for (t = bl->names; t; t = chain)
     {
       /* Pushing a decl into a scope clobbers its DECL_CHAIN.
@@ -1164,11 +1159,26 @@ pph_add_bindings_to_namespace (struct cp_binding_level
*bl, tree ns)
 
   for (t = bl->namespaces; t; t = chain)
     {
+      struct cp_binding_level* ns_lvl;
+
       /* Pushing a decl into a scope clobbers its DECL_CHAIN.
 	 Preserve it.  */
       chain = DECL_CHAIN (t);
       pushdecl_into_namespace (t, ns);
-      pph_add_bindings_to_namespace (NAMESPACE_LEVEL (t), t);
+
+      /* FIXME pph: verify whether this namespace exists already,
+	 if it does we should merge it.  */
+      ns_lvl = NAMESPACE_LEVEL (t);
+      /* FIXME pph: the only benefit of making this call is the embedded call
to
+	 varpool_finalize_decl for the names contained in this namespace and
+	 it's transitive closure of namespaces, the bindings themselves do NOT
+	 need to be added to this namespace as they are already part of it.  */
+      pph_add_bindings_to_namespace (ns_lvl, t);
+      /* Adding the bindings to another namespace automatically reverses them,
but
+	 since these were already part of this namespace, they weren't: reverse
+	 them in place now.  */
+      ns_lvl->names = nreverse (ns_lvl->names);
+      ns_lvl->namespaces = nreverse (ns_lvl->namespaces);
     }
 }
 
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index d1e757f..445fca5 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -584,21 +584,44 @@ pph_out_label_binding (pph_stream *stream,
cp_label_binding *lb, bool ref_p)
 }
 
 
+/* Outputs chained tree T by nulling out it's chain first and restoring it
+   after the streaming is done. STREAM and REF_P are as in
+   pph_out_chain_filtered.  */
+
+static inline void
+pph_out_chained_tree (pph_stream *stream, tree t, bool ref_p)
+{
+  tree saved_chain;
+
+  saved_chain = TREE_CHAIN (t);
+  TREE_CHAIN (t) = NULL_TREE;
+
+  pph_out_tree_or_ref_1 (stream, t, ref_p, 2);
+
+  TREE_CHAIN (t) = saved_chain;
+}
+
+
 /* Output a chain of nodes to STREAM starting with FIRST.  Skip any
    nodes that do not match FILTER.  REF_P is true if nodes in the chain
-   should be emitted as references.  */
+   should be emitted as references.  Stream the chain in the reverse order
+   if REVERSE is true.*/
 
 static void
 pph_out_chain_filtered (pph_stream *stream, tree first, bool ref_p,
-			   enum chain_filter filter)
+			   enum chain_filter filter, bool reverse)
 {
   unsigned count;
+  int i;
   tree t;
+  tree *to_stream = NULL;
 
   /* Special case.  If the caller wants no filtering, it is much
      faster to just call pph_out_chain directly.  */
   if (filter == NONE)
     {
+      if (reverse)
+	nreverse (first);
       pph_out_chain (stream, first, ref_p);
       return;
     }
@@ -612,23 +635,36 @@ pph_out_chain_filtered (pph_stream *stream, tree first,
bool ref_p,
     }
   pph_out_uint (stream, count);
 
+  /* We cannot use the actual chain and simply reverse that before streaming
+     out below as there are other chains being streamed out as part of
+     streaming the trees in the current chain and this creates conflicts.
+     Thus, we will create an array containing all the trees we want to stream
+     out and stream that backwards without altering the chain itself.  */
+  if (reverse && count > 0)
+    {
+      to_stream = (tree*) xcalloc (count, sizeof (tree));
+      i = 0;
+    }
+
   /* Output all the nodes that match the filter.  */
   for (t = first; t; t = TREE_CHAIN (t))
     {
-      tree saved_chain;
-
       /* Apply filters to T.  */
       if (filter == NO_BUILTINS && DECL_P (t) && DECL_IS_BUILTIN (t))
 	continue;
 
-      /* Clear TREE_CHAIN to avoid blindly recursing into the rest
-	 of the list.  */
-      saved_chain = TREE_CHAIN (t);
-      TREE_CHAIN (t) = NULL_TREE;
+      if (reverse)
+	to_stream[i++] = t;
+      else
+	pph_out_chained_tree (stream, t, ref_p);
+    }
 
-      pph_out_tree_or_ref_1 (stream, t, ref_p, 2);
+  if (reverse && count > 0)
+    {
+      for (i = count - 1; i >=0; i--)
+	pph_out_chained_tree (stream, to_stream[i], ref_p);
 
-      TREE_CHAIN (t) = saved_chain;
+      free (to_stream);
     }
 }
 
@@ -648,13 +684,16 @@ pph_out_binding_level (pph_stream *stream, struct
cp_binding_level *bl,
   if (!pph_out_start_record (stream, bl))
     return;
 
-  pph_out_chain_filtered (stream, bl->names, ref_p, NO_BUILTINS);
-  pph_out_chain_filtered (stream, bl->namespaces, ref_p, NO_BUILTINS);
+  pph_out_chain_filtered (stream, bl->names, ref_p, NO_BUILTINS, true);
+  pph_out_chain_filtered (stream, bl->namespaces, ref_p, NO_BUILTINS, true);
 
   pph_out_tree_vec (stream, bl->static_decls, ref_p);
 
-  pph_out_chain_filtered (stream, bl->usings, ref_p, NO_BUILTINS);
-  pph_out_chain_filtered (stream, bl->using_directives, ref_p, NO_BUILTINS);
+  /* FIXME pph: we probably want to reverse those as well, but to avoid
changing
+     the behaviour for this patch I set it to false.  */
+  pph_out_chain_filtered (stream, bl->usings, ref_p, NO_BUILTINS, false);
+  pph_out_chain_filtered (stream, bl->using_directives, ref_p, NO_BUILTINS,
+      false);
 
   pph_out_uint (stream, VEC_length (cp_class_binding, bl->class_shadowed));
   for (i = 0; VEC_iterate (cp_class_binding, bl->class_shadowed, i, cs); i++)
diff --git a/gcc/testsuite/g++.dg/pph/c1pr36533.cc
b/gcc/testsuite/g++.dg/pph/c1pr36533.cc
index d8d6d8c..b44e8c9 100644
--- a/gcc/testsuite/g++.dg/pph/c1pr36533.cc
+++ b/gcc/testsuite/g++.dg/pph/c1pr36533.cc
@@ -1,3 +1,2 @@
 /* { dg-options "-w -fpermissive" } */
-// pph asm xdiff
 #include "c1pr36533.h"
diff --git a/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc
b/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc
index d2ebd27..446e2f7 100644
--- a/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc
+++ b/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc
@@ -1,3 +1,3 @@
 // { dg-xfail-if "INFINITE" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "internal compiler error: in lto_streamer_cache_get, at
lto-streamer.c" "" { xfail *-*-* } 0 }
+// { dg-bogus "internal compiler error: in lto_get_pickled_tree, at
lto-streamer-in.c" "" { xfail *-*-* } 0 }
 #include "c1pr44948-1a.h"
diff --git a/gcc/testsuite/g++.dg/pph/x1functions.cc
b/gcc/testsuite/g++.dg/pph/x1functions.cc
index 78df01b..d4e2cf7 100644
--- a/gcc/testsuite/g++.dg/pph/x1functions.cc
+++ b/gcc/testsuite/g++.dg/pph/x1functions.cc
@@ -1,5 +1,3 @@
-// pph asm xdiff
-
 #include "x1functions.h"
 
 int type::mbr_decl_then_def(int i)      // need body

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