diff mbox

[pph] Stream chain of struct fields (issue4631072)

Message ID 20110624231320.D05F61C0DAE@gchare.mtv.corp.google.com
State New
Headers show

Commit Message

Gab Charette June 24, 2011, 11:13 p.m. UTC 
We were only streaming the first field of every struct. Struct fields have a chain link to the next field, thus we need to stream the DECL_CHAIN of every field as well recursively.

I limited this to VAR_DECL and FUNCTION_DECL for now (which fixes all of our current bugs in regards to the struct fields issues), are there any other DECLs that can potentially be fields of a struct?

I was tempted to stream the DECL_CHAIN for the whole "group" VAR_DECL belongs to in pph_read_tree, but as it turns out we don't want to do this for NAMESPACE_DECL as it gets a new chain link when re-added to the bindings early on and changing it here causes an ICE in one of the test cases.
Hence, I reverted to the conservative way of only streaming it for what we know we need it for.

Syntax-wise: Is it ok to play this 'case' fall through trick with VAR_DECL or should I make a separate case entry with it's own break?

2011-06-24  Gabriel Charette  <gchare@google.com>

	* gcc/cp/pph-streamer-in.c (pph_read_tree):
	Stream in DECL_CHAIN of VAR_DECL.
	Stream in DECL_CHAIN of FUNCTION_DECL.
	* gcc/cp/pph-streamer-out.c (pph_write_tree):
	Stream out DECL_CHAIN of VAR_DECL.
	Stream out DECL_CHAIN of FUNCTION_DECL.
	* gcc/testsuite/g++.dg/pph/x1functions.cc: Fixed bogus, now asm diff.
	* gcc/testsuite/g++.dg/pph/x1variables.cc: Fixed bogus, now asm diff.


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

Comments

Diego Novillo June 27, 2011, 2:37 p.m. UTC | #1 
On Fri, Jun 24, 2011 at 19:13, Gabriel Charette <gchare@google.com> wrote:
> We were only streaming the first field of every struct. Struct fields have a chain link to the next field, thus we need to stream the DECL_CHAIN of every field as well recursively.
>
> I limited this to VAR_DECL and FUNCTION_DECL for now (which fixes all of our current bugs in regards to the struct fields issues), are there any other DECLs that can potentially be fields of a struct?

FIELD_DECLs are the natural members of structs.  But those are handled
by generic streaming.  Also needed for PARM_DECLs (already handled as
well).  VAR_DECLs and FUNCTION_DECLs were the only ones missing.

> Syntax-wise: Is it ok to play this 'case' fall through trick with VAR_DECL or should I make a
> separate case entry with it's own break?

Cleaner to just distinguish them inside the main group.  I've tweaked
it and committed to the branch.

We are still missing some link attributes.  The code compiles but does
not link.  I will look into that today.


Diego.
diff mbox

Patch

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 0e6763f..8d12839 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -1216,12 +1216,13 @@  pph_read_tree (struct lto_input_block *ib ATTRIBUTE_UNUSED,
       DECL_INITIAL (expr) = pph_in_tree (stream);
       break;
 
+    case VAR_DECL:
+      DECL_CHAIN (expr) = pph_in_tree (stream);
     case CONST_DECL:
     case FIELD_DECL:
     case NAMESPACE_DECL:
     case PARM_DECL:
     case USING_DECL:
-    case VAR_DECL:
       /* FIXME pph: Should we merge DECL_INITIAL into lang_specific? */
       DECL_INITIAL (expr) = pph_in_tree (stream);
       pph_in_lang_specific (stream, expr);
@@ -1232,6 +1233,7 @@  pph_read_tree (struct lto_input_block *ib ATTRIBUTE_UNUSED,
       pph_in_lang_specific (stream, expr);
       DECL_SAVED_TREE (expr) = pph_in_tree (stream);
       DECL_STRUCT_FUNCTION (expr) = pph_in_struct_function (stream);
+      DECL_CHAIN (expr) = pph_in_tree (stream);
       break;
 
     case TYPE_DECL:
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index 613cdcd..e85a629 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -1062,12 +1062,13 @@  pph_write_tree (struct output_block *ob, tree expr, bool ref_p)
       pph_out_tree_or_ref_1 (stream, DECL_INITIAL (expr), ref_p, 3);
       break;
 
+    case VAR_DECL:
+      pph_out_tree_or_ref_1 (stream, DECL_CHAIN (expr), ref_p, 3);
     case CONST_DECL:
     case FIELD_DECL:
     case NAMESPACE_DECL:
     case PARM_DECL:
     case USING_DECL:
-    case VAR_DECL:
       /* FIXME pph: Should we merge DECL_INITIAL into lang_specific? */
       pph_out_tree_or_ref_1 (stream, DECL_INITIAL (expr), ref_p, 3);
       pph_out_lang_specific (stream, expr, ref_p);
@@ -1078,6 +1079,7 @@  pph_write_tree (struct output_block *ob, tree expr, bool ref_p)
       pph_out_lang_specific (stream, expr, ref_p);
       pph_out_tree_or_ref_1 (stream, DECL_SAVED_TREE (expr), ref_p, 3);
       pph_out_struct_function (stream, DECL_STRUCT_FUNCTION (expr), ref_p);
+      pph_out_tree_or_ref_1 (stream, DECL_CHAIN (expr), ref_p, 3);
       break;
 
     case TYPE_DECL:
diff --git a/gcc/testsuite/g++.dg/pph/x1functions.cc b/gcc/testsuite/g++.dg/pph/x1functions.cc
index 20cde5c..78df01b 100644
--- a/gcc/testsuite/g++.dg/pph/x1functions.cc
+++ b/gcc/testsuite/g++.dg/pph/x1functions.cc
@@ -1,5 +1,4 @@ 
-// { dg-xfail-if "BOGUS" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "'mbr_decl_inline' was not declared in this scope" "" { xfail *-*-* } 0 }
+// pph asm xdiff
 
 #include "x1functions.h"
 
diff --git a/gcc/testsuite/g++.dg/pph/x1variables.cc b/gcc/testsuite/g++.dg/pph/x1variables.cc
index bac3136..0f0814f 100644
--- a/gcc/testsuite/g++.dg/pph/x1variables.cc
+++ b/gcc/testsuite/g++.dg/pph/x1variables.cc
@@ -1,6 +1,4 @@ 
-// { dg-xfail-if "BOGUS" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "c1variables.h:5:8: error: 'int D::mbr_uninit_plain' is not a static member of 'struct D'" "" { xfail *-*-* } 0 }
-// { dg-bogus "c1variables.h:6:14: error: 'const int D::mbr_init_const' is not a static member of 'struct D'" "" { xfail *-*-* } 0 }
+// pph asm xdiff
 
 #include "x1variables.h"