From patchwork Thu Apr 30 23:28:21 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 466769 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id C9C60140320 for ; Fri, 1 May 2015 09:28:34 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass reason="1024-bit key; unprotected key" header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=EoLNrMzd; dkim-adsp=none (unprotected policy); dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; q=dns; s=default; b=pvpru2e9EV/ylwnBAm8/QuchlOhTT1SL7FlyzeQY6VN YWJMNpmbTcB8bWhLuQfMGO0IDesSO67BVOPH5GpfrTvwAmEN6wEGPzdzetx+B8c7 IX8GBaooPgyTQqkmPM2u4uueAqr4zIqaEndmH1JREhSyc5XXXQVRsSZZ6SDW8nCQ = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; s=default; bh=XXPTey81N9AokatQY3VQ4gDwAc8=; b=EoLNrMzdl9DyiM2Az zxd+IPlYMzMHa3wg3xJR6ryT9E4lzDa5LdJTABC31/1B2Mj9IF2k0GAfwfEdRfqN 3RYylw6j09ur6yms5g7hfoDAiSWAGGjbSY/9D1TWEXpSTiCDYKdTwuGfECA6ttDu p/1vD1SoUyWB7TWDC6L0VWAiTo= Received: (qmail 65060 invoked by alias); 30 Apr 2015 23:28:26 -0000 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 Received: (qmail 65050 invoked by uid 89); 30 Apr 2015 23:28:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 30 Apr 2015 23:28:24 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3UNSNt2009552 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 30 Apr 2015 19:28:23 -0400 Received: from reynosa.quesejoda.com (vpn-59-57.rdu2.redhat.com [10.10.59.57]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3UNSLxj020964; Thu, 30 Apr 2015 19:28:22 -0400 Message-ID: <5542BA95.5010106@redhat.com> Date: Thu, 30 Apr 2015 16:28:21 -0700 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: jason merrill , Jakub Jelinek CC: gcc-patches Subject: [debug-early] revert removal of deferred_asm_names Hello gentlemen. Both of you suggested I revert my removal of deferred_asm_names from dwarf2out, since it may cause mangling of DECLs that may ultimately not be needed. A little background... This all started with a regression for pr44197.c in the branch. What is happening is that the C front-end uses the presence of DECL_ASSEMBLER_NAME to determine if an __asm__ alias is already present for a symbol. Unfortunately, passing said symbol through dwarf2out_early_global_decl, triggers DECL_ASSEMBLER_NAME, so all symbols will have their assembler name set, and there is no way to diagnose multiple conflicting __asm__ aliases. Having thought about this some more, unless I'm misunderstanding something, I don't thing we'll cause mangling of DECLs that may not be eventually mangled. I mean, everything that ends up in deferred_asm_name, will ultimately end up in a DIE that will ultimately get a mangled DECL_ASSEMBLER_NAME. It's not like we're pruning unused DIEs _before_ we iterate through deferred_asm_name and generate DECL_ASSEMBLER_NAMEs for them (in mainline). For that matter, the original reason for deferred_asm_name was not to avoid mangling but to avoid template instantiation (something which Jason fixed in mainline as part of my original removal of deferred_asm_name). The original raison d'etre is here: https://gcc.gnu.org/ml/gcc-patches/2009-06/msg00030.html (Unless of course, another reason for deferred_asm_names was to avoid bloating memory usage earlier in the compilation process??? Although if we're going to generate the field, does it matter that much if we do it earlier?) Be that as it may, even if you agree with me, the pr44197.c regression needs to be addressed, and resurrecting deferred_asm_names does the trick of delaying setting DECL_ASSEMBLER_NAME until after the parser has finished. With this patch I have moved the flushing of deferred_asm_names to dwarf2out_early_finish instead of dwarf2out_finish, so it happens after the parser has finished, but does not live past LTO and upset Richi ;-). Could either of you comment on this, so I may at least add a relevant comment to the reason for deferred_asm_names (whether it exists to avoid parser insanity or to avoid unnecessary mangling)? Either way, I'm committing to the branch. Regressions down to 2. Aldy commit c6cb62c9e8fbd60d800ed3aa709a50b77fedf9c8 Author: Aldy Hernandez Date: Thu Apr 30 15:52:12 2015 -0700 Revert removal of deferred_asm_name and move its clearing to dwarf2out_early_finish. diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index f512ec7..c51cea1 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2806,6 +2806,10 @@ static GTY(()) comdat_type_node *comdat_type_list; /* A list of DIEs with a NULL parent waiting to be relocated. */ static GTY(()) limbo_die_node *limbo_die_list; +/* A list of DIEs for which we may have to generate + DW_AT_{,MIPS_}linkage_name once their DECL_ASSEMBLER_NAMEs are set. */ +static GTY(()) limbo_die_node *deferred_asm_name; + struct dwarf_file_hasher : ggc_hasher { typedef const char *compare_type; @@ -17237,9 +17241,22 @@ add_linkage_name (dw_die_ref die, tree decl) && (TREE_CODE (decl) == FUNCTION_DECL || TREE_CODE (decl) == VAR_DECL) && TREE_PUBLIC (decl) && !(TREE_CODE (decl) == VAR_DECL && DECL_REGISTER (decl)) - && die->die_tag != DW_TAG_member - && DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl)) - add_linkage_attr (die, decl); + && die->die_tag != DW_TAG_member) + { + /* Defer until we have an assembler name set. */ + if (!DECL_ASSEMBLER_NAME_SET_P (decl)) + { + limbo_die_node *asm_name; + + asm_name = ggc_cleared_alloc (); + asm_name->die = die; + asm_name->created_for = decl; + asm_name->next = deferred_asm_name; + deferred_asm_name = asm_name; + } + else if (DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl)) + add_linkage_attr (die, decl); + } } /* Add a DW_AT_name attribute and source coordinate attribute for the @@ -23882,6 +23899,37 @@ comdat_type_hasher::equal (const comdat_type_node *type_node_1, DWARF_TYPE_SIGNATURE_SIZE)); } +/* Move a DW_AT_{,MIPS_}linkage_name attribute just added to dw_die_ref + to the location it would have been added, should we know its + DECL_ASSEMBLER_NAME when we added other attributes. This will + probably improve compactness of debug info, removing equivalent + abbrevs, and hide any differences caused by deferring the + computation of the assembler name, triggered by e.g. PCH. */ + +static inline void +move_linkage_attr (dw_die_ref die) +{ + unsigned ix = vec_safe_length (die->die_attr); + dw_attr_node linkage = (*die->die_attr)[ix - 1]; + + gcc_assert (linkage.dw_attr == DW_AT_linkage_name + || linkage.dw_attr == DW_AT_MIPS_linkage_name); + + while (--ix > 0) + { + dw_attr_node *prev = &(*die->die_attr)[ix - 1]; + + if (prev->dw_attr == DW_AT_decl_line || prev->dw_attr == DW_AT_name) + break; + } + + if (ix != vec_safe_length (die->die_attr) - 1) + { + die->die_attr->pop (); + die->die_attr->quick_insert (ix, linkage); + } +} + /* Helper function for resolve_addr, mark DW_TAG_base_type nodes referenced from typed stack ops and count how often they are used. */ @@ -25059,7 +25107,7 @@ dwarf2out_finish (const char *filename) || !node->die->dumped_early); /* Flush out any latecomers to the limbo party. */ - dwarf2out_early_finish(); + dwarf2out_early_finish (); /* PCH might result in DW_AT_producer string being restored from the header compilation, so always fill it with empty string initially @@ -25407,6 +25455,25 @@ dwarf2out_finish (const char *filename) static void dwarf2out_early_finish (void) { + limbo_die_node *node, *next_node; + + /* Add DW_AT_linkage_name for all deferred DIEs. */ + for (node = deferred_asm_name; node; node = node->next) + { + tree decl = node->created_for; + /* When generating LTO bytecode we can not generate new assembler + names at this point and all important decls got theirs via + free-lang-data. */ + if (((!flag_generate_lto && !flag_generate_offload) + || DECL_ASSEMBLER_NAME_SET_P (decl)) + && DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl)) + { + add_linkage_attr (node->die, decl); + move_linkage_attr (node->die); + } + } + deferred_asm_name = NULL; + /* Traverse the limbo die list, and add parent/child links. The only dies without parents that should be here are concrete instances of inline functions, and the comp_unit_die. We can ignore the comp_unit_die. @@ -25415,7 +25482,6 @@ dwarf2out_early_finish (void) The point here is to flush out the limbo list so that it is empty and we don't need to stream it for LTO. */ - limbo_die_node *node, *next_node; for (node = limbo_die_list; node; node = next_node) { dw_die_ref die = node->die;