public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [pph] Fix global variable assembly ordering (issue4627087)
@ 2011-07-02  1:35 Gabriel Charette
  2011-07-04 16:49 ` Diego Novillo
  0 siblings, 1 reply; 2+ messages in thread
From: Gabriel Charette @ 2011-07-02  1:35 UTC (permalink / raw)
  To: reply, crowl, dnovillo, gcc-patches

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.

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;

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [pph] Fix global variable assembly ordering (issue4627087)
  2011-07-02  1:35 [pph] Fix global variable assembly ordering (issue4627087) Gabriel Charette
@ 2011-07-04 16:49 ` Diego Novillo
  0 siblings, 0 replies; 2+ messages in thread
From: Diego Novillo @ 2011-07-04 16:49 UTC (permalink / raw)
  To: Gabriel Charette; +Cc: reply, crowl, gcc-patches

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.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-07-04 16:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-02  1:35 [pph] Fix global variable assembly ordering (issue4627087) Gabriel Charette
2011-07-04 16:49 ` Diego Novillo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).