| Message ID | 20110709012047.0D90B1C36BA@gchare.mtv.corp.google.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jul 8, 2011 at 21:20, Gabriel Charette <gchare@google.com> wrote: > 2011-07-08 Gabriel Charette <gchare@google.com> > > * pph-streamer-in.c (pph_in_function_decl): Stream in > DECL_CHAIN of FUNCTION_DECL only if it's part of a RECORD_OR_UNION_TYPE > (pph_read_tree): Stream in DECL_CHAIN of VAR_DECL only if it's part > of a RECORD_OR_UNION_TYPE. > * pph-streamer-out.c (pph_out_function_decl): Stream out > DECL_CHAIN of FUNCTION_DECL only if it's part of a RECORD_OR_UNION_TYPE > (pph_write_tree): Stream out DECL_CHAIN of VAR_DECL only if it's part > of a RECORD_OR_UNION_TYPE. Gab, do you still need this patch? In principle, it doesn't make a lot of sense to restrict when we save the DECL_CHAIN in this way. It's not obvious what this would fix or help with. Diego.
The reason I put that patch out is that sometimes, when we stream an actual chain, lto_input_chain is going to rebuild the new chain it's meant to be, but then pph_read_tree (which is called after by the name_hook to finish reading special parts of the tree) overwrites the DECL_CHAIN that was introduced by lto_input_chain. The only time we need to actually stream in/out the DECL_CHAIN is when streaming unions/structs because from what I looked at in lto it looks like we are not doing lto_output_chain, but lto_output_tree on the first member of the fields' chain (not sure how that even works in lto... but in pph we used to only get the first member of structs streamed and streaming DECL_CHAIN was the fix for it...) I introduced this fix because it broke my patch trying to stream out the chains backwards (as it would overwrite the chain I was trying to create backwards on input, I think this didn't show up before because the chain being built on input was the same as the one existing on output (thus overwriting with the same value...) ) Even if this doesn't break tests anymore, we probably still want this, no point adding stuff to the pph image that is not needed... Any idea why lto doesn't call lto_output_chain, but simply lto_output_tree to output the chains for struct/union? Gab On Tue, Jul 12, 2011 at 12:21 PM, Diego Novillo <dnovillo@google.com> wrote: > On Fri, Jul 8, 2011 at 21:20, Gabriel Charette <gchare@google.com> wrote: > >> 2011-07-08 Gabriel Charette <gchare@google.com> >> >> * pph-streamer-in.c (pph_in_function_decl): Stream in >> DECL_CHAIN of FUNCTION_DECL only if it's part of a RECORD_OR_UNION_TYPE >> (pph_read_tree): Stream in DECL_CHAIN of VAR_DECL only if it's part >> of a RECORD_OR_UNION_TYPE. >> * pph-streamer-out.c (pph_out_function_decl): Stream out >> DECL_CHAIN of FUNCTION_DECL only if it's part of a RECORD_OR_UNION_TYPE >> (pph_write_tree): Stream out DECL_CHAIN of VAR_DECL only if it's part >> of a RECORD_OR_UNION_TYPE. > > Gab, do you still need this patch? In principle, it doesn't make a > lot of sense to restrict when we save the DECL_CHAIN in this way. > It's not obvious what this would fix or help with. > > > Diego. >
On 11-07-12 16:43 , Gabriel Charette wrote: > Even if this doesn't break tests anymore, we probably still want this, > no point adding stuff to the pph image that is not needed... Actually, the reverse is true. We want to write out the IL exactly as the original parser emitted it. There are things we decide not to write because they are better re-generated when the pph image is being read (e.g., function numbers, DECL_RTL), but > Any idea why lto doesn't call lto_output_chain, but simply > lto_output_tree to output the chains for struct/union? LTO did not need those chains because once in the middle-end they are not used. We are working at the parser level, so we need them. Perhaps we won't need to write these chains, but first I'd like to understand why. Since we are streaming the chains backwards without new breakage, let's leave it out for now. Diego.
On Tue, Jul 12, 2011 at 3:32 PM, Diego Novillo <dnovillo@google.com> wrote: > On 11-07-12 16:43 , Gabriel Charette wrote: > >> Even if this doesn't break tests anymore, we probably still want this, >> no point adding stuff to the pph image that is not needed... > > Actually, the reverse is true. We want to write out the IL exactly as the > original parser emitted it. There are things we decide not to write because > they are better re-generated when the pph image is being read (e.g., > function numbers, DECL_RTL), but > Well so lto_input_chain, called from pph_in_chain for every single chain, already reconstructs the DECL_CHAIN on input (DECL_CHAIN is actually set to NULL before streaming it out each element in the chain anyways). The only case where that wasn't true was when we output structs (and unions I think: we need to add a test for unions), because structs don't seem to use output chain (I don't have the code in front of me, but I know they would only output the tree (i.e. the tree was, in that case, responsible for streaming its DECL_CHAIN)). I'm surprised it no longer breaks... I'll have a look when I come back on Friday if it's still a debated issue then. Gab
diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c index 0bab93b..d78ee91 100644 --- a/gcc/cp/pph-streamer-in.c +++ b/gcc/cp/pph-streamer-in.c @@ -1396,7 +1396,8 @@ pph_in_function_decl (pph_stream *stream, tree fndecl) pph_in_lang_specific (stream, fndecl); DECL_SAVED_TREE (fndecl) = pph_in_tree (stream); DECL_STRUCT_FUNCTION (fndecl) = pph_in_struct_function (stream, fndecl); - DECL_CHAIN (fndecl) = pph_in_tree (stream); + if (DECL_CONTEXT (fndecl) && RECORD_OR_UNION_TYPE_P (DECL_CONTEXT (fndecl))) + DECL_CHAIN (fndecl) = pph_in_tree (stream); if (DECL_SAVED_TREE (fndecl)) VEC_safe_push (tree, gc, stream->fns_to_expand, fndecl); } @@ -1436,7 +1437,9 @@ pph_read_tree (struct lto_input_block *ib ATTRIBUTE_UNUSED, DECL_INITIAL (expr) = pph_in_tree (stream); pph_in_lang_specific (stream, expr); /* DECL_CHAIN is handled by generic code, except for VAR_DECLs. */ - if (TREE_CODE (expr) == VAR_DECL) + if (TREE_CODE (expr) == VAR_DECL + && DECL_CONTEXT (expr) + && RECORD_OR_UNION_TYPE_P (DECL_CONTEXT (expr))) DECL_CHAIN (expr) = pph_in_tree (stream); break; diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c index 089bb13..d1e757f 100644 --- a/gcc/cp/pph-streamer-out.c +++ b/gcc/cp/pph-streamer-out.c @@ -1257,7 +1257,8 @@ pph_out_function_decl (pph_stream *stream, tree fndecl, bool ref_p) pph_out_lang_specific (stream, fndecl, ref_p); pph_out_tree_or_ref_1 (stream, DECL_SAVED_TREE (fndecl), ref_p, 3); pph_out_struct_function (stream, DECL_STRUCT_FUNCTION (fndecl), ref_p); - pph_out_tree_or_ref_1 (stream, DECL_CHAIN (fndecl), ref_p, 3); + if (DECL_CONTEXT (fndecl) && RECORD_OR_UNION_TYPE_P (DECL_CONTEXT (fndecl))) + pph_out_tree_or_ref_1 (stream, DECL_CHAIN (fndecl), ref_p, 3); } /* Callback for writing ASTs to a stream. This writes all the fields @@ -1292,7 +1293,9 @@ 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); pph_out_lang_specific (stream, expr, ref_p); /* DECL_CHAIN is handled by generic code, except for VAR_DECLs. */ - if (TREE_CODE (expr) == VAR_DECL) + if (TREE_CODE (expr) == VAR_DECL + && DECL_CONTEXT (expr) + && RECORD_OR_UNION_TYPE_P (DECL_CONTEXT (expr))) pph_out_tree_or_ref_1 (stream, DECL_CHAIN (expr), ref_p, 3); break;