From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2799 invoked by alias); 26 Sep 2016 14:12:07 -0000 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 Received: (qmail 2776 invoked by uid 89); 26 Sep 2016 14:12:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.1 required=5.0 tests=BAYES_00,KAM_ASCII_DIVIDERS,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=Die X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 26 Sep 2016 14:11:56 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id F0609AAD1; Mon, 26 Sep 2016 14:11:50 +0000 (UTC) Date: Mon, 26 Sep 2016 14:30:00 -0000 From: Richard Biener To: gcc-patches@gcc.gnu.org cc: jason@redhat.com Subject: Re: [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish In-Reply-To: Message-ID: References: User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2016-09/txt/msg01837.txt.bz2 On Fri, 23 Sep 2016, Richard Biener wrote: > On Fri, 23 Sep 2016, Richard Biener wrote: > > > On Thu, 22 Sep 2016, Richard Biener wrote: > > > > > > > > This merges moving of unused type pruning from late to early finish as > > > well as handling of debug types and dwarf2 dups elimination. It adds > > > a flag to DIEs so we can mark them as removed in case sth after > > > early finish tries to lookup a DIE for a removed DIE again - we shouldn't > > > re-use the removed DIE (w/o parent) there. > > > > > > I suppose at some point we should re-think how pruning of "unused" > > > stuff is supposed to work. Given my grand plan is to get rid of > > > debug hooks and allow FEs direct control over the DWARF it should > > > be ultimatively their decision what to remove (err, not create, in > > > the first place). > > > > > > Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk? > > > > Testing this separately shows the need to port another hunk. If we > > prune types early we have to make sure to not re-create them late > > via typedefs in BLOCKs. > > > > Index: gcc/dwarf2out.c > > =================================================================== > > --- gcc/dwarf2out.c (revision 240363) > > +++ gcc/dwarf2out.c (working copy) > > @@ -23255,7 +23242,13 @@ process_scope_var (tree stmt, tree decl, > > die = lookup_decl_die (decl_or_origin); > > else if (TREE_CODE (decl_or_origin) == TYPE_DECL > > && TYPE_DECL_IS_STUB (decl_or_origin)) > > - die = lookup_type_die (TREE_TYPE (decl_or_origin)); > > + { > > + /* Avoid re-creating the DIE late if it was optimized > > + as unused early (this BLOCK reference does not count as "use"). > > */ > > + die = lookup_type_die (TREE_TYPE (decl_or_origin)); > > + if (! die) > > + return; > > + } > > else > > die = NULL; > > Testing went fine if you consider moving > > + /* Generate separate CUs for each of the include files we've seen. > + They will go into limbo_die_list. */ > + if (flag_eliminate_dwarf2_dups) > + break_out_includes (comp_unit_die ()); > > not being part of this patch. I did statistics over patched/unpatched > GCC by means of > > readelf -wi gcc/.o | grep 'DW_TAG_[_a-z]*type' | wc -l > > and the difference is in the noise (diff unpatched patched): > > -gcc/alias.o 2639 > +gcc/alias.o 2640 > -gcc/auto-profile.o 4941 > -gcc/bb-reorder.o 2243 > +gcc/auto-profile.o 4945 > +gcc/bb-reorder.o 2315 > -gcc/bt-load.o 1837 > +gcc/bt-load.o 1836 > -gcc/cfg.o 1350 > +gcc/cfg.o 1349 > ... > > There are DW_TAG_typedef no longer occuring (from vec.o): > > <1>: Abbrev Number: 180 (DW_TAG_subprogram) > DW_AT_specification: <0x6a3a> > DW_AT_inline : 3 (declared as inline and inlined) > DW_AT_sibling : <0xe7ff> > <2>: Abbrev Number: 63 (DW_TAG_formal_parameter) > DW_AT_name : (indirect string, offset: 0x3f48): alloc > DW_AT_decl_file : 5 > DW_AT_decl_line : 1050 > DW_AT_type : <0x6d> > <2>: Abbrev Number: 51 (DW_TAG_typedef) > DW_AT_name : (indirect string, offset: 0x1435f): > vec_embedde > d > DW_AT_decl_file : 5 > DW_AT_decl_line : 1052 > DW_AT_type : <0x665a> > <2>: Abbrev Number: 0 > > The DW_TAG_typedef is missing in the patched variant. This DIE is > not refered to in the the unpatched variant (I didn't check why we > don't prune it). > > There appear some extra stray DW_TAG_typedef DIEs though: > > <13><11a28>: Abbrev Number: 207 (DW_TAG_typedef) > <13><11a2a>: Abbrev Number: 0 > > I'll check where those come from. Ok, so the former is from premark_used_types having no effect for early debug as it is called in gen_subprogram_die before we had a chance to generate type DIEs from its blocks. The fix is to simply move it until after that. The latter is because we re-create those in place of the removed ones in process_scope_var so the fix is to extend the mitigation that was added for the TYPE_DECL_IS_STUB to all types (or type decls). Note this change is also pending from Pierre-Marie to force re-parenting of existing TYPE_DECLs for Ada. With those fixes the stats for all stage3 gcc/*.o files look like -gcc/gcov-tool.o 506 +gcc/gcov-tool.o 512 -gcc/graphite-isl-ast-to-gimple.o 2262 +gcc/graphite-isl-ast-to-gimple.o 2264 -gcc/plugin.o 648 +gcc/plugin.o 650 those are new DIEs per premark_types_used_by_global_vars_helper due to the same issue we have more DIEs early now -- optimization didn't get a chance to remove anything but trivially unreachable stuff. I believe we did not yet address that "regression" caused by the early debug merge in any way (and this adds a tiny bit to it). Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk? Thanks, Richard. 2016-09-26 Richard Biener * dwarf2out.c (struct die_struct): Add removed flag. (lookup_type_die): If the DIE is marked as removed, clear TYPE_SYMTAB_DIE and return NULL. (lookup_decl_die): If the DIE is marked as removed, remove it from the hash and return NULL. (mark_removed): New helper. (prune_unused_types_prune): Call it for removed DIEs. (gen_subprogram_die): Move the premark_used_types call to after DIEs for the functions scopes are generated. (process_scope_var): Do not re-create pruned types or type decls. Make sure to also re-parent type decls. (dwarf2out_finish): Move unused type pruning and debug_types handling ... (dwarf2out_early_finish): ... here. Index: gcc/dwarf2out.c =================================================================== --- gcc/dwarf2out.c (revision 240494) +++ gcc/dwarf2out.c (working copy) @@ -2687,6 +2687,10 @@ typedef struct GTY((chain_circular ("%h. /* Die is used and must not be pruned as unused. */ BOOL_BITFIELD die_perennial_p : 1; BOOL_BITFIELD comdat_type_p : 1; /* DIE has a type signature */ + /* Whether this DIE was removed from the DIE tree, for example via + prune_unused_types. We don't consider those present from the + DIE lookup routines. */ + BOOL_BITFIELD removed : 1; /* Lots of spare bits. */ } die_node; @@ -5066,7 +5070,13 @@ new_die (enum dwarf_tag tag_value, dw_di static inline dw_die_ref lookup_type_die (tree type) { - return TYPE_SYMTAB_DIE (type); + dw_die_ref die = TYPE_SYMTAB_DIE (type); + if (die && die->removed) + { + TYPE_SYMTAB_DIE (type) = NULL; + return NULL; + } + return die; } /* Given a TYPE_DIE representing the type TYPE, if TYPE is an @@ -5131,7 +5141,16 @@ decl_die_hasher::equal (die_node *x, tre static inline dw_die_ref lookup_decl_die (tree decl) { - return decl_die_table->find_with_hash (decl, DECL_UID (decl)); + dw_die_ref *die = decl_die_table->find_slot_with_hash (decl, DECL_UID (decl), + NO_INSERT); + if (!die) + return NULL; + if ((*die)->removed) + { + decl_die_table->clear_slot (die); + return NULL; + } + return *die; } /* Returns a hash value for X (which really is a var_loc_list). */ @@ -20415,8 +20434,6 @@ gen_subprogram_die (tree decl, dw_die_re int declaration = (current_function_decl != decl || class_or_namespace_scope_p (context_die)); - premark_used_types (DECL_STRUCT_FUNCTION (decl)); - /* Now that the C++ front end lazily declares artificial member fns, we might need to retrofit the declaration into its class. */ if (!declaration && !origin && !old_die @@ -21138,6 +21155,9 @@ gen_subprogram_die (tree decl, dw_die_re call_site_count = -1; tail_call_site_count = -1; } + + /* Mark used types after we have created DIEs for the functions scopes. */ + premark_used_types (DECL_STRUCT_FUNCTION (decl)); } /* Returns a hash value for X (which really is a die_struct). */ @@ -23221,9 +23241,16 @@ process_scope_var (tree stmt, tree decl, if (TREE_CODE (decl_or_origin) == FUNCTION_DECL) die = lookup_decl_die (decl_or_origin); - else if (TREE_CODE (decl_or_origin) == TYPE_DECL - && TYPE_DECL_IS_STUB (decl_or_origin)) - die = lookup_type_die (TREE_TYPE (decl_or_origin)); + else if (TREE_CODE (decl_or_origin) == TYPE_DECL) + { + if (TYPE_DECL_IS_STUB (decl_or_origin)) + die = lookup_type_die (TREE_TYPE (decl_or_origin)); + else + die = lookup_decl_die (decl_or_origin); + /* Avoid re-creating the DIE late if it was optimized as unused early. */ + if (! die && ! early_dwarf) + return; + } else die = NULL; @@ -26176,6 +26203,16 @@ prune_unused_types_update_strings (dw_di } } +/* Mark DIE and its children as removed. */ + +static void +mark_removed (dw_die_ref die) +{ + dw_die_ref c; + die->removed = true; + FOR_EACH_CHILD (die, c, mark_removed (c)); +} + /* Remove from the tree DIE any dies that aren't marked. */ static void @@ -26205,12 +26242,14 @@ prune_unused_types_prune (dw_die_ref die die->die_child = prev; } c->die_sib = NULL; + mark_removed (c); return; } else { next = c->die_sib; c->die_sib = NULL; + mark_removed (c); } if (c != prev->die_sib) @@ -27816,32 +27855,6 @@ dwarf2out_finish (const char *) resolve_addr (comp_unit_die ()); move_marked_base_types (); - if (flag_eliminate_unused_debug_types) - prune_unused_types (); - - /* Generate separate COMDAT sections for type DIEs. */ - if (use_debug_types) - { - break_out_comdat_types (comp_unit_die ()); - - /* Each new type_unit DIE was added to the limbo die list when created. - Since these have all been added to comdat_type_list, clear the - limbo die list. */ - limbo_die_list = NULL; - - /* For each new comdat type unit, copy declarations for incomplete - types to make the new unit self-contained (i.e., no direct - references to the main compile unit). */ - for (ctnode = comdat_type_list; ctnode != NULL; ctnode = ctnode->next) - copy_decls_for_unworthy_types (ctnode->root_die); - copy_decls_for_unworthy_types (comp_unit_die ()); - - /* In the process of copying declarations from one unit to another, - we may have left some declarations behind that are no longer - referenced. Prune them. */ - prune_unused_types (); - } - /* Generate separate CUs for each of the include files we've seen. They will go into limbo_die_list. */ if (flag_eliminate_dwarf2_dups) @@ -28177,6 +28190,33 @@ dwarf2out_early_finish (const char *file } deferred_asm_name = NULL; + if (flag_eliminate_unused_debug_types) + prune_unused_types (); + + /* Generate separate COMDAT sections for type DIEs. */ + if (use_debug_types) + { + break_out_comdat_types (comp_unit_die ()); + + /* Each new type_unit DIE was added to the limbo die list when created. + Since these have all been added to comdat_type_list, clear the + limbo die list. */ + limbo_die_list = NULL; + + /* For each new comdat type unit, copy declarations for incomplete + types to make the new unit self-contained (i.e., no direct + references to the main compile unit). */ + for (comdat_type_node *ctnode = comdat_type_list; + ctnode != NULL; ctnode = ctnode->next) + copy_decls_for_unworthy_types (ctnode->root_die); + copy_decls_for_unworthy_types (comp_unit_die ()); + + /* In the process of copying declarations from one unit to another, + we may have left some declarations behind that are no longer + referenced. Prune them. */ + prune_unused_types (); + } + /* The early debug phase is now finished. */ early_dwarf_finished = true; }