From patchwork Wed Sep 3 17:54:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 385610 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 78144140274 for ; Thu, 4 Sep 2014 03:55:25 +1000 (EST) 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:references :in-reply-to:content-type; q=dns; s=default; b=PeSgKKddu7BicjYwv gCHsF2aqXg+L7x8jo0yhYorSRwxE6bHkVANe+hm/i4i8+UtMAXuy0aQzcGzpiQG7 Qt1EDjC4sTFqp2LNK+0VoI8L2ckb+U5L0q2dB9JxW1aftVOdsO2GaABFose/5wiT bF+424brheRv3E0DFb6yFepuMI= 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:references :in-reply-to:content-type; s=default; bh=zEUeBZVzIDHgQoCKvS2NN/p czUk=; b=vfx46GdG9Rdw3fbtYCx0OzdRd2lyIBSaanfd239y0ESNLXAO/kCbmuj tmcT04jHi5+XNC6N3kOHhyDLNhn1jpg79umrAK6HEufOOyQYnDvptHHLXrR7PyvP ARQCXFPUX3UuFG+s2pHxWExDcsxkPKA0i7Hp+ahpM7DBwGZC6ki4= Received: (qmail 10966 invoked by alias); 3 Sep 2014 17:55:01 -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 10956 invoked by uid 89); 3 Sep 2014 17:55:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS 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; Wed, 03 Sep 2014 17:54:58 +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 s83Hsu4n010192 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 3 Sep 2014 13:54:57 -0400 Received: from reynosa.quesejoda.com (vpn-61-4.rdu2.redhat.com [10.10.61.4]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s83Hstj0014364; Wed, 3 Sep 2014 13:54:55 -0400 Message-ID: <540755E5.7060602@redhat.com> Date: Wed, 03 Sep 2014 10:54:45 -0700 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: Jason Merrill , Richard Biener CC: gcc-patches Subject: Re: [debug-early] reuse variable DIEs and fix their context References: <53FD45A7.4000804@redhat.com> <53FF6840.9030505@redhat.com> <53FF6E61.6030507@redhat.com> In-Reply-To: <53FF6E61.6030507@redhat.com> [Jason, Richard]: Is it useful for my patches to contain ChangeLog entries? I find them mildly annoying for something that will inevitably be rewritten multiple times, but if it aids in reviewing my WIP, I am more than happy to continue including them. On 08/28/14 11:01, Jason Merrill wrote: > On 08/28/2014 01:34 PM, Aldy Hernandez wrote: >> I wonder if instead of early dumping of all the DECLs, we could only >> dump the toplevel scoped DECLs, and let inheritance set the proper >> contexts. > > Yes, I think this makes a lot more sense; do it at a well-defined point > in compilation rather than as part of free_lang_data. Great. It turned out, this was a cleaner approach as well. >> The problem being that to calculate `ext_block' above, we need intimate >> knowledge of scopes and such, only available in the FE. Is there a >> generic way of determining if a DECL is in global scope? > > Why not do it in the FE, i.e. *_write_global_declarations? This is what I've done in this patch. I'm no longer generating dwarf early from free_lang_data, instead I'm using the global_decl debug hook and adding an EARLY argument. Then I call it twice, once after the FE is done, and once after the full compilation has finished (cgraph has been generated, etc). The goal is to have the first pass generate the DIEs and the 2nd pass fill in location information and such. Generating the globals first solves the context issue. The recursive nature of generating DIEs gets everything right. For that matter, with the attached patch, I actually get *LESS* guality failures than before. Unexpected, but I'm not going to complain ;-). I have added a few (temporary) checks to make sure we're not regenerating DIEs when we already have one (at least for DECLs). These should go away after this work is incorporated into mainline. FYI, I am only handling C for now while we iron out the general idea. How does this look? Aldy diff --git a/gcc/ChangeLog.debug-early b/gcc/ChangeLog.debug-early index df571a4..980b655 100644 --- a/gcc/ChangeLog.debug-early +++ b/gcc/ChangeLog.debug-early @@ -1,3 +1,31 @@ +2014-09-03 Aldy Hernandez + + * c/c-decl.c (write_global_declarations_1): Call global_decl() + with early=true. + (write_global_declarations_2): Call global_decl() with + early=false. + * dbxout.c (dbxout_global_decl): New argument. + * debug.c (do_nothing_debug_hooks): Use debug_nothing_tree_bool + for global_decl hook. + (debug_nothing_tree_bool): New. + (struct gcc_debug_hooks): New argument to global_decl. + * dwarf2out.c (output_die): Add misc debugging information. + (gen_variable_die): Do not reparent children. + (dwarf2out_global_decl): Add new documentation. Add EARLY + argument. + (dwarf2out_decl): Make sure we don't generate new DIEs if we + already have a DIE. + * cp/name-lookup.c (do_namespace_alias): New argument to + global_decl debug hook. + * fortran/trans-decl.c (gfc_emit_parameter_debug_info): Same. + * godump.c (go_global_decl): Same. + * lto/lto-lang.c (lto_write_globals): Same. + * sdbout.c (sdbout_global_decl): Same. + * toplev.c (emit_debug_global_declarations): Same. + * vmsdbgout.c (vmsdbgout_global_decl): Same. + * tree.c (free_lang_data_in_decl): Do not call + dwarf2out_early_decl from here. + 2014-08-26 Aldy Hernandez * dwarf2out.c (struct die_struct): Add dumped_early field. diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index b4995a6..1e09404 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -10308,7 +10308,10 @@ c_write_global_declarations_1 (tree globals) while (reconsider); for (decl = globals; decl; decl = DECL_CHAIN (decl)) - check_global_declaration_1 (decl); + { + check_global_declaration_1 (decl); + debug_hooks->global_decl (decl, /*early=*/true); + } } /* A subroutine of c_write_global_declarations Emit debug information for each @@ -10320,7 +10323,7 @@ c_write_global_declarations_2 (tree globals) tree decl; for (decl = globals; decl ; decl = DECL_CHAIN (decl)) - debug_hooks->global_decl (decl); + debug_hooks->global_decl (decl, /*early=*/false); } /* Callback to collect a source_ref from a DECL. */ diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index ebcbb5c..45b3b99 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -3859,7 +3859,7 @@ do_namespace_alias (tree alias, tree name_space) /* Emit debug info for namespace alias. */ if (!building_stmt_list_p ()) - (*debug_hooks->global_decl) (alias); + (*debug_hooks->global_decl) (alias, /*early=*/false); } /* Like pushdecl, only it places X in the current namespace, diff --git a/gcc/dbxout.c b/gcc/dbxout.c index d856bdd..208cec9 100644 --- a/gcc/dbxout.c +++ b/gcc/dbxout.c @@ -325,7 +325,7 @@ static int dbxout_symbol_location (tree, tree, const char *, rtx); static void dbxout_symbol_name (tree, const char *, int); static void dbxout_common_name (tree, const char *, stab_code_type); static const char *dbxout_common_check (tree, int *); -static void dbxout_global_decl (tree); +static void dbxout_global_decl (tree, bool); static void dbxout_type_decl (tree, int); static void dbxout_handle_pch (unsigned); static void debug_free_queue (void); @@ -1320,7 +1320,7 @@ dbxout_function_decl (tree decl) /* Debug information for a global DECL. Called from toplev.c after compilation proper has finished. */ static void -dbxout_global_decl (tree decl) +dbxout_global_decl (tree decl, bool early ATTRIBUTE_UNUSED) { if (TREE_CODE (decl) == VAR_DECL && !DECL_EXTERNAL (decl)) { diff --git a/gcc/debug.c b/gcc/debug.c index dba068c..b5818de 100644 --- a/gcc/debug.c +++ b/gcc/debug.c @@ -43,7 +43,7 @@ const struct gcc_debug_hooks do_nothing_debug_hooks = debug_nothing_tree, /* begin_function */ debug_nothing_int, /* end_function */ debug_nothing_tree, /* function_decl */ - debug_nothing_tree, /* global_decl */ + debug_nothing_tree_bool, /* global_decl */ debug_nothing_tree_int, /* type_decl */ debug_nothing_tree_tree_tree_bool, /* imported_module_or_decl */ debug_nothing_tree, /* deferred_inline_function */ @@ -71,6 +71,12 @@ debug_nothing_tree (tree decl ATTRIBUTE_UNUSED) } void +debug_nothing_tree_bool (tree decl ATTRIBUTE_UNUSED, + bool early ATTRIBUTE_UNUSED) +{ +} + +void debug_nothing_tree_tree (tree t1 ATTRIBUTE_UNUSED, tree t2 ATTRIBUTE_UNUSED) { diff --git a/gcc/debug.h b/gcc/debug.h index 28bc210..e89a9e8 100644 --- a/gcc/debug.h +++ b/gcc/debug.h @@ -93,8 +93,11 @@ struct gcc_debug_hooks void (* function_decl) (tree decl); /* Debug information for a global DECL. Called from toplev.c after - compilation proper has finished. */ - void (* global_decl) (tree decl); + compilation proper has finished. EARLY is true if global_decl() + is being called early on in the compilation process (i.e., before + cgraph information is available and before full location + information is available). */ + void (* global_decl) (tree decl, bool early); /* Debug information for a type DECL. Called from toplev.c after compilation proper, also from various language front ends to @@ -156,6 +159,7 @@ extern void debug_nothing_int_charstar_int_bool (unsigned int, const char *, extern void debug_nothing_int (unsigned int); extern void debug_nothing_int_int (unsigned int, unsigned int); extern void debug_nothing_tree (tree); +extern void debug_nothing_tree_bool (tree, bool); extern void debug_nothing_tree_tree (tree, tree); extern void debug_nothing_tree_int (tree, int); extern void debug_nothing_tree_tree_tree_bool (tree, tree, tree, bool); diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 5859228..b577757 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2430,7 +2430,7 @@ static void dwarf2out_function_decl (tree); static void dwarf2out_begin_block (unsigned, unsigned); static void dwarf2out_end_block (unsigned, unsigned); static bool dwarf2out_ignore_block (const_tree); -static void dwarf2out_global_decl (tree); +static void dwarf2out_global_decl (tree, bool); static void dwarf2out_type_decl (tree, int); static void dwarf2out_imported_module_or_decl (tree, tree, tree, bool); static void dwarf2out_imported_module_or_decl_1 (tree, tree, tree, @@ -8711,10 +8711,11 @@ output_die (dw_die_ref die) if (! die->comdat_type_p && die->die_id.die_symbol) output_die_symbol (die); - dw2_asm_output_data_uleb128 (die->die_abbrev, "(DIE (%#lx) %s (parent DIE=%#lx))", + dw2_asm_output_data_uleb128 (die->die_abbrev, "(DIE (%#lx) %s (parent DIE=%#lx) early=%d)", (unsigned long)die->die_offset, dwarf_tag_name (die->die_tag), - die->die_parent ? die->die_parent->die_offset : 0); + die->die_parent ? die->die_parent->die_offset : 0, + die->dumped_early); FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a) { @@ -19009,13 +19010,11 @@ gen_variable_die (tree decl, tree origin, dw_die_ref context_die) if (old_die && !declaration && !local_scope_p (context_die)) return; - /* When DIEs are created early, the context is the compilation unit. - Adjust the context when we know what it is the second time - around. */ + /* If a DIE was dumped early, it still needs location info. Skip to + the part where we fill the location bits. */ if (old_die && old_die->dumped_early) { - if (old_die->die_parent != context_die) - reparent_child (old_die, context_die); + gcc_assert (old_die->die_parent == context_die); var_die = old_die; old_die = NULL; goto gen_variable_die_location; @@ -20818,12 +20817,29 @@ gen_decl_die (tree decl, tree origin, dw_die_ref context_die) return NULL; } -/* Output debug information for global decl DECL. Called from toplev.c after - compilation proper has finished. */ +/* Output debug information for global decl DECL. Called from + toplev.c after compilation proper has finished. + + dwarf2out_decl() will be called twice on each global symbol: once + immediately after parsing (EARLY=true), and once after the full + compilation has finished (EARLY=false). There are checks in + dwarf2out_decl() to make sure that if we have a DECL DIE upon + entry, that the previously created DIE is reused. No new DECL DIEs + should be created when EARLY=false. + + The second time dwarf2out_decl() is called (or for that matter, the + second time any DECL DIE is seen throughout dwarf2out), only + information not previously available (e.g. location) is tacked onto + the early dumped DIE. That's the plan anyhow ;-). */ static void -dwarf2out_global_decl (tree decl) +dwarf2out_global_decl (tree decl, bool early) { + if (early) + { + dwarf2out_early_decl (decl); + return; + } /* Output DWARF2 information for file-scope tentative data object declarations, file-scope (extern) function declarations (which had no corresponding body) and file-scope tagged type declarations @@ -21019,6 +21035,19 @@ dwarf2out_decl (tree decl) { dw_die_ref context_die = comp_unit_die (); +#ifdef ENABLE_CHECKING + /* Save some info so we can later determine if we erroneously + created a DIE for something we had already created a DIE for. + We should always be reusing DIEs created early. */ + dw_die_ref early_die = NULL; + if (decl_die_table) + { + early_die = lookup_decl_die (decl); + if (early_die && !early_die->dumped_early) + early_die = NULL; + } +#endif + switch (TREE_CODE (decl)) { case ERROR_MARK: @@ -21144,6 +21173,11 @@ dwarf2out_decl (tree decl) dw_die_ref die = lookup_decl_die (decl); if (die) check_die (die, 0); +#ifdef ENABLE_CHECKING + /* If we early created a DIE, make sure it didn't get re-created by + mistake. */ + gcc_assert (!early_die || early_die == die); +#endif return die; } diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index 6afa6f3..38e6f99 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -4693,7 +4693,7 @@ gfc_emit_parameter_debug_info (gfc_symbol *sym) TREE_TYPE (decl), sym->attr.dimension, false, false); - debug_hooks->global_decl (decl); + debug_hooks->global_decl (decl, /*early=*/false); } diff --git a/gcc/godump.c b/gcc/godump.c index 7566f4d..01f8410 100644 --- a/gcc/godump.c +++ b/gcc/godump.c @@ -496,9 +496,9 @@ go_function_decl (tree decl) /* A global variable decl. */ static void -go_global_decl (tree decl) +go_global_decl (tree decl, bool early) { - real_debug_hooks->global_decl (decl); + real_debug_hooks->global_decl (decl, early); go_decl (decl); } diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c index 9e8524a..1f39949 100644 --- a/gcc/lto/lto-lang.c +++ b/gcc/lto/lto-lang.c @@ -1093,7 +1093,7 @@ lto_write_globals (void) varpool_node *vnode; FOR_EACH_DEFINED_VARIABLE (vnode) if (!decl_function_context (vnode->decl)) - debug_hooks->global_decl (vnode->decl); + debug_hooks->global_decl (vnode->decl, /*early=*/false); } static tree diff --git a/gcc/sdbout.c b/gcc/sdbout.c index 7b6f457..d81b184 100644 --- a/gcc/sdbout.c +++ b/gcc/sdbout.c @@ -119,7 +119,7 @@ static void sdbout_begin_block (unsigned int, unsigned int); static void sdbout_end_block (unsigned int, unsigned int); static void sdbout_source_line (unsigned int, const char *, int, bool); static void sdbout_end_epilogue (unsigned int, const char *); -static void sdbout_global_decl (tree); +static void sdbout_global_decl (tree, bool); static void sdbout_begin_prologue (unsigned int, const char *); static void sdbout_end_prologue (unsigned int, const char *); static void sdbout_begin_function (tree); @@ -142,7 +142,6 @@ static void sdbout_field_types (tree); static void sdbout_one_type (tree); static void sdbout_parms (tree); static void sdbout_reg_parms (tree); -static void sdbout_global_decl (tree); /* Random macros describing parts of SDB data. */ @@ -1422,7 +1421,7 @@ sdbout_reg_parms (tree parms) after compilation proper has finished. */ static void -sdbout_global_decl (tree decl) +sdbout_global_decl (tree decl, bool early ATTRIBUTE_UNUSED) { if (TREE_CODE (decl) == VAR_DECL && !DECL_EXTERNAL (decl) diff --git a/gcc/toplev.c b/gcc/toplev.c index 492a7ef..ceefa1b 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -532,7 +532,7 @@ emit_debug_global_declarations (tree *vec, int len) timevar_push (TV_SYMOUT); for (i = 0; i < len; i++) - debug_hooks->global_decl (vec[i]); + debug_hooks->global_decl (vec[i], /*early=*/false); timevar_pop (TV_SYMOUT); } diff --git a/gcc/tree.c b/gcc/tree.c index afc8e3d..8e9876e 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -5032,10 +5032,6 @@ free_lang_data_in_decl (tree decl) { gcc_assert (DECL_P (decl)); - /* Early dumping of DECLs before we lose language data. */ - if (debug_info_level > DINFO_LEVEL_NONE) - dwarf2out_early_decl (decl); - /* Give the FE a chance to remove its own data first. */ lang_hooks.free_lang_data (decl); diff --git a/gcc/vmsdbgout.c b/gcc/vmsdbgout.c index 463a418..2ad9e9b 100644 --- a/gcc/vmsdbgout.c +++ b/gcc/vmsdbgout.c @@ -163,7 +163,7 @@ static void vmsdbgout_begin_epilogue (unsigned int, const char *); static void vmsdbgout_end_epilogue (unsigned int, const char *); static void vmsdbgout_begin_function (tree); static void vmsdbgout_decl (tree); -static void vmsdbgout_global_decl (tree); +static void vmsdbgout_global_decl (tree, bool); static void vmsdbgout_type_decl (tree, int); static void vmsdbgout_abstract_function (tree); @@ -1510,10 +1510,10 @@ vmsdbgout_decl (tree decl) /* Not implemented in VMS Debug. */ static void -vmsdbgout_global_decl (tree decl) +vmsdbgout_global_decl (tree decl, bool early) { if (write_symbols == VMS_AND_DWARF2_DEBUG) - (*dwarf2_debug_hooks.global_decl) (decl); + (*dwarf2_debug_hooks.global_decl) (decl, early); } /* Not implemented in VMS Debug. */