From patchwork Fri Sep 30 04:09:32 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andi Kleen X-Patchwork-Id: 117020 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 715241007D8 for ; Fri, 30 Sep 2011 14:10:08 +1000 (EST) Received: (qmail 23457 invoked by alias); 30 Sep 2011 04:10:02 -0000 Received: (qmail 23029 invoked by uid 22791); 30 Sep 2011 04:09:59 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mga03.intel.com (HELO mga03.intel.com) (143.182.124.21) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 30 Sep 2011 04:09:43 +0000 Received: from azsmga002.ch.intel.com ([10.2.17.35]) by azsmga101.ch.intel.com with ESMTP; 29 Sep 2011 21:09:41 -0700 X-ExtLoop1: 1 Received: from unknown (HELO localhost) ([10.255.14.179]) by AZSMGA002.ch.intel.com with ESMTP; 29 Sep 2011 21:09:38 -0700 Date: Thu, 29 Sep 2011 21:09:32 -0700 From: Andi Kleen To: Diego Novillo Cc: "H.J. Lu" , "H.J. Lu" , gcc-patches@gcc.gnu.org Subject: Re: PATCH: PR lto/50568: [4.7 Regression] Massive LTO failures Message-ID: <20110930040931.GA13480@alboin.amr.corp.intel.com> References: <20110930005702.GA15754@intel.com> <4E852B8F.3000509@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4E852B8F.3000509@google.com> User-Agent: Mutt/1.5.20 (2009-06-14) 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 I updated the patch according to your comments. Ok now? -Andi gcc/lto/: 2011-09-29 H.J. Lu Andi Kleen PR lto/50568 * lto.c (lto_splay_tree_delete_id): New. (lto_splay_tree_compare_ids): Likewise. (lto_splay_tree_lookup): Likewise. (lto_splay_tree_id_equal_p): Likewise. (lto_splay_tree_insert): Likewise. (lto_splay_tree_new): Likewise. (lto_resolution_read): Change id to unsigned HOST_WIDE_INT. Use lto_splay_tree_id_equal_p and lto_splay_tree_lookup. (create_subid_section_table): Use lto_splay_tree_lookup and lto_splay_tree_insert. (lto_file_read): Use lto_splay_tree_new. lto-plugin/: 2011-09-29 H.J. Lu Andi Kleen PR lto/50568 * lto-plugin.c (sym_aux): Change id to unsigned long long. (plugin_symtab): Likewise. (dump_symtab): Likewise. (resolve_conflicts): Likewise. (process_symtab): Likewise. --- gcc/lto/lto.c | 84 ++++++++++++++++++++++++++++++++++++++++++---- lto-plugin/lto-plugin.c | 14 ++++--- 2 files changed, 84 insertions(+), 14 deletions(-) diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c index 77eb1a1..778e33e 100644 --- a/gcc/lto/lto.c +++ b/gcc/lto/lto.c @@ -93,6 +93,71 @@ lto_obj_create_section_hash_table (void) return htab_create (37, hash_name, eq_name, free_with_string); } +/* Delete an allocated integer KEY in the splay tree. */ + +static void +lto_splay_tree_delete_id (splay_tree_key key) +{ + free ((void *) key); +} + +/* Compare splay tree node ids A and B. */ + +static int +lto_splay_tree_compare_ids (splay_tree_key a, splay_tree_key b) +{ + unsigned HOST_WIDE_INT ai; + unsigned HOST_WIDE_INT bi; + + ai = *(unsigned HOST_WIDE_INT *) a; + bi = *(unsigned HOST_WIDE_INT *) b; + + if (ai < bi) + return -1; + else if (ai > bi) + return 1; + return 0; +} + +/* Look up splay tree node by ID in splay tree T. */ + +static splay_tree_node +lto_splay_tree_lookup (splay_tree t, unsigned HOST_WIDE_INT id) +{ + return splay_tree_lookup (t, (splay_tree_key) &id); +} + +/* Check if KEY has ID. */ + +static bool +lto_splay_tree_id_equal_p (splay_tree_key key, unsigned HOST_WIDE_INT id) +{ + return *(unsigned HOST_WIDE_INT *) key == id; +} + +/* Insert a splay tree node into tree T with ID as key and FILE_DATA as value. + The ID is allocated separately because we need HOST_WIDE_INTs which may + be wider than a splay_tree_key. */ + +static void +lto_splay_tree_insert (splay_tree t, unsigned HOST_WIDE_INT id, + struct lto_file_decl_data *file_data) +{ + unsigned HOST_WIDE_INT *idp = XCNEW (unsigned HOST_WIDE_INT); + *idp = id; + splay_tree_insert (t, (splay_tree_key) idp, (splay_tree_value) file_data); +} + +/* Create a splay tree. */ + +static splay_tree +lto_splay_tree_new (void) +{ + return splay_tree_new (lto_splay_tree_compare_ids, + lto_splay_tree_delete_id, + NULL); +} + /* Read the constructors and inits. */ static void @@ -944,14 +1009,16 @@ lto_resolution_read (splay_tree file_ids, FILE *resolution, lto_file *file) for (i = 0; i < num_symbols; i++) { int t; - unsigned index, id; + unsigned index; + unsigned HOST_WIDE_INT id; char r_str[27]; enum ld_plugin_symbol_resolution r = (enum ld_plugin_symbol_resolution) 0; unsigned int j; unsigned int lto_resolution_str_len = sizeof (lto_resolution_str) / sizeof (char *); - t = fscanf (resolution, "%u %x %26s %*[^\n]\n", &index, &id, r_str); + t = fscanf (resolution, "%u " HOST_WIDE_INT_PRINT_HEX_PURE " %26s %*[^\n]\n", + &index, &id, r_str); if (t != 3) internal_error ("invalid line in the resolution file"); if (index > max_index) @@ -968,11 +1035,12 @@ lto_resolution_read (splay_tree file_ids, FILE *resolution, lto_file *file) if (j == lto_resolution_str_len) internal_error ("invalid resolution in the resolution file"); - if (!(nd && nd->key == id)) + if (!(nd && lto_splay_tree_id_equal_p (nd->key, id))) { - nd = splay_tree_lookup (file_ids, id); + nd = lto_splay_tree_lookup (file_ids, id); if (nd == NULL) - internal_error ("resolution sub id %x not in object file", id); + internal_error ("resolution sub id " HOST_WIDE_INT_PRINT_HEX_PURE + " not in object file", id); } file_data = (struct lto_file_decl_data *)nd->value; @@ -1015,7 +1083,7 @@ create_subid_section_table (void **slot, void *data) return 1; /* Find hash table of sub module id */ - nd = splay_tree_lookup (file_ids, id); + nd = lto_splay_tree_lookup (file_ids, id); if (nd != NULL) { file_data = (struct lto_file_decl_data *)nd->value; @@ -1026,7 +1094,7 @@ create_subid_section_table (void **slot, void *data) memset(file_data, 0, sizeof (struct lto_file_decl_data)); file_data->id = id; file_data->section_hash_table = lto_obj_create_section_hash_table ();; - splay_tree_insert (file_ids, id, (splay_tree_value)file_data); + lto_splay_tree_insert (file_ids, id, file_data); } /* Copy section into sub module hash table */ @@ -1104,7 +1172,7 @@ lto_file_read (lto_file *file, FILE *resolution_file, int *count) /* Find all sub modules in the object and put their sections into new hash tables in a splay tree. */ - file_ids = splay_tree_new (splay_tree_compare_ints, NULL, NULL); + file_ids = lto_splay_tree_new (); htab_traverse (section_hash_table, create_subid_section_table, file_ids); /* Add resolutions to file ids */ diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c index 4b5828b..9323bd2 100644 --- a/lto-plugin/lto-plugin.c +++ b/lto-plugin/lto-plugin.c @@ -80,12 +80,13 @@ along with this program; see the file COPYING3. If not see /* The part of the symbol table the plugin has to keep track of. Note that we must keep SYMS until all_symbols_read is called to give the linker time to - copy the symbol information. */ + copy the symbol information. + The id must be 64bit to minimze collisions. */ struct sym_aux { uint32_t slot; - unsigned id; + unsigned long long id; unsigned next_conflict; }; @@ -94,7 +95,7 @@ struct plugin_symtab int nsyms; struct sym_aux *aux; struct ld_plugin_symbol *syms; - unsigned id; + unsigned long long id; }; /* Encapsulates object file data during symbol scan. */ @@ -359,7 +360,8 @@ dump_symtab (FILE *f, struct plugin_symtab *symtab) assert (resolution != LDPR_UNKNOWN); - fprintf (f, "%u %x %s %s\n", (unsigned int) slot, symtab->aux[j].id, + fprintf (f, "%u %llx %s %s\n", + (unsigned int) slot, symtab->aux[j].id, lto_resolution_str[resolution], symtab->syms[j].name); } @@ -759,7 +761,7 @@ resolve_conflicts (struct plugin_symtab *t, struct plugin_symtab *conflicts) { SWAP (struct ld_plugin_symbol, *orig, *s); SWAP (uint32_t, orig_aux->slot, aux->slot); - SWAP (unsigned, orig_aux->id, aux->id); + SWAP (unsigned long long, orig_aux->id, aux->id); /* Don't swap conflict chain pointer */ } @@ -809,7 +811,7 @@ process_symtab (void *data, const char *name, off_t offset, off_t length) s = strrchr (name, '.'); if (s) - sscanf (s, ".%x", &obj->out->id); + sscanf (s, ".%llx", &obj->out->id); secdata = xmalloc (length); offset += obj->file->offset; if (offset != lseek (obj->file->fd, offset, SEEK_SET)