From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31605 invoked by alias); 5 May 2011 09:07:13 -0000 Received: (qmail 31573 invoked by uid 22791); 5 May 2011 09:07:09 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST X-Spam-Check-By: sourceware.org Received: from mail-ww0-f51.google.com (HELO mail-ww0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 05 May 2011 09:06:53 +0000 Received: by wwf26 with SMTP id 26so2007681wwf.8 for ; Thu, 05 May 2011 02:06:51 -0700 (PDT) MIME-Version: 1.0 Received: by 10.227.98.9 with SMTP id o9mr2256009wbn.25.1304586411601; Thu, 05 May 2011 02:06:51 -0700 (PDT) Received: by 10.227.20.74 with HTTP; Thu, 5 May 2011 02:06:51 -0700 (PDT) In-Reply-To: <20110504202659.AD3CFECC6@topo.tor.corp.google.com> References: <20110504202659.AD3CFECC6@topo.tor.corp.google.com> Date: Thu, 05 May 2011 09:09:00 -0000 Message-ID: Subject: Re: [pph] Add streamer hook for preloading common nodes (issue4478043) From: Richard Guenther 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-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-05/txt/msg00380.txt.bz2 On Wed, May 4, 2011 at 10:26 PM, Diego Novillo wrote: > > This patch adds a new streamer hook to populate the streamer cache > with common nodes. > > The nodes we populate for GIMPLE in lto_get_common_nodes is not > sufficient for C++, unsurprisingly. > > The patch fixes these regressions in pph.exp: > > FAIL: g++.dg/pph/p1stdlib.cc =A0-fpph-map=3Dpph.map -I. (test for excess = errors) > FAIL: g++.dg/pph/p1stdlib.cc , PPH assembly missing > > There is a second part to this patch to handle INTEGER_CSTs as regular > trees (so they can be cached). =A0This would allow us to handle special > constants in the C++ FE like void_zero_node, but that's giving me some > trouble with LTO tests. > > Tested on x86_64. =A0Committed to the branch. I think we should move away from pre-loading the streamer cache, that has caused enough trouble when the common nodes are originating from different Frontends and when compiling units with different flags which happen to change those nodes (think of the hoops we jump through to support that for -f[un]signed-char). Richard. > > Diego. > > ChangeLog.pph > > =A0 =A0 =A0 =A0* Makefile.in (cgraphunit.o): Add dependency on LTO_STREAM= ER_H. > =A0 =A0 =A0 =A0* cgraphunit.c: Include lto-streamer.h > =A0 =A0 =A0 =A0(cgraph_finalize_compilation_unit): Call gimple_streamer_h= ooks_init > =A0 =A0 =A0 =A0if LTO is enabled. > =A0 =A0 =A0 =A0* lto-streamer-out.c (lto_output): Move call to > =A0 =A0 =A0 =A0gimple_streamer_hooks_init to cgraph_finalize_compilation_= unit. > =A0 =A0 =A0 =A0* lto-streamer.c (lto_get_common_nodes): Remove if0 hack. > =A0 =A0 =A0 =A0(lto_streamer_cache_create): Call streamer_hooks.get_commo= n_nodes > =A0 =A0 =A0 =A0instead of lto_get_common_nodes. > =A0 =A0 =A0 =A0(gimple_streamer_hooks_init): Set h->get_common_nodes to > =A0 =A0 =A0 =A0lto_get_common_nodes. > =A0 =A0 =A0 =A0* lto-streamer.h (struct lto_streamer_hooks): Add field > =A0 =A0 =A0 =A0get_common_nodes. > > cp/ChangeLog.pph > > =A0 =A0 =A0 =A0* pph-streamer.c (pph_get_common_nodes): New. > =A0 =A0 =A0 =A0(pph_stream_hooks_init): Set it in h->get_common_nodes. > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index 0af93ba..f96e059 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -3001,7 +3001,7 @@ cgraphunit.o : cgraphunit.c $(CONFIG_H) $(SYSTEM_H)= coretypes.h $(TM_H) \ > =A0 =A0$(TREE_FLOW_H) $(TREE_PASS_H) debug.h $(DIAGNOSTIC_H) \ > =A0 =A0$(FIBHEAP_H) output.h $(PARAMS_H) $(RTL_H) $(TIMEVAR_H) $(IPA_PROP= _H) \ > =A0 =A0gt-cgraphunit.h tree-iterator.h $(COVERAGE_H) $(TREE_DUMP_H) \ > - =A0 tree-pretty-print.h gimple-pretty-print.h > + =A0 tree-pretty-print.h gimple-pretty-print.h $(LTO_STREAMER_H) > =A0cgraphbuild.o : cgraphbuild.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM= _H) \ > =A0 =A0$(TREE_H) langhooks.h $(CGRAPH_H) intl.h pointer-set.h $(GIMPLE_H)= \ > =A0 =A0$(TREE_FLOW_H) $(TREE_PASS_H) $(IPA_UTILS_H) $(EXCEPT_H) \ > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > index 3dbfc2b..0d1ec89 100644 > --- a/gcc/cgraphunit.c > +++ b/gcc/cgraphunit.c > @@ -138,6 +138,7 @@ along with GCC; see the file COPYING3. =A0If not see > =A0#include "output.h" > =A0#include "coverage.h" > =A0#include "plugin.h" > +#include "lto-streamer.h" > > =A0static void cgraph_expand_all_functions (void); > =A0static void cgraph_mark_functions_to_output (void); > @@ -1062,6 +1063,10 @@ cgraph_finalize_compilation_unit (void) > =A0{ > =A0 timevar_push (TV_CGRAPH); > > + =A0/* If LTO is enabled, initialize the streamer hooks needed by GIMPLE= . =A0*/ > + =A0if (flag_lto) > + =A0 =A0gimple_streamer_hooks_init (); > + > =A0 /* If we're here there's no current function anymore. =A0Some fronten= ds > =A0 =A0 =A0are lazy in clearing these. =A0*/ > =A0 current_function_decl =3D NULL; > diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c > index 5f2112e..c363c06 100644 > --- a/gcc/cp/pph-streamer.c > +++ b/gcc/cp/pph-streamer.c > @@ -55,6 +55,36 @@ pph_indexable_with_decls_p (tree t) > =A0} > > > +/* Generate a vector of common nodes that should always be streamed as > + =A0 indexes into the streamer cache. =A0These nodes are always built by > + =A0 the front end, so there is no need to emit them. =A0*/ > + > +static VEC(tree,heap) * > +pph_get_common_nodes (void) > +{ > + =A0unsigned i; > + =A0VEC(tree,heap) *common_nodes =3D NULL; > + > + =A0for (i =3D itk_char; i < itk_none; i++) > + =A0 =A0VEC_safe_push (tree, heap, common_nodes, integer_types[i]); > + > + =A0for (i =3D 0; i < TYPE_KIND_LAST; i++) > + =A0 =A0VEC_safe_push (tree, heap, common_nodes, sizetype_tab[i]); > + > + =A0/* global_trees[] can have NULL entries in it. =A0Skip them. =A0*/ > + =A0for (i =3D 0; i < TI_MAX; i++) > + =A0 =A0if (global_trees[i]) > + =A0 =A0 =A0VEC_safe_push (tree, heap, common_nodes, global_trees[i]); > + > + =A0/* c_global_trees[] can have NULL entries in it. =A0Skip them. =A0*/ > + =A0for (i =3D 0; i < CTI_MAX; i++) > + =A0 =A0if (c_global_trees[i]) > + =A0 =A0 =A0VEC_safe_push (tree, heap, common_nodes, c_global_trees[i]); > + > + =A0return common_nodes; > +} > + > + > =A0/* Initialize all the streamer hooks used for streaming ASTs. =A0*/ > > =A0static void > @@ -62,6 +92,9 @@ pph_stream_hooks_init (void) > =A0{ > =A0 lto_streamer_hooks *h =3D streamer_hooks_init (); > =A0 h->name =3D "C++ AST"; > + =A0h->get_common_nodes =3D pph_get_common_nodes; > =A0 h->is_streamable =3D pph_is_streamable; > =A0 h->write_tree =3D pph_stream_write_tree; > =A0 h->read_tree =3D pph_stream_read_tree; > diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c > index c26de5d..b578419 100644 > --- a/gcc/lto-streamer-out.c > +++ b/gcc/lto-streamer-out.c > @@ -2198,7 +2198,6 @@ lto_output (cgraph_node_set set, varpool_node_set v= set) > =A0 int i, n_nodes; > =A0 lto_cgraph_encoder_t encoder =3D lto_get_out_decl_state ()->cgraph_no= de_encoder; > > - =A0gimple_streamer_hooks_init (); > =A0 lto_writer_init (); > > =A0 n_nodes =3D lto_cgraph_encoder_size (encoder); > diff --git a/gcc/lto-streamer.c b/gcc/lto-streamer.c > index 04a9f7a..d6b78c8 100644 > --- a/gcc/lto-streamer.c > +++ b/gcc/lto-streamer.c > @@ -550,10 +550,6 @@ lto_get_common_nodes (void) > =A0 else > =A0 =A0 main_identifier_node =3D get_identifier ("main"); > > - =A0/* FIXME pph. =A0These assertions are never met while in the front e= nd. > - =A0 =A0 There should be a way of checking this only when we are in LTO > - =A0 =A0 mode. =A0*/ > -#if 0 > =A0 gcc_assert (ptrdiff_type_node =3D=3D integer_type_node); > > =A0 /* FIXME lto. =A0In the C++ front-end, fileptr_type_node is defined a= s a > @@ -564,7 +560,6 @@ lto_get_common_nodes (void) > =A0 =A0 =A0These should be assured in pass_ipa_free_lang_data. =A0*/ > =A0 gcc_assert (fileptr_type_node =3D=3D ptr_type_node); > =A0 gcc_assert (TYPE_MAIN_VARIANT (fileptr_type_node) =3D=3D ptr_type_nod= e); > -#endif > > =A0 seen_nodes =3D pointer_set_create (); > > @@ -631,7 +626,7 @@ lto_streamer_cache_create (void) > =A0 /* Load all the well-known tree nodes that are always created by > =A0 =A0 =A0the compiler on startup. =A0This prevents writing them out > =A0 =A0 =A0unnecessarily. =A0*/ > - =A0common_nodes =3D lto_get_common_nodes (); > + =A0common_nodes =3D streamer_hooks ()->get_common_nodes (); > > =A0 FOR_EACH_VEC_ELT (tree, common_nodes, i, node) > =A0 =A0 preload_common_node (cache, node); > @@ -820,6 +815,7 @@ gimple_streamer_hooks_init (void) > =A0 h->name =3D "gimple"; > =A0 h->reader_init =3D gimple_streamer_reader_init; > =A0 h->writer_init =3D NULL; > + =A0h->get_common_nodes =3D lto_get_common_nodes; > =A0 h->is_streamable =3D lto_is_streamable; > =A0 h->write_tree =3D gimple_streamer_write_tree; > =A0 h->read_tree =3D gimple_streamer_read_tree; > diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h > index b6f4b79..8be17da 100644 > --- a/gcc/lto-streamer.h > +++ b/gcc/lto-streamer.h > @@ -55,6 +55,16 @@ typedef struct lto_streamer_hooks { > =A0 /* A string identifying this streamer. =A0*/ > =A0 const char *name; > > + =A0/* Called by lto_streamer_cache_create to instantiate a cache of > + =A0 =A0 well-known nodes. =A0These are tree nodes that are always > + =A0 =A0 instantiated by the compiler on startup. =A0Additionally, these > + =A0 =A0 nodes need to be shared. =A0This function produces a vector of > + =A0 =A0 these well known trees that are then added to the streamer cach= e. > + =A0 =A0 This way, the writer will only write out a reference to the tree > + =A0 =A0 and the reader will instantiate the tree out of this > + =A0 =A0 pre-populated cache. =A0*/ > + =A0VEC(tree,heap) *(*get_common_nodes) (void); > + > =A0 /* Called by lto_reader_init after it does basic initialization. =A0*/ > =A0 void (*reader_init) (void); > > > -- > This patch is available for review at http://codereview.appspot.com/44780= 43 >