From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2052 invoked by alias); 1 Jun 2011 16:03:24 -0000 Received: (qmail 2041 invoked by uid 22791); 1 Jun 2011 16:03:22 -0000 X-SWARE-Spam-Status: No, hits=-2.2 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; Wed, 01 Jun 2011 16:03:06 +0000 Received: from kpbe11.cbf.corp.google.com (kpbe11.cbf.corp.google.com [172.25.105.75]) by smtp-out.google.com with ESMTP id p51G33b1005579 for ; Wed, 1 Jun 2011 09:03:04 -0700 Received: from gyf2 (gyf2.prod.google.com [10.243.50.66]) by kpbe11.cbf.corp.google.com with ESMTP id p51G2KHU019441 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Wed, 1 Jun 2011 09:03:02 -0700 Received: by gyf2 with SMTP id 2so3239309gyf.39 for ; Wed, 01 Jun 2011 09:03:02 -0700 (PDT) MIME-Version: 1.0 Received: by 10.150.141.10 with SMTP id o10mr3735659ybd.70.1306944182130; Wed, 01 Jun 2011 09:03:02 -0700 (PDT) Received: by 10.151.41.13 with HTTP; Wed, 1 Jun 2011 09:03:02 -0700 (PDT) In-Reply-To: References: <20110601001232.CCD681DA1C9@topo.tor.corp.google.com> Date: Wed, 01 Jun 2011 16:03:00 -0000 Message-ID: Subject: Re: [lto] Merge streamer hooks from pph branch. (issue4568043) From: Diego Novillo To: Richard Guenther Cc: reply@codereview.appspotmail.com, Jan Hubicka , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=UTF-8 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-06/txt/msg00063.txt.bz2 On Wed, Jun 1, 2011 at 08:07, Richard Guenther wrote: >> =C2=A0static void cgraph_expand_all_functions (void); >> =C2=A0static void cgraph_mark_functions_to_output (void); >> @@ -1092,6 +1093,10 @@ cgraph_finalize_compilation_unit (void) >> =C2=A0{ >> =C2=A0 =C2=A0timevar_push (TV_CGRAPH); >> >> + =C2=A0/* If LTO is enabled, initialize the streamer hooks needed by GI= MPLE. =C2=A0*/ >> + =C2=A0if (flag_lto) >> + =C2=A0 =C2=A0gimple_streamer_hooks_init (); > > Ugh. =C2=A0Isn't there a better entry for this? =C2=A0Are you going to add > > =C2=A0if (flag_pph) > =C2=A0 =C2=A0init_hooks_some_other_way (); > > here? =C2=A0It looks it rather belongs to opts.c or toplev.c if the hooks > are really initialized dependent on compiler flags. Not at all, this is for gimple, specifically. The front end initializes hooks in its own way. The problem here is that the gimple hooks are needed by the middle end. If we initialize gimple hooks too early, the FE will override them. So we need to initialize them after the front end is done (hence the location for this call). I'm happy to move this somewhere else, but it needs to happen right before the middle end starts calling LTO pickling routines. > >> =C2=A0 =C2=A0/* If we're here there's no current function anymore. =C2= =A0Some frontends >> =C2=A0 =C2=A0 =C2=A0 are lazy in clearing these. =C2=A0*/ >> =C2=A0 =C2=A0current_function_decl =3D NULL; >> diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c >> index 88966f2..801fe6f 100644 >> --- a/gcc/lto-streamer-in.c >> +++ b/gcc/lto-streamer-in.c >> @@ -1833,6 +1833,7 @@ static void >> =C2=A0unpack_value_fields (struct bitpack_d *bp, tree expr) >> =C2=A0{ >> =C2=A0 =C2=A0enum tree_code code; >> + =C2=A0lto_streamer_hooks *h =3D streamer_hooks (); > > A function to access a global ... we have lang_hooks and targetm, > so please simply use streamer_hooks as a variable. > streamer_hooks ()->preload_common_nodes (cache) looks super-ugly. I did not want to add yet another global. I don't feel too strong about this one, given the presence of lang_hooks and targetm. So, you prefer the direct global access? >> @@ -1864,26 +1865,11 @@ unpack_value_fields (struct bitpack_d *bp, tree = expr) >> =C2=A0 =C2=A0if (CODE_CONTAINS_STRUCT (code, TS_BLOCK)) >> =C2=A0 =C2=A0 =C2=A0unpack_ts_block_value_fields (bp, expr); >> >> - =C2=A0if (CODE_CONTAINS_STRUCT (code, TS_SSA_NAME)) >> - =C2=A0 =C2=A0{ >> - =C2=A0 =C2=A0 =C2=A0/* We only stream the version number of SSA names.= =C2=A0*/ >> - =C2=A0 =C2=A0 =C2=A0gcc_unreachable (); >> - =C2=A0 =C2=A0} >> - >> - =C2=A0if (CODE_CONTAINS_STRUCT (code, TS_STATEMENT_LIST)) >> - =C2=A0 =C2=A0{ >> - =C2=A0 =C2=A0 =C2=A0/* This is only used by GENERIC. =C2=A0*/ >> - =C2=A0 =C2=A0 =C2=A0gcc_unreachable (); >> - =C2=A0 =C2=A0} >> - >> - =C2=A0if (CODE_CONTAINS_STRUCT (code, TS_OMP_CLAUSE)) >> - =C2=A0 =C2=A0{ >> - =C2=A0 =C2=A0 =C2=A0/* This is only used by High GIMPLE. =C2=A0*/ >> - =C2=A0 =C2=A0 =C2=A0gcc_unreachable (); >> - =C2=A0 =C2=A0} >> - >> =C2=A0 =C2=A0if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL)) >> =C2=A0 =C2=A0 =C2=A0unpack_ts_translation_unit_decl_value_fields (bp, ex= pr); >> + >> + =C2=A0if (h->unpack_value_fields) >> + =C2=A0 =C2=A0h->unpack_value_fields (bp, expr); > > I suppose the LTO implementation has a gcc_unreachable () for > the cases we do not handle here? Right. This was already superfluous. It's tested already by lto_is_streamable(). > >> =C2=A0} >> >> >> @@ -1935,8 +1921,17 @@ lto_materialize_tree (struct lto_input_block *ib,= struct data_in *data_in, >> =C2=A0 =C2=A0 =C2=A0} >> =C2=A0 =C2=A0else >> =C2=A0 =C2=A0 =C2=A0{ >> - =C2=A0 =C2=A0 =C2=A0/* All other nodes can be materialized with a raw = make_node call. =C2=A0*/ >> - =C2=A0 =C2=A0 =C2=A0result =3D make_node (code); >> + =C2=A0 =C2=A0 =C2=A0lto_streamer_hooks *h =3D streamer_hooks (); >> + >> + =C2=A0 =C2=A0 =C2=A0/* For all other nodes, see if the streamer knows = how to allocate >> + =C2=A0 =C2=A0 =C2=A0it. =C2=A0*/ >> + =C2=A0 =C2=A0 =C2=A0if (h->alloc_tree) >> + =C2=A0 =C2=A0 result =3D h->alloc_tree (code, ib, data_in); >> + >> + =C2=A0 =C2=A0 =C2=A0/* If the hook did not handle it, materialize the = tree with a raw >> + =C2=A0 =C2=A0 =C2=A0make_node call. =C2=A0*/ >> + =C2=A0 =C2=A0 =C2=A0if (result =3D=3D NULL_TREE) >> + =C2=A0 =C2=A0 result =3D make_node (code); >> =C2=A0 =C2=A0 =C2=A0} >> >> =C2=A0#ifdef LTO_STREAMER_DEBUG >> @@ -2031,12 +2026,8 @@ lto_input_ts_decl_common_tree_pointers (struct lt= o_input_block *ib, >> =C2=A0{ >> =C2=A0 =C2=A0DECL_SIZE (expr) =3D lto_input_tree (ib, data_in); >> =C2=A0 =C2=A0DECL_SIZE_UNIT (expr) =3D lto_input_tree (ib, data_in); >> - >> - =C2=A0if (TREE_CODE (expr) !=3D FUNCTION_DECL >> - =C2=A0 =C2=A0 =C2=A0&& TREE_CODE (expr) !=3D TRANSLATION_UNIT_DECL) >> - =C2=A0 =C2=A0DECL_INITIAL (expr) =3D lto_input_tree (ib, data_in); >> - > > Why move those? =C2=A0DECL_INITIAL _is_ in decl_common. I needed to move the handling of DECL_INITIAL in the writer. This forces us to move the handling in the reader. Otherwise, reader and writer will be out of sync (DECL_INITIAL is now written last). > Where do those checks go? =C2=A0Or do we simply lose them? They already are in lto_is_streamable. See above. >> - =C2=A0if (TREE_CODE (result) =3D=3D VAR_DECL) >> - =C2=A0 =C2=A0lto_register_var_decl_in_symtab (data_in, result); >> - =C2=A0else if (TREE_CODE (result) =3D=3D FUNCTION_DECL && !DECL_BUILT_= IN (result)) >> - =C2=A0 =C2=A0lto_register_function_decl_in_symtab (data_in, result); >> + =C2=A0if (h->register_decls_in_symtab_p) >> + =C2=A0 =C2=A0{ >> + =C2=A0 =C2=A0 =C2=A0if (TREE_CODE (result) =3D=3D VAR_DECL) >> + =C2=A0 =C2=A0 lto_register_var_decl_in_symtab (data_in, result); >> + =C2=A0 =C2=A0 =C2=A0else if (TREE_CODE (result) =3D=3D FUNCTION_DECL &= & !DECL_BUILT_IN (result)) >> + =C2=A0 =C2=A0 lto_register_function_decl_in_symtab (data_in, result); >> + =C2=A0 =C2=A0} > > I would say we should defer symtab registering to uniquify_nodes time, > when we are sure we completely streamed in the tree we want to register > (then we don't have to deal with partially read trees in those functions). > > Can you work on a preparatory patch for this please? =C2=A0That would get > rid for the need of this hook and clean up things at the same time. That's a much better idea. Will do. > >> >> =C2=A0 =C2=A0/* end_marker =3D */ lto_input_1_unsigned (ib); >> >> @@ -2682,6 +2659,22 @@ lto_read_tree (struct lto_input_block *ib, struct= data_in *data_in, >> =C2=A0} >> >> >> +/* LTO streamer hook for reading GIMPLE trees. =C2=A0IB and DATA_IN are= as in >> + =C2=A0 lto_read_tree. =C2=A0EXPR is the tree was materialized by lto_r= ead_tree and >> + =C2=A0 needs GIMPLE specific data to be filled in. =C2=A0*/ >> + >> +void >> +gimple_streamer_read_tree (struct lto_input_block *ib, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0struct data_in *data_in, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0tree expr) >> +{ >> + =C2=A0if (DECL_P (expr) >> + =C2=A0 =C2=A0 =C2=A0&& TREE_CODE (expr) !=3D FUNCTION_DECL >> + =C2=A0 =C2=A0 =C2=A0&& TREE_CODE (expr) !=3D TRANSLATION_UNIT_DECL) >> + =C2=A0 =C2=A0DECL_INITIAL (expr) =3D lto_input_tree (ib, data_in); > > What's wrong with doing this in the decl-common path? Just that if the writer code moves, the reader needs to move as well to avoid out-of-sync problems. >> @@ -772,9 +758,23 @@ lto_output_tree_ref (struct output_block *ob, tree = expr) >> =C2=A0 =C2=A0 =C2=A0 =C2=A0break; >> >> =C2=A0 =C2=A0 =C2=A0default: >> - =C2=A0 =C2=A0 =C2=A0/* No other node is indexable, so it should have b= een handled >> - =C2=A0 =C2=A0 =C2=A0by lto_output_tree. =C2=A0*/ >> - =C2=A0 =C2=A0 =C2=A0gcc_unreachable (); >> + =C2=A0 =C2=A0 =C2=A0{ >> + =C2=A0 =C2=A0 lto_streamer_hooks *h =3D streamer_hooks (); >> + >> + =C2=A0 =C2=A0 /* See if streamer hooks allows this node to be indexabl= e with >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0VAR_DECLs. =C2=A0*/ > > with VAR_DECLs? =C2=A0More like "similar to global decls."? Changed. > >> + =C2=A0 =C2=A0 if (h->indexable_with_decls_p && h->indexable_with_decls= _p (expr)) >> + =C2=A0 =C2=A0 =C2=A0 { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 output_record_start (ob, LTO_global_decl_r= ef); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 lto_output_var_decl_index (ob->decl_state,= ob->main_stream, expr); > > Why hook it this way and not > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (h->output_tree_ref > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 && h->output_tree= _ref (...)) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break; > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gcc_unreachable (); > > I find the flag vs. function hook stuff somewhat odd. Sure. It's > >> + =C2=A0 =C2=A0 =C2=A0 } >> + =C2=A0 =C2=A0 else >> + =C2=A0 =C2=A0 =C2=A0 { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* No other node is indexable, so it shoul= d have been >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 handled by lto_output_tree. =C2=A0*/ >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 gcc_unreachable (); >> + =C2=A0 =C2=A0 =C2=A0 } >> + =C2=A0 =C2=A0 =C2=A0} >> =C2=A0 =C2=A0 =C2=A0} >> =C2=A0} >> >> @@ -883,27 +883,10 @@ lto_output_ts_decl_common_tree_pointers (struct ou= tput_block *ob, tree expr, >> =C2=A0 =C2=A0lto_output_tree_or_ref (ob, DECL_SIZE (expr), ref_p); >> =C2=A0 =C2=A0lto_output_tree_or_ref (ob, DECL_SIZE_UNIT (expr), ref_p); >> >> - =C2=A0if (TREE_CODE (expr) !=3D FUNCTION_DECL >> - =C2=A0 =C2=A0 =C2=A0&& TREE_CODE (expr) !=3D TRANSLATION_UNIT_DECL) >> - =C2=A0 =C2=A0{ >> - =C2=A0 =C2=A0 =C2=A0tree initial =3D DECL_INITIAL (expr); >> - =C2=A0 =C2=A0 =C2=A0if (TREE_CODE (expr) =3D=3D VAR_DECL >> - =C2=A0 =C2=A0 =C2=A0 && (TREE_STATIC (expr) || DECL_EXTERNAL (expr)) >> - =C2=A0 =C2=A0 =C2=A0 && initial) >> - =C2=A0 =C2=A0 { >> - =C2=A0 =C2=A0 =C2=A0 lto_varpool_encoder_t varpool_encoder =3D ob->dec= l_state->varpool_node_encoder; >> - =C2=A0 =C2=A0 =C2=A0 struct varpool_node *vnode =3D varpool_get_node (= expr); >> - =C2=A0 =C2=A0 =C2=A0 if (!vnode) >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 initial =3D error_mark_node; >> - =C2=A0 =C2=A0 =C2=A0 else if (!lto_varpool_encoder_encode_initializer_= p (varpool_encoder, >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vnode)) >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 initial =3D NULL; >> - =C2=A0 =C2=A0 } >> - >> - =C2=A0 =C2=A0 =C2=A0lto_output_tree_or_ref (ob, initial, ref_p); >> - =C2=A0 =C2=A0} >> + =C2=A0/* Do not stream DECL_INITIAL. =C2=A0*/ > > That's a gross comment ;) =C2=A0We _do_ stream it, but now in a hook. Fair enough. > I suppose all the streamer functions should lose their lto_ prefix - > reading them with such comments will get confusing ... Yeah, good point. I'll send a rename patch. >> =C2=A0 =C2=A0/* We should not see any non-GIMPLE tree nodes here. =C2=A0= */ >> =C2=A0 =C2=A0code =3D TREE_CODE (expr); >> - =C2=A0if (!lto_is_streamable (expr)) >> - =C2=A0 =C2=A0internal_error ("tree code %qs is not supported in gimple= streams", >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tree_code_name= [code]); >> + =C2=A0if (h->is_streamable && !h->is_streamable (expr)) >> + =C2=A0 =C2=A0internal_error ("tree code %qs is not supported in %s str= eams", >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 h->name, tree_= code_name[code]); > > I'd say this hook should always exist. Sure. >> =C2=A0 =C2=A0/* The header of a tree node consists of its tag, the size = of >> =C2=A0 =C2=A0 =C2=A0 the node, and any other information needed to insta= ntiate >> =C2=A0 =C2=A0 =C2=A0 EXPR on the reading side (such as the number of slo= ts in >> =C2=A0 =C2=A0 =C2=A0 variable sized nodes). =C2=A0*/ >> =C2=A0 =C2=A0tag =3D lto_tree_code_to_tag (code); >> + =C2=A0gcc_assert ((unsigned) tag < (unsigned) LTO_NUM_TAGS); > > Doesn't Honzas streaming this as enum already assert this? Yeah. I had removed this before testing, but I sent out the original patch. >> +/* GIMPLE hook for writing GIMPLE-specific parts of trees. =C2=A0OB, EX= PR >> + =C2=A0 and REF_P are as in lto_write_tree. =C2=A0*/ >> + >> +void >> +gimple_streamer_write_tree (struct output_block *ob, tree expr, bool re= f_p) >> +{ >> + =C2=A0if (TREE_CODE (expr) =3D=3D FUNCTION_DECL) >> + =C2=A0 =C2=A0{ >> + =C2=A0 =C2=A0 =C2=A0/* DECL_SAVED_TREE holds the GENERIC representatio= n for DECL. >> + =C2=A0 =C2=A0 =C2=A0At this point, it should not exist. =C2=A0Either b= ecause it was >> + =C2=A0 =C2=A0 =C2=A0converted to gimple or because DECL didn't have a = GENERIC >> + =C2=A0 =C2=A0 =C2=A0representation in this TU. =C2=A0*/ >> + =C2=A0 =C2=A0 =C2=A0gcc_assert (DECL_SAVED_TREE (expr) =3D=3D NULL_TRE= E); >> + =C2=A0 =C2=A0} > > I think we can drop this check. Done. >> @@ -1438,8 +1450,27 @@ lto_output_tree (struct output_block *ob, tree ex= pr, bool ref_p) >> =C2=A0 =C2=A0 =C2=A0 to be materialized by the reader (to implement TYPE= _CACHED_VALUES). =C2=A0*/ >> =C2=A0 =C2=A0if (TREE_CODE (expr) =3D=3D INTEGER_CST) >> =C2=A0 =C2=A0 =C2=A0{ >> - =C2=A0 =C2=A0 =C2=A0lto_output_integer_cst (ob, expr, ref_p); >> - =C2=A0 =C2=A0 =C2=A0return; >> + =C2=A0 =C2=A0 =C2=A0bool is_special; >> + >> + =C2=A0 =C2=A0 /* There are some constants that are special to the stre= amer >> + =C2=A0 =C2=A0 (e.g., void_zero_node, truthvalue_false_node). >> + =C2=A0 =C2=A0 These constants cannot be rematerialized with >> + =C2=A0 =C2=A0 build_int_cst_wide because they may actually lack a type= (like >> + =C2=A0 =C2=A0 void_zero_node) and they need to be pointer-identical to= trees >> + =C2=A0 =C2=A0 materialized by the compiler tables like global_trees or >> + =C2=A0 =C2=A0 c_global_trees. >> + >> + =C2=A0 =C2=A0 If the streamer told us that it has special constants, t= hey >> + =C2=A0 =C2=A0 will be preloaded in the streamer cache. =C2=A0If we fin= d a match, >> + =C2=A0 =C2=A0 then stream the constant as a reference so the reader can >> + =C2=A0 =C2=A0 re-materialize it from the cache. =C2=A0*/ >> + =C2=A0 =C2=A0 =C2=A0is_special =3D streamer_hooks ()->has_unique_integ= er_csts_p >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&& lto_streamer= _cache_lookup (ob->writer_cache, expr, NULL); >> + =C2=A0 =C2=A0 =C2=A0if (!is_special) >> + =C2=A0 =C2=A0 { >> + =C2=A0 =C2=A0 =C2=A0 lto_output_integer_cst (ob, expr, ref_p); >> + =C2=A0 =C2=A0 =C2=A0 return; >> + =C2=A0 =C2=A0 } > > ??? =C2=A0We should not arrive here for such global trees. =C2=A0Please d= o not > merge this part of the patch as part of the hook introducing (keep > patches simple, make them do a single thing ...) Not sure what you are objecting to. We do execute this for global trees in the C++ FE (as described in the comment). Are you objecting to never handling unique constants or to merging this handling until the pph bits are in? >> @@ -2238,6 +2269,8 @@ static void >> =C2=A0lto_writer_init (void) >> =C2=A0{ >> =C2=A0 =C2=A0lto_streamer_init (); >> + =C2=A0if (streamer_hooks ()->writer_init) >> + =C2=A0 =C2=A0streamer_hooks ()->writer_init (); > > This hook should always exist. =C2=A0Why is this called in a context with > lto_*? There is common streaming code that both the gimple and c++ streamers use. I suppose both could call lto_streamer_init and then call their own initializer routines instead of doing a hook. >> + >> + >> +/* Return true if EXPR is a tree node that can be written to disk. =C2= =A0*/ >> + >> +static inline bool >> +lto_is_streamable (tree expr) >> +{ >> + =C2=A0enum tree_code code =3D TREE_CODE (expr); >> + >> + =C2=A0/* Notice that we reject SSA_NAMEs as well. =C2=A0We only emit t= he SSA >> + =C2=A0 =C2=A0 name version in lto_output_tree_ref (see output_ssa_name= s). =C2=A0*/ >> + =C2=A0return !is_lang_specific (expr) >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D SSA_NAME >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D CALL_EXPR >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D LANG_TYPE >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D MODIFY_EXPR >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D INIT_EXPR >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D TARGET_EXPR >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D BIND_EXPR >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D WITH_CLEANUP_EXPR >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D STATEMENT_LIST >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D OMP_CLAUSE >> + =C2=A0 =C2=A0 =C2=A0&& code !=3D OPTIMIZATION_NODE >> + =C2=A0 =C2=A0 =C2=A0&& (code =3D=3D CASE_LABEL_EXPR >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|| code =3D=3D DECL_EXPR >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|| TREE_CODE_CLASS (code) !=3D tcc_s= tatement); >> +} > > This change (with the removal of the cases with the unreachable()s) > could be made separately (directly calling it instead of hooking it). I still need to hook this because pph_is_streamable is much more lenient. > >> + >> +/* Initialize all the streamer hooks used for streaming GIMPLE. =C2=A0*/ >> + >> +void >> +gimple_streamer_hooks_init (void) > > It's more like lto_streamer_hooks_init, no? =C2=A0You are basically > turning lto_streamer_* to tree_streamer_* and make lto_streamer_* > tree_streamer_* + lto_streamer_hooks, no? I was about to call them gimple_streamer_hooks, but lto_streamer_hooks is also fine with me. Subsequent patch. So: 1- Call the generic implementation and streamer hooks tree_streamer_* 2- Rename the lto-specific parts of streaming to lto_streamer_* 3- Move generic streaming implementation to tree-streamer.[ch] Does that sound OK? >> @@ -186,7 +192,7 @@ enum LTO_tags >> >> =C2=A0 =C2=A0 =C2=A0 Conversely, to map between LTO tags and tree/gimple= codes, the >> =C2=A0 =C2=A0 =C2=A0 reverse operation must be applied. =C2=A0*/ >> - =C2=A0LTO_bb0 =3D 1 + NUM_TREE_CODES + LAST_AND_UNUSED_GIMPLE_CODE, >> + =C2=A0LTO_bb0 =3D 1 + MAX_TREE_CODES + LAST_AND_UNUSED_GIMPLE_CODE, >> =C2=A0 =C2=A0LTO_bb1, > > This (and related) change can be merged separately. Yes. Thanks for the feedback. Diego.