Patchwork [pph] Fix x1dynarra1, x1dynarray2a and x1dynarray2b (issue4921051)

login
register
mail settings
Submitter Diego Novillo
Date Aug. 22, 2011, 3:22 p.m.
Message ID <20110822152200.59EA41DA1CF@topo.tor.corp.google.com>
Download mbox | patch
Permalink /patch/110936/
State New
Headers show

Comments

Diego Novillo - Aug. 22, 2011, 3:22 p.m.
This patch fixes some template test cases.  We were trying to 
expand functions that did not really need expanding (templates).  This
got me thinking that we are not approaching the expansion properly.

We are trying to re-create all the cgraph creation done during the
compilation of the header file.  Rather than re-creating all this, it
would be more efficient to save the state of the callgraph and varpool
at the time of pph generation and then simply recreate it at read
time.  I will experiment with that, see if it actually makes any
sense.

Tested on x86_64.  Applied to pph.

	* pph-streamer-out.c (pph_out_symtab): Mark the function as
 	having a body if it has a cgraph node associated with it.
 	* pph-streamer-in.c (pph_in_binding_level): Call pph_in_tree to
 	read BL->BLOCKS.
 	* pph-streamer-out.c (pph_out_binding_level_1): Call pph_out_tree
 	to write BL->BLOCKS.

testsuite/ChangeLog.pph
2011-08-18   Diego Novillo  <dnovillo@google.com>

	* g++.dg/pph/x1dynarray1.cc: Mark fixed.
	* g++.dg/pph/x1dynarray2a.cc: Mark fixed.
	* g++.dg/pph/x1dynarray2b.cc: Mark fixed.
Gab Charette - Aug. 22, 2011, 4:13 p.m.
On Mon, Aug 22, 2011 at 8:22 AM, Diego Novillo <dnovillo@google.com> wrote:
>
> This patch fixes some template test cases.  We were trying to
> expand functions that did not really need expanding (templates).  This
> got me thinking that we are not approaching the expansion properly.
>
> We are trying to re-create all the cgraph creation done during the
> compilation of the header file.  Rather than re-creating all this, it
> would be more efficient to save the state of the callgraph and varpool
> at the time of pph generation and then simply recreate it at read
> time.  I will experiment with that, see if it actually makes any
> sense.
>

I had this idea earlier when playing with varpool for functions not
being streamed with their "rest_of_decl"compilation" state. The
problem for doing this if I remember correctly is that calling the
functions which generate those creates some unique ordering and IDs,
so that we need to do it in order of the current include order of each
pph in the current TU, and since the order of each pph in a given
compilation is not set ahead of time, I'm not sure this is possible
(at least not simple). We could probably find a way to merge those
states on the way in, but that would most likely tie us to the
implementation of those calls..

Gab

Patch

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 9686edf..ae3ede9 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -557,7 +557,7 @@  pph_in_binding_level (pph_stream *stream, cp_binding_level *to_register)
       VEC_safe_push (cp_label_binding, gc, bl->shadowed_labels, sl);
     }
 
-  bl->blocks = pph_in_chain (stream);
+  bl->blocks = pph_in_tree (stream);
   bl->this_entity = pph_in_tree (stream);
   bl->level_chain = pph_in_binding_level (stream, NULL);
   bl->dead_vars_from_for = pph_in_tree_vec (stream);
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index b563891..fe0758f 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -574,7 +574,7 @@  pph_out_binding_level_1 (pph_stream *stream, cp_binding_level *bl,
   FOR_EACH_VEC_ELT (cp_label_binding, bl->shadowed_labels, i, sl)
     pph_out_label_binding (stream, sl);
 
-  pph_out_chain (stream, bl->blocks);
+  pph_out_tree (stream, bl->blocks);
   pph_out_tree (stream, bl->this_entity);
   pph_out_binding_level (stream, bl->level_chain, filter);
   pph_out_tree_vec (stream, bl->dead_vars_from_for);
@@ -1128,7 +1128,9 @@  pph_out_symtab (pph_stream *stream)
   FOR_EACH_VEC_ELT (tree, stream->symtab.v, i, decl)
     if (TREE_CODE (decl) == FUNCTION_DECL && DECL_STRUCT_FUNCTION (decl))
       {
-	if (DECL_SAVED_TREE (decl))
+	/* If this is a regular (non-template) function with a body,
+	   mark it for expansion during reading.  */
+	if (DECL_SAVED_TREE (decl) && cgraph_get_node (decl))
 	  pph_out_symtab_marker (stream, PPH_SYMTAB_FUNCTION_BODY);
 	else
 	  pph_out_symtab_marker (stream, PPH_SYMTAB_FUNCTION);
diff --git a/gcc/testsuite/g++.dg/pph/x1dynarray1.cc b/gcc/testsuite/g++.dg/pph/x1dynarray1.cc
index 2a1f9f6..6ef279d 100644
--- a/gcc/testsuite/g++.dg/pph/x1dynarray1.cc
+++ b/gcc/testsuite/g++.dg/pph/x1dynarray1.cc
@@ -1,5 +1,4 @@ 
-// { dg-do link }
-// { dg-xfail-if "BOGUS" { "*-*-*" } { "-fpph-map=pph.map" } }
+// { dg-do run }
 
 #include "x0dynarray1.h"
 
diff --git a/gcc/testsuite/g++.dg/pph/x1dynarray2a.cc b/gcc/testsuite/g++.dg/pph/x1dynarray2a.cc
index f941086..252e801 100644
--- a/gcc/testsuite/g++.dg/pph/x1dynarray2a.cc
+++ b/gcc/testsuite/g++.dg/pph/x1dynarray2a.cc
@@ -1,5 +1,4 @@ 
-// { dg-xfail-if "BOGUS" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-do link }
+// { dg-do run }
 
 #include "x0dynarray2.h"
 
diff --git a/gcc/testsuite/g++.dg/pph/x1dynarray2b.cc b/gcc/testsuite/g++.dg/pph/x1dynarray2b.cc
index 8f14149..c942d9d 100644
--- a/gcc/testsuite/g++.dg/pph/x1dynarray2b.cc
+++ b/gcc/testsuite/g++.dg/pph/x1dynarray2b.cc
@@ -1,5 +1,4 @@ 
-// { dg-xfail-if "BOGUS" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-do link }
+// { dg-do run }
 
 #include "x0dynarray2.h"