Re: [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish

[Richard Biener <rguenther@suse.de>]

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/<file>.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><e7db>: Abbrev Number: 180 (DW_TAG_subprogram)
>     <e7dd>   DW_AT_specification: <0x6a3a>
>     <e7e1>   DW_AT_inline      : 3      (declared as inline and inlined)
>     <e7e2>   DW_AT_sibling     : <0xe7ff>
>  <2><e7e6>: Abbrev Number: 63 (DW_TAG_formal_parameter)
>     <e7e7>   DW_AT_name        : (indirect string, offset: 0x3f48): alloc
>     <e7eb>   DW_AT_decl_file   : 5
>     <e7ec>   DW_AT_decl_line   : 1050
>     <e7ee>   DW_AT_type        : <0x6d>
>  <2><e7f2>: Abbrev Number: 51 (DW_TAG_typedef)
>     <e7f3>   DW_AT_name        : (indirect string, offset: 0x1435f): 
> vec_embedde
> d
>     <e7f7>   DW_AT_decl_file   : 5
>     <e7f8>   DW_AT_decl_line   : 1052
>     <e7fa>   DW_AT_type        : <0x665a>
>  <2><e7fe>: 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  <rguenther@suse.de>

	* 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;
 }