From patchwork Mon Jul 11 21:42:28 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Diego Novillo X-Patchwork-Id: 104289 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 0D745B6F75 for ; Tue, 12 Jul 2011 07:42:56 +1000 (EST) Received: (qmail 30244 invoked by alias); 11 Jul 2011 21:42:55 -0000 Received: (qmail 30233 invoked by uid 22791); 11 Jul 2011 21:42:53 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, TW_CX, 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; Mon, 11 Jul 2011 21:42:38 +0000 Received: from kpbe17.cbf.corp.google.com (kpbe17.cbf.corp.google.com [172.25.105.81]) by smtp-out.google.com with ESMTP id p6BLgUs8032530; Mon, 11 Jul 2011 14:42:31 -0700 Received: from topo.tor.corp.google.com (topo.tor.corp.google.com [172.29.41.2]) by kpbe17.cbf.corp.google.com with ESMTP id p6BLgShX021429; Mon, 11 Jul 2011 14:42:29 -0700 Received: by topo.tor.corp.google.com (Postfix, from userid 54752) id 4B2BB1DA197; Mon, 11 Jul 2011 17:42:28 -0400 (EDT) To: reply@codereview.appspotmail.com, crowl@google.com, gchare@google.com, gcc-patches@gcc.gnu.org Subject: [pph] Add alternate addresses to register in the cache (issue4685054) Message-Id: <20110711214228.4B2BB1DA197@topo.tor.corp.google.com> Date: Mon, 11 Jul 2011 17:42:28 -0400 (EDT) From: dnovillo@google.com (Diego Novillo) 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-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org This patch adapts an idea from Gab that allow us to register alternate addresses in the cache. The problem here is making sure that symbols read from a PPH file reference the right bindings. If a symbol is in the global namespace when compiling a header file, its bindings will point to NAMESPACE_LEVEL(global_namespace)->bindings, but that global_namespace is the global_namespace instantiated for the header file. When reading that PPH image from a translation unit, we need to refer to the bindings of the *current* global_namespace. In general we solve this by inserting the pointer in the streamer cache. For instance, to avoid instantiating a second global_namespace decl, the initialization code of both the writer and the reader store global_namespace into the streaming cache. This way, all the references to global_namespace point to the current global_namespace as known by the writer and the reader. However, we cannot use the same trick on the bindings for global_namespace. If we simply inserted it into the cache then writing out NAMESPACE_LEVEL(global_namespace)->bindings would simply write a reference to the current one and on the reader side, it would simply restore a pointer to the current translation unit's bindings. Without ever actually writing or reading anything (since it was satisified from the cache). Therefore, we want a mechanism that allows the reader to: (a) read all the symbols in the global bindings, and (b) references to the global binding made by the symbols should point to the global bindings of the current translation unit (instead of the one in the PPH image). That's where ALLOC_AND_REGISTER_ALTERNATE comes in. When called, it allocates the data structure but registers another pointer in the cache. We use this trick when calling pph_in_binding_level from the toplevel: + new_bindings = pph_in_binding_level (stream, scope_chain->bindings); This way, when pph_in_binding_level tries to allocate the binding structure read from STREAM, it registers scope_chain->bindings in the cache. This way, references to the original file's global binding are automatically redirected to the current translation unit's global bindings. Gab, I modified your original implementation to move all the logic to the place where we need to make this decision. This way, it is easier to tell which functions need this alternate registration, instead of relying on some status flag squirreled away in the STREAM data structure. Tested on x86_64. Applied to branch. 2011-07-11 Diego Novillo Gabriel Charette * pph-streamer-in.c (ALLOC_AND_REGISTER_ALTERNATE): Define. (pph_in_binding_level): Add argument TO_REGISTER. Call ALLOC_AND_REGISTER_ALTERNATE if set. Update all users. (pph_register_decls_in_symtab): Call varpool_finalize_decl on all file-local symbols. (pph_in_scope_chain): Call pph_in_binding_level with scope_chain->bindings as the alternate pointer to register in the streaming cache. --- This patch is available for review at http://codereview.appspot.com/4685054 diff --git a/gcc/cp/ChangeLog.pph b/gcc/cp/ChangeLog.pph index 1011902..f18c2f4 100644 --- a/gcc/cp/ChangeLog.pph +++ b/gcc/cp/ChangeLog.pph @@ -1,3 +1,16 @@ +2011-07-11 Diego Novillo + Gabriel Charette + + * pph-streamer-in.c (ALLOC_AND_REGISTER_ALTERNATE): Define. + (pph_in_binding_level): Add argument TO_REGISTER. Call + ALLOC_AND_REGISTER_ALTERNATE if set. + Update all users. + (pph_register_decls_in_symtab): Call varpool_finalize_decl + on all file-local symbols. + (pph_in_scope_chain): Call pph_in_binding_level with + scope_chain->bindings as the alternate pointer to + register in the streaming cache. + 2011-07-07 Diego Novillo * pph-streamer-in.c (pph_register_decls_in_symtab): Rename diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c index 571ebf5..903cd94 100644 --- a/gcc/cp/pph-streamer-in.c +++ b/gcc/cp/pph-streamer-in.c @@ -42,6 +42,18 @@ along with GCC; see the file COPYING3. If not see pph_register_shared_data (STREAM, DATA, IX); \ } while (0) +/* Same as ALLOC_AND_REGISTER, but instead of registering DATA into the + cache at slot IX, it registers ALT_DATA. Used to support mapping + pointers to global data in the original STREAM that need to point + to a different instance when aggregating individual PPH files into + the current translation unit (see pph_in_binding_level for an + example). */ +#define ALLOC_AND_REGISTER_ALTERNATE(STREAM, IX, DATA, ALLOC_EXPR, ALT_DATA)\ + do { \ + (DATA) = (ALLOC_EXPR); \ + pph_register_shared_data (STREAM, ALT_DATA, IX); \ + } while (0) + /* Callback for unpacking value fields in ASTs. BP is the bitpack we are unpacking from. EXPR is the tree to unpack. */ @@ -482,7 +494,8 @@ pph_in_qual_use_vec (pph_stream *stream) /* Forward declaration to break cyclic dependencies. */ -static struct cp_binding_level *pph_in_binding_level (pph_stream *); +static struct cp_binding_level *pph_in_binding_level (pph_stream *, + struct cp_binding_level *); /* Helper for pph_in_cxx_binding. Read and return a cxx_binding instance from STREAM. */ @@ -505,7 +518,7 @@ pph_in_cxx_binding_1 (pph_stream *stream) value = pph_in_tree (stream); type = pph_in_tree (stream); ALLOC_AND_REGISTER (stream, ix, cb, cxx_binding_make (value, type)); - cb->scope = pph_in_binding_level (stream); + cb->scope = pph_in_binding_level (stream, NULL); bp = pph_in_bitpack (stream); cb->value_is_inherited = bp_unpack_value (&bp, 1); cb->is_local = bp_unpack_value (&bp, 1); @@ -581,10 +594,26 @@ pph_in_label_binding (pph_stream *stream) } -/* Read and return an instance of cp_binding_level from STREAM. */ +/* Read and return an instance of cp_binding_level from STREAM. + TO_REGISTER is used when the caller wants to read a binding level, + but register a different binding level in the streaming cache. + This is needed when reading the binding for global_namespace. + + When the file was created global_namespace was localized to that + particular file. However, when instantiating a PPH file into + memory, we are now using the global_namespace for the current + translation unit. Therefore, every reference to the + global_namespace and its bindings should be pointing to the + CURRENT global namespace, not the one in STREAM. + + Therefore, if TO_REGISTER is set, and we read a binding level from + STREAM for the first time, instead of registering the newly + instantiated binding level into the cache, we register the binding + level given in TO_REGISTER. This way, subsequent references to the + global binding level will be done to the one set in TO_REGISTER. */ static struct cp_binding_level * -pph_in_binding_level (pph_stream *stream) +pph_in_binding_level (pph_stream *stream, struct cp_binding_level *to_register) { unsigned i, num, ix; struct cp_binding_level *bl; @@ -597,7 +626,14 @@ pph_in_binding_level (pph_stream *stream) else if (marker == PPH_RECORD_SHARED) return (struct cp_binding_level *) pph_in_shared_data (stream, ix); - ALLOC_AND_REGISTER (stream, ix, bl, ggc_alloc_cleared_cp_binding_level ()); + /* If TO_REGISTER is set, register that binding level instead of the newly + allocated binding level into slot IX. */ + if (to_register == NULL) + ALLOC_AND_REGISTER (stream, ix, bl, ggc_alloc_cleared_cp_binding_level ()); + else + ALLOC_AND_REGISTER_ALTERNATE (stream, ix, bl, + ggc_alloc_cleared_cp_binding_level (), + to_register); bl->names = pph_in_chain (stream); bl->namespaces = pph_in_chain (stream); @@ -627,7 +663,7 @@ pph_in_binding_level (pph_stream *stream) bl->blocks = pph_in_chain (stream); bl->this_entity = pph_in_tree (stream); - bl->level_chain = pph_in_binding_level (stream); + bl->level_chain = pph_in_binding_level (stream, NULL); bl->dead_vars_from_for = pph_in_tree_vec (stream); bl->statement_list = pph_in_chain (stream); bl->binding_depth = pph_in_uint (stream); @@ -713,7 +749,7 @@ pph_in_language_function (pph_stream *stream) /* FIXME pph. We are not reading lf->x_named_labels. */ - lf->bindings = pph_in_binding_level (stream); + lf->bindings = pph_in_binding_level (stream, NULL); lf->x_local_names = pph_in_tree_vec (stream); /* FIXME pph. We are not reading lf->extern_decl_map. */ @@ -854,7 +890,7 @@ pph_in_struct_function (pph_stream *stream, tree decl) static void pph_in_ld_ns (pph_stream *stream, struct lang_decl_ns *ldns) { - ldns->level = pph_in_binding_level (stream); + ldns->level = pph_in_binding_level (stream, NULL); } @@ -1140,10 +1176,10 @@ pph_in_lang_type (pph_stream *stream) /* Register all the symbols in binding level BL in the callgraph symbol - table. NS is the namespace where all the symbols in BL live. */ + table. */ static void -pph_register_decls_in_symtab (struct cp_binding_level *bl, tree ns) +pph_register_decls_in_symtab (struct cp_binding_level *bl) { tree t; @@ -1153,18 +1189,16 @@ pph_register_decls_in_symtab (struct cp_binding_level *bl, tree ns) bl->namespaces = nreverse (bl->namespaces); for (t = bl->names; t; t = DECL_CHAIN (t)) - if (DECL_NAME (t) && IDENTIFIER_NAMESPACE_BINDINGS (DECL_NAME (t))) - { - cxx_binding *b = IDENTIFIER_NAMESPACE_BINDINGS (DECL_NAME (t)); - b->scope = NAMESPACE_LEVEL (ns); - - if (TREE_CODE (t) == VAR_DECL && TREE_STATIC (t) && !DECL_EXTERNAL (t)) - varpool_finalize_decl (t); - } + { + /* Add file-local symbols to the varpool. */ + if (TREE_CODE (t) == VAR_DECL && TREE_STATIC (t) && !DECL_EXTERNAL (t)) + varpool_finalize_decl (t); + } + /* Recurse into the namespaces contained in BL. */ for (t = bl->namespaces; t; t = DECL_CHAIN (t)) if (NAMESPACE_LEVEL (t)) - pph_register_decls_in_symtab (NAMESPACE_LEVEL (t), t); + pph_register_decls_in_symtab (NAMESPACE_LEVEL (t)); } @@ -1173,18 +1207,21 @@ pph_register_decls_in_symtab (struct cp_binding_level *bl, tree ns) static void pph_in_scope_chain (pph_stream *stream) { - struct saved_scope *file_scope_chain; unsigned i; tree decl; cp_class_binding *cb; cp_label_binding *lb; struct cp_binding_level *cur_bindings, *new_bindings; - file_scope_chain = ggc_alloc_cleared_saved_scope (); - file_scope_chain->bindings = new_bindings = pph_in_binding_level (stream); + /* When reading the symbols in STREAM's global binding level, make + sure that references to the global binding level point to + scope_chain->bindings. Otherwise, identifiers read from STREAM + will have the wrong bindings and will fail name lookups. */ cur_bindings = scope_chain->bindings; + new_bindings = pph_in_binding_level (stream, scope_chain->bindings); - pph_register_decls_in_symtab (new_bindings, global_namespace); + /* Register all the symbols in STREAM with the call graph. */ + pph_register_decls_in_symtab (new_bindings); /* Merge the bindings from STREAM into saved_scope->bindings. */ chainon (cur_bindings->names, new_bindings->names);