Patchwork [pph] Fix global variable assembly ordering (issue4627087)

login
register
mail settings
Submitter Gab Charette
Date July 2, 2011, 1:35 a.m.
Message ID <20110702013502.8FD271C1CDF@gchare.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/102977/
State New
Headers show

Comments

Gab Charette - July 2, 2011, 1:35 a.m.
As variables are discovered (while parsing the header) they are added to the varpool and their RTL is built.

We do not stream, nor the varpool, nor the RTL (and I don't think we want to + that wouldn't work with multiple pph).

We want to rebuild the varpool when streaming the global variables of the pph in so as to redefine them in the varpool in the same order they would have been found in a regular #include style parse.

I'm not sure whether "global variables, not externals" is specific enough or too broad (I can't reuse the caller of varpool_finalize_decl (rest_of_decl_compilation) to take care of this logic because it needs some parser state which we no longer have). I will create more tests next week with different orderings for functions, structs, etc. coming in from the pph.

For now, this fixes 8 tests :).

Tested with bootstrap and pph regression testing.

PS: Just realized I didn't put a comment in the code for that fix, should I add one?

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

	* gcc/cp/pph-streamer-in.c (pph_add_bindings_to_namespace):
	Rebuild the varpool for global variables coming in from the pph.
	* gcc/testsuite/g++.dg/pph/c120060625-1.cc: Expect no asm difference.
	* gcc/testsuite/g++.dg/pph/c1struct.cc: Likewise.
	* gcc/testsuite/g++.dg/pph/c1variables.cc: Likewise.
	* gcc/testsuite/g++.dg/pph/c1varorder.cc: Likewise.
	* gcc/testsuite/g++.dg/pph/c2builtin1.cc: Likewise.
	* gcc/testsuite/g++.dg/pph/c2builtin2.cc: Likewise.
	* gcc/testsuite/g++.dg/pph/x1struct2.cc: Likewise.
	* gcc/testsuite/g++.dg/pph/x1variables.cc: Likewise.


--
This patch is available for review at http://codereview.appspot.com/4627087
Diego Novillo - July 4, 2011, 4:49 p.m.
On Fri, Jul 1, 2011 at 21:35, Gabriel Charette <gchare@google.com> wrote:
> As variables are discovered (while parsing the header) they are added to the varpool and their RTL is built.
>
> We do not stream, nor the varpool, nor the RTL (and I don't think we want to + that wouldn't
> work with multiple pph).

Right.  Additionally, saving RTL makes the PPH target-dependent.  We
don't want that.

>
> We want to rebuild the varpool when streaming the global variables of the pph in so as to
> redefine them in the varpool in the same order they would have been found in a regular
> #include style parse.

Right.

> I'm not sure whether "global variables, not externals" is specific enough or too broad (I can't reuse the caller
> of varpool_finalize_decl (rest_of_decl_compilation) to take care of this logic because it needs some parser
> state which we no longer have). I will create more tests next week with different orderings for functions,
> structs, etc. coming in from the pph.

Hm, I think we actually want to call rest_of_decl_compilation here.
This is also used from the LTO front end when reconstructing
variables.  Your patch is in the right direction, though, so I've
applied it for now.


Diego.

Patch

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 3ac5243..0ddcc75 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -1157,6 +1157,9 @@  pph_add_bindings_to_namespace (struct cp_binding_level *bl, tree ns)
 	 Preserve it.  */
       chain = DECL_CHAIN (t);
       pushdecl_into_namespace (t, ns);
+
+      if (TREE_CODE (t) == VAR_DECL && TREE_STATIC (t) && !DECL_EXTERNAL (t))
+	varpool_finalize_decl (t);
     }
 
   for (t = bl->namespaces; t; t = chain)
diff --git a/gcc/testsuite/g++.dg/pph/c120060625-1.cc b/gcc/testsuite/g++.dg/pph/c120060625-1.cc
index d09be39..05c7929 100644
--- a/gcc/testsuite/g++.dg/pph/c120060625-1.cc
+++ b/gcc/testsuite/g++.dg/pph/c120060625-1.cc
@@ -1,2 +1 @@ 
-// pph asm xdiff
 #include "c120060625-1.h"
diff --git a/gcc/testsuite/g++.dg/pph/c1struct.cc b/gcc/testsuite/g++.dg/pph/c1struct.cc
index 88c215b..8de9166 100644
--- a/gcc/testsuite/g++.dg/pph/c1struct.cc
+++ b/gcc/testsuite/g++.dg/pph/c1struct.cc
@@ -1,5 +1,3 @@ 
-// pph asm xdiff
-
 #include "c1struct.h"
 
 thing var1;
diff --git a/gcc/testsuite/g++.dg/pph/c1variables.cc b/gcc/testsuite/g++.dg/pph/c1variables.cc
index 12a5f30..4d3c5a7 100644
--- a/gcc/testsuite/g++.dg/pph/c1variables.cc
+++ b/gcc/testsuite/g++.dg/pph/c1variables.cc
@@ -1,5 +1,3 @@ 
-// pph asm xdiff
-
 #include "c1variables.h"
 
 int gbl_initial = 1;
diff --git a/gcc/testsuite/g++.dg/pph/c1varorder.cc b/gcc/testsuite/g++.dg/pph/c1varorder.cc
index a7a65ec..2791427 100644
--- a/gcc/testsuite/g++.dg/pph/c1varorder.cc
+++ b/gcc/testsuite/g++.dg/pph/c1varorder.cc
@@ -1,5 +1,3 @@ 
-// pph asm xdiff
-
 #include "c1varorder.h"
 
 int foo(void)
diff --git a/gcc/testsuite/g++.dg/pph/c2builtin1.cc b/gcc/testsuite/g++.dg/pph/c2builtin1.cc
index 9a3a7a1..67327cb 100644
--- a/gcc/testsuite/g++.dg/pph/c2builtin1.cc
+++ b/gcc/testsuite/g++.dg/pph/c2builtin1.cc
@@ -1,5 +1,3 @@ 
-// pph asm xdiff
-
 #include "c2builtin1.h"
 #include "c2builtin2.h"
 #include "c2builtin3.h"
diff --git a/gcc/testsuite/g++.dg/pph/c2builtin2.cc b/gcc/testsuite/g++.dg/pph/c2builtin2.cc
index 186ebcf..0ccae57 100644
--- a/gcc/testsuite/g++.dg/pph/c2builtin2.cc
+++ b/gcc/testsuite/g++.dg/pph/c2builtin2.cc
@@ -1,4 +1,3 @@ 
-// pph asm xdiff
 #include "a2builtin4.h"
 #include "c2builtin5.h"
 #include "c2builtin6.h"
diff --git a/gcc/testsuite/g++.dg/pph/x1struct2.cc b/gcc/testsuite/g++.dg/pph/x1struct2.cc
index be1af39..f31fb8c 100644
--- a/gcc/testsuite/g++.dg/pph/x1struct2.cc
+++ b/gcc/testsuite/g++.dg/pph/x1struct2.cc
@@ -1,5 +1,3 @@ 
-// pph asm xdiff
-
 #include "x1struct2.h"
 
 type D::method()
diff --git a/gcc/testsuite/g++.dg/pph/x1variables.cc b/gcc/testsuite/g++.dg/pph/x1variables.cc
index 0f0814f..f9fd76e 100644
--- a/gcc/testsuite/g++.dg/pph/x1variables.cc
+++ b/gcc/testsuite/g++.dg/pph/x1variables.cc
@@ -1,5 +1,3 @@ 
-// pph asm xdiff
-
 #include "x1variables.h"
 
 int D::mbr_init_plain = 4;