From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31696 invoked by alias); 12 Jul 2011 20:35:17 -0000 Received: (qmail 31307 invoked by uid 22791); 12 Jul 2011 20:35:15 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.67) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 12 Jul 2011 20:34:44 +0000 Received: from kpbe13.cbf.corp.google.com (kpbe13.cbf.corp.google.com [172.25.105.77]) by smtp-out.google.com with ESMTP id p6CKYgOa029949 for ; Tue, 12 Jul 2011 13:34:43 -0700 Received: from yic24 (yic24.prod.google.com [10.243.65.152]) by kpbe13.cbf.corp.google.com with ESMTP id p6CKYI95029215 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Tue, 12 Jul 2011 13:34:41 -0700 Received: by yic24 with SMTP id 24so2989571yic.21 for ; Tue, 12 Jul 2011 13:34:41 -0700 (PDT) MIME-Version: 1.0 Received: by 10.100.236.21 with SMTP id j21mr373090anh.130.1310502881096; Tue, 12 Jul 2011 13:34:41 -0700 (PDT) Received: by 10.100.254.9 with HTTP; Tue, 12 Jul 2011 13:34:41 -0700 (PDT) In-Reply-To: <20110712191913.042251DA1C8@topo.tor.corp.google.com> References: <20110712191913.042251DA1C8@topo.tor.corp.google.com> Date: Tue, 12 Jul 2011 20:43:00 -0000 Message-ID: Subject: Re: [pph] Fix 3 asm differences (issue4695048) From: Gabriel Charette To: Diego Novillo Cc: reply@codereview.appspotmail.com, crowl@google.com, gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-07/txt/msg00960.txt.bz2 I like the modified implementation with VEC. We probably want pph_register_decl_in_symtab to be inline as it does so little now. Now that you simply chainon bindings, you probably want to nreverse them before you chain them on (this way we will stream them in from first->last as this pacth does (to alloc stuff in order), but them in the chain we want them to be last->first as they should be if they had been pushed as they are in the original parser). Gab On Tue, Jul 12, 2011 at 12:19 PM, Diego Novillo wrote: > > This patch is a slight adaptation of Gab's fix to the order in which > we stream chains (http://codereview.appspot.com/4657092). =A0I mostly > just changed how we keep the temporary list to reverse (it now uses a > VEC instead of a custom-build linked list). > > The other change is in the reader. =A0We were not registering symbols in > scope_chain->static_aggregates as they come from the PPH file (which > would cause an ICE in x1hardlookup.cc). > > This fixes 3 tests, but we still have some asm differences that are > similar in nature: when reinstantiating PPH images, the compiler emits > some symbols in different order, causing different numbering and > naming in the assembler output (we need to generate identical output > from a pph or from text). > > Tested on x86_64. =A0Committed to branch. > > > Diego. > > 2011-07-12 =A0 Diego Novillo =A0 > > =A0 =A0 =A0 =A0* pph-streamer-in.c (pph_register_decl_in_symtab): New. > =A0 =A0 =A0 =A0(pph_register_binding_in_symtab): Rename from > =A0 =A0 =A0 =A0pph_register_decls_in_symtab. =A0Update all users. > =A0 =A0 =A0 =A0Do not call nreverse on bl->names and bl->namespaces. > =A0 =A0 =A0 =A0Call pph_register_decl_in_symtab. > =A0 =A0 =A0 =A0(pph_read_file_contents): Register decls in > =A0 =A0 =A0 =A0FILE_STATIC_AGGREGATES. > > 2011-07-12 =A0Gabriel Charette =A0 > =A0 =A0 =A0 =A0 =A0 =A0Diego Novillo =A0 > > =A0 =A0 =A0 =A0* pph-streamer-out.c (pph_out_chained_tree): New. > =A0 =A0 =A0 =A0(pph_out_chain_filtered): Add REVERSE_P parameter. > =A0 =A0 =A0 =A0If REVERSE_P is set, write the list in reverse order. > =A0 =A0 =A0 =A0Update all users. > =A0 =A0 =A0 =A0(pph_out_binding_level): Write out lists bl->names, > =A0 =A0 =A0 =A0bl->namespaces, bl->usings and bl->using_directives in > =A0 =A0 =A0 =A0reverse. > > > testsuite/ChangeLog.pph > 2011-07-12 =A0Gabriel Charette =A0 > > =A0 =A0 =A0 =A0* g++.dg/pph/c1pr44948-1a.cc: Mark fixed. > =A0 =A0 =A0 =A0* g++.dg/pph/c2pr36533.cc: Mark fixed. > =A0 =A0 =A0 =A0* g++.dg/pph/x2functions.cc: Mark fixed. > > diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c > index 63f8965..e7d1d00 100644 > --- a/gcc/cp/pph-streamer-in.c > +++ b/gcc/cp/pph-streamer-in.c > @@ -1175,29 +1175,33 @@ pph_in_lang_type (pph_stream *stream) > =A0} > > > +/* Register DECL with the middle end. =A0*/ > + > +static void > +pph_register_decl_in_symtab (tree decl) > +{ > + =A0if (TREE_CODE (decl) =3D=3D VAR_DECL > + =A0 =A0 =A0&& TREE_STATIC (decl) > + =A0 =A0 =A0&& !DECL_EXTERNAL (decl)) > + =A0 =A0varpool_finalize_decl (decl); > +} > + > + > =A0/* Register all the symbols in binding level BL in the callgraph symbol > =A0 =A0table. =A0*/ > > =A0static void > -pph_register_decls_in_symtab (struct cp_binding_level *bl) > +pph_register_binding_in_symtab (struct cp_binding_level *bl) > =A0{ > =A0 tree t; > > - =A0/* The chains are built backwards (ref: add_decl_to_level), > - =A0 =A0 reverse them before putting them back in. =A0*/ > - =A0bl->names =3D nreverse (bl->names); > - =A0bl->namespaces =3D nreverse (bl->namespaces); > - > + =A0/* Add file-local symbols to the varpool. =A0*/ > =A0 for (t =3D bl->names; t; t =3D DECL_CHAIN (t)) > - =A0 =A0{ > - =A0 =A0 =A0/* Add file-local symbols to the varpool. =A0*/ > - =A0 =A0 =A0if (TREE_CODE (t) =3D=3D VAR_DECL && TREE_STATIC (t) && !DEC= L_EXTERNAL (t)) > - =A0 =A0 =A0 varpool_finalize_decl (t); > - =A0 =A0} > + =A0 =A0pph_register_decl_in_symtab (t); > > =A0 /* Recurse into the namespaces contained in BL. =A0*/ > =A0 for (t =3D bl->namespaces; t; t =3D DECL_CHAIN (t)) > - =A0 =A0pph_register_decls_in_symtab (NAMESPACE_LEVEL (t)); > + =A0 =A0pph_register_binding_in_symtab (NAMESPACE_LEVEL (t)); > =A0} > > > @@ -1220,7 +1224,7 @@ pph_in_scope_chain (pph_stream *stream) > =A0 new_bindings =3D pph_in_binding_level (stream, scope_chain->bindings); > > =A0 /* Register all the symbols in STREAM with the call graph. =A0*/ > - =A0pph_register_decls_in_symtab (new_bindings); > + =A0pph_register_binding_in_symtab (new_bindings); > > =A0 /* Merge the bindings from STREAM into saved_scope->bindings. =A0*/ > =A0 chainon (cur_bindings->names, new_bindings->names); > @@ -1413,6 +1417,16 @@ pph_read_file_contents (pph_stream *stream) > =A0 file_static_aggregates =3D pph_in_tree (stream); > =A0 static_aggregates =3D chainon (file_static_aggregates, static_aggrega= tes); > > + =A0/* Register all symbols in FILE_STATIC_AGGREGATES with the middle en= d. > + =A0 =A0 Each element of this list is an INIT_EXPR expression. =A0*/ > + =A0for (t =3D file_static_aggregates; t; t =3D TREE_CHAIN (t)) > + =A0 =A0{ > + =A0 =A0 =A0tree lhs =3D TREE_OPERAND (TREE_PURPOSE (t), 0); > + =A0 =A0 =A0tree rhs =3D TREE_OPERAND (TREE_PURPOSE (t), 1); > + =A0 =A0 =A0pph_register_decl_in_symtab (lhs); > + =A0 =A0 =A0pph_register_decl_in_symtab (rhs); > + =A0 =A0} > + > =A0 /* Expand all the functions with bodies that we read from STREAM. =A0= */ > =A0 FOR_EACH_VEC_ELT (tree, stream->fns_to_expand, i, fndecl) > =A0 =A0 { > diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c > index f7bf739..9c9a1f8 100644 > --- a/gcc/cp/pph-streamer-out.c > +++ b/gcc/cp/pph-streamer-out.c > @@ -584,21 +584,44 @@ pph_out_label_binding (pph_stream *stream, cp_label= _binding *lb, bool ref_p) > =A0} > > > +/* Outputs chained tree T by nulling out its chain first and restoring it > + =A0 after the streaming is done. STREAM and REF_P are as in > + =A0 pph_out_chain_filtered. =A0*/ > + > +static inline void > +pph_out_chained_tree (pph_stream *stream, tree t, bool ref_p) > +{ > + =A0tree saved_chain; > + > + =A0saved_chain =3D TREE_CHAIN (t); > + =A0TREE_CHAIN (t) =3D NULL_TREE; > + > + =A0pph_out_tree_or_ref_1 (stream, t, ref_p, 2); > + > + =A0TREE_CHAIN (t) =3D saved_chain; > +} > + > + > =A0/* Output a chain of nodes to STREAM starting with FIRST. =A0Skip any > =A0 =A0nodes that do not match FILTER. =A0REF_P is true if nodes in the c= hain > - =A0 should be emitted as references. =A0*/ > + =A0 should be emitted as references. =A0Stream the chain in the reverse= order > + =A0 if REVERSE is true.*/ > > =A0static void > =A0pph_out_chain_filtered (pph_stream *stream, tree first, bool ref_p, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0enum chain_filter fi= lter) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0enum chain_filter fi= lter, bool reverse_p) > =A0{ > =A0 unsigned count; > + =A0int i; > =A0 tree t; > + =A0VEC(tree,gc) *reverse_list =3D NULL; > > =A0 /* Special case. =A0If the caller wants no filtering, it is much > =A0 =A0 =A0faster to just call pph_out_chain directly. =A0*/ > =A0 if (filter =3D=3D NONE) > =A0 =A0 { > + =A0 =A0 =A0if (reverse_p) > + =A0 =A0 =A0 nreverse (first); > =A0 =A0 =A0 pph_out_chain (stream, first, ref_p); > =A0 =A0 =A0 return; > =A0 =A0 } > @@ -612,24 +635,31 @@ pph_out_chain_filtered (pph_stream *stream, tree fi= rst, bool ref_p, > =A0 =A0 } > =A0 pph_out_uint (stream, count); > > + =A0/* We cannot use the actual chain and simply reverse that before > + =A0 =A0 streaming out below as there are other chains being streamed out > + =A0 =A0 as part of streaming the trees in the current chain and this > + =A0 =A0 creates conflicts. Thus, we will create an array containing all > + =A0 =A0 the trees we want to stream out and stream that backwards witho= ut > + =A0 =A0 altering the chain itself. =A0*/ > + =A0if (reverse_p && count > 0) > + =A0 =A0reverse_list =3D VEC_alloc (tree, gc, count); > + > =A0 /* Output all the nodes that match the filter. =A0*/ > =A0 for (t =3D first; t; t =3D TREE_CHAIN (t)) > =A0 =A0 { > - =A0 =A0 =A0tree saved_chain; > - > =A0 =A0 =A0 /* Apply filters to T. =A0*/ > =A0 =A0 =A0 if (filter =3D=3D NO_BUILTINS && DECL_P (t) && DECL_IS_BUILTI= N (t)) > =A0 =A0 =A0 =A0continue; > > - =A0 =A0 =A0/* Clear TREE_CHAIN to avoid blindly recursing into the rest > - =A0 =A0 =A0 =A0of the list. =A0*/ > - =A0 =A0 =A0saved_chain =3D TREE_CHAIN (t); > - =A0 =A0 =A0TREE_CHAIN (t) =3D NULL_TREE; > - > - =A0 =A0 =A0pph_out_tree_or_ref_1 (stream, t, ref_p, 2); > - > - =A0 =A0 =A0TREE_CHAIN (t) =3D saved_chain; > + =A0 =A0 =A0if (reverse_p) > + =A0 =A0 =A0 VEC_quick_push (tree, reverse_list, t); > + =A0 =A0 =A0else > + =A0 =A0 =A0 pph_out_chained_tree (stream, t, ref_p); > =A0 =A0 } > + > + =A0if (reverse_p && count > 0) > + =A0 =A0FOR_EACH_VEC_ELT_REVERSE (tree, reverse_list, i, t) > + =A0 =A0 =A0pph_out_chained_tree (stream, t, ref_p); > =A0} > > > @@ -648,13 +678,14 @@ pph_out_binding_level (pph_stream *stream, struct c= p_binding_level *bl, > =A0 if (!pph_out_start_record (stream, bl)) > =A0 =A0 return; > > - =A0pph_out_chain_filtered (stream, bl->names, ref_p, NO_BUILTINS); > - =A0pph_out_chain_filtered (stream, bl->namespaces, ref_p, NO_BUILTINS); > + =A0pph_out_chain_filtered (stream, bl->names, ref_p, NO_BUILTINS, true); > + =A0pph_out_chain_filtered (stream, bl->namespaces, ref_p, NO_BUILTINS, = true); > > =A0 pph_out_tree_vec (stream, bl->static_decls, ref_p); > > - =A0pph_out_chain_filtered (stream, bl->usings, ref_p, NO_BUILTINS); > - =A0pph_out_chain_filtered (stream, bl->using_directives, ref_p, NO_BUIL= TINS); > + =A0pph_out_chain_filtered (stream, bl->usings, ref_p, NO_BUILTINS, true= ); > + =A0pph_out_chain_filtered (stream, bl->using_directives, ref_p, NO_BUIL= TINS, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 true); > > =A0 pph_out_uint (stream, VEC_length (cp_class_binding, bl->class_shadowe= d)); > =A0 FOR_EACH_VEC_ELT (cp_class_binding, bl->class_shadowed, i, cs) > diff --git a/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc b/gcc/testsuite/g++= .dg/pph/c1pr44948-1a.cc > index c13ee87..0340dc5 100644 > --- a/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc > +++ b/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc > @@ -1,3 +1,3 @@ > =A0// { dg-xfail-if "INFINITE" { "*-*-*" } { "-fpph-map=3Dpph.map" } } > -// { dg-bogus "internal compiler error: in lto_streamer_cache_get, at lt= o-streamer.c" "" { xfail *-*-* } 0 } > +// { dg-bogus "internal compiler error: in lto_get_pickled_tree, at lto-= streamer-in.c" "" { xfail *-*-* } 0 } > =A0#include "c0pr44948-1a.h" > diff --git a/gcc/testsuite/g++.dg/pph/c2pr36533.cc b/gcc/testsuite/g++.dg= /pph/c2pr36533.cc > index d8d6d8c..b44e8c9 100644 > --- a/gcc/testsuite/g++.dg/pph/c2pr36533.cc > +++ b/gcc/testsuite/g++.dg/pph/c2pr36533.cc > @@ -1,3 +1,2 @@ > =A0/* { dg-options "-w -fpermissive" } */ > -// pph asm xdiff > =A0#include "c1pr36533.h" > diff --git a/gcc/testsuite/g++.dg/pph/x2functions.cc b/gcc/testsuite/g++.= dg/pph/x2functions.cc > index 78df01b..d4e2cf7 100644 > --- a/gcc/testsuite/g++.dg/pph/x2functions.cc > +++ b/gcc/testsuite/g++.dg/pph/x2functions.cc > @@ -1,5 +1,3 @@ > -// pph asm xdiff > - > =A0#include "x1functions.h" > > =A0int type::mbr_decl_then_def(int i) =A0 =A0 =A0// need body > > -- > This patch is available for review at http://codereview.appspot.com/46950= 48 >