[pph] Stream and restore static_aggregates

[Diego Novillo, 2011-07-06 04:08:25]

This patch is a partial fix for c1eabi1.cc.  We were missing static
initializers.  There is another source of assembly difference in that
file, so I still need to dig some more.

This fixes x2nontrivinit.cc, however.

I am still not happy with the way we add bindings and symbols to
namespaces.  We are missing several things, but I'm not quite sure
what yet.  We'll need to restructure pph_add_bindings_to_namespace.

Tested on x86_64.  Committed to branch.


Diego.

	* pph-streamer-in.c (pph_add_bindings_to_namespace): Recurse into
 	each namespace bindings, if needed.
 	(pph_read_file_contents): Add static aggregates from STREAM into
 	static_aggregates.
 	* pph-streamer-out.c (pph_write_file_contents): Write
 	static_aggregates.

testsuite/ChangeLog.pph

	* g++.dg/pph/x2nontrivinit.cc: Mark fixed.

diff --git a/gcc/cp/ChangeLog.pph b/gcc/cp/ChangeLog.pph
index 46b18be..a70ab0e 100644
--- a/gcc/cp/ChangeLog.pph
+++ b/gcc/cp/ChangeLog.pph
@@ -1,3 +1,12 @@
+2011-07-05   Diego Novillo  <dnovillo@google.com>
+
+	* pph-streamer-in.c (pph_add_bindings_to_namespace): Recurse into
+	each namespace bindings, if needed.
+	(pph_read_file_contents): Add static aggregates from STREAM into
+	static_aggregates.
+	* pph-streamer-out.c (pph_write_file_contents): Write
+	static_aggregates.
+
 2011-07-04  Gabriel Charette  <gchare@google.com>
 
 	* pph-streamer-in.c (pph_add_bindings_to_namespace):
diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 72536a5..0bab93b 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -1167,11 +1167,9 @@ pph_add_bindings_to_namespace (struct cp_binding_level
*bl, tree ns)
       /* Pushing a decl into a scope clobbers its DECL_CHAIN.
 	 Preserve it.  */
       chain = DECL_CHAIN (t);
-
-      /* FIXME pph: we should first check to see if it isn't already there.
-	 If it is, we should use this function recursively to merge
-	 the bindings in T in the corresponding namespace.  */
       pushdecl_into_namespace (t, ns);
+      if (NAMESPACE_LEVEL (t))
+	pph_add_bindings_to_namespace (NAMESPACE_LEVEL (t), t);
     }
 }
 
@@ -1314,7 +1312,7 @@ pph_read_file_contents (pph_stream *stream)
   cpp_ident_use *bad_use;
   const char *cur_def;
   cpp_idents_used idents_used;
-  tree fndecl, t, file_keyed_classes;
+  tree fndecl, t, file_keyed_classes, file_static_aggregates;
   unsigned i;
   VEC(tree,gc) *file_unemitted_tinfo_decls;
 
@@ -1342,6 +1340,9 @@ pph_read_file_contents (pph_stream *stream)
   for (i = 0; VEC_iterate (tree, file_unemitted_tinfo_decls, i, t); i++)
     VEC_safe_push (tree, gc, unemitted_tinfo_decls, t);
 
+  file_static_aggregates = pph_in_tree (stream);
+  static_aggregates = chainon (file_static_aggregates, static_aggregates);
+
   /* Expand all the functions with bodies that we read from STREAM.  */
   for (i = 0; VEC_iterate (tree, stream->fns_to_expand, i, fndecl); i++)
     {
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index acc0352..abe05a2 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -1202,6 +1202,7 @@ pph_write_file_contents (pph_stream *stream,
cpp_idents_used *idents_used)
     pph_dump_namespace (pph_logfile, global_namespace);
   pph_out_tree (stream, keyed_classes, false);
   pph_out_tree_vec (stream, unemitted_tinfo_decls, false);
+  pph_out_tree (stream, static_aggregates, false);
 }
 
 
diff --git a/gcc/testsuite/ChangeLog.pph b/gcc/testsuite/ChangeLog.pph
index 530adb5..da45f01 100644
--- a/gcc/testsuite/ChangeLog.pph
+++ b/gcc/testsuite/ChangeLog.pph
@@ -1,3 +1,7 @@
+2011-07-05  Diego Novillo  <dnovillo@google.com>
+
+	* g++.dg/pph/x2nontrivinit.cc: Mark fixed.
+
 2011-07-04   Diego Novillo  <dnovillo@google.com>
 
 	* g++.dg/pph/pph.map: Sort.
diff --git a/gcc/testsuite/g++.dg/pph/x2nontrivinit.cc
b/gcc/testsuite/g++.dg/pph/x2nontrivinit.cc
index b142ac2..96ecce2 100644
--- a/gcc/testsuite/g++.dg/pph/x2nontrivinit.cc
+++ b/gcc/testsuite/g++.dg/pph/x2nontrivinit.cc
@@ -1,3 +1 @@
-// pph asm xdiff
-
 #include "x2nontrivinit.h"
--
This patch is available for review at http://codereview.appspot.com/4626096

[Gabriel Charette, 2011-07-06 17:12:16]

On Tue, Jul 5, 2011 at 9:08 PM, Diego Novillo <dnovillo@google.com> wrote:

>        * pph-streamer-in.c (pph_add_bindings_to_namespace):
> diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
> index 72536a5..0bab93b 100644
> --- a/gcc/cp/pph-streamer-in.c
> +++ b/gcc/cp/pph-streamer-in.c
> @@ -1167,11 +1167,9 @@ pph_add_bindings_to_namespace (struct cp_binding_level
*bl, tree ns)
>       /* Pushing a decl into a scope clobbers its DECL_CHAIN.
>         Preserve it.  */
>       chain = DECL_CHAIN (t);
> -
> -      /* FIXME pph: we should first check to see if it isn't already there.
> -        If it is, we should use this function recursively to merge
> -        the bindings in T in the corresponding namespace.  */
>       pushdecl_into_namespace (t, ns);
> +      if (NAMESPACE_LEVEL (t))
> +       pph_add_bindings_to_namespace (NAMESPACE_LEVEL (t), t);
>     }
>  }

I had removed these two lines because that pretty much does nothing (I
think, at least removing them didn't make anything fail...)... When we
stream out the other namespaces (through the tree which's root is
scope_chain->bindings), we also stream out these namespaces' bindings.
This is all rebuilt when streaming in, so calling
pph_add_bindings_to_namespace (NAMESPACE_LEVEL (t), t);
seems like it's trying to add to namespace t all the bindings that are
already in it... (unless pushdecl_into_namespace does more magic then
adding the bindings to the namespace?)

To me this only made sense if we had found a corresponding namespace
we wanted to merge the streamed in bindings into...

Otherwise I agree we will need to modify
pph_add_bindings_to_namespace. It is still missing the merge of
"usings" and "using_directives". "static_decls" if I remember
correctly were automatically reinserted when doing
pushdecl_into_namespace for the static decls in "names".

Gab

[Diego Novillo, 2011-07-06 18:27:36]

On 11-07-06 13:12 , Gabriel Charette wrote:

> I had removed these two lines because that pretty much does nothing (I
> think, at least removing them didn't make anything fail...)... When we

It started causing an ICE in the test case that I marked fixed and 
others (because we now stream static_aggregates).

> Otherwise I agree we will need to modify
> pph_add_bindings_to_namespace. It is still missing the merge of
> "usings" and "using_directives". "static_decls" if I remember
> correctly were automatically reinserted when doing
> pushdecl_into_namespace for the static decls in "names".

Yeah.  Jason suggests merging scope_chain->bindings read from the PPH 
image into scope_chain->bindings from the translation unit.  I've 
started down that road.  More later.


Diego.