[pph] Stream chain of struct fields (issue4631072)

[pph] Stream chain of struct fields (issue4631072)

[Gabriel Charette]

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.

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"
 

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

Re: [pph] Stream chain of struct fields (issue4631072)

[Diego Novillo]

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.

Re: [pph] Stream chain of struct fields (issue4631072)

[gchare]

Fixes two pph BOGUS bugs which now result in an asm diff.

Tested with bootstrap build and pph regression testing.

http://codereview.appspot.com/4631072/