public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish
@ 2016-09-22 14:45 Richard Biener
  2016-09-23  8:22 ` Richard Biener
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2016-09-22 14:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason


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?

I believe this is the last part I can reasonably split out (but I'll
have a second look after merging back from trunk once this and the other
pending patch is approved).

Thanks,
Richard.

2016-09-22  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.
	(dwarf2out_finish): Move unused type pruning debug_types handling
	and breaking out includes ...
	(dwarf2out_early_finish): ... here.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 240353)
+++ gcc/dwarf2out.c	(working copy)
@@ -2705,6 +2705,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;
@@ -5098,7 +5102,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
@@ -5163,7 +5173,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).  */
@@ -26195,6 +26214,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
@@ -26224,12 +26253,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)
@@ -27835,37 +27866,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)
-    break_out_includes (comp_unit_die ());
-
   /* Traverse the DIE's and add sibling attributes to those DIE's that
      have children.  */
   add_sibling_attributes (comp_unit_die ());
@@ -28193,6 +28193,38 @@ 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 ();
+    }
+
+  /* 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 ());
+
   /* The early debug phase is now finished.  */
   early_dwarf_finished = true;
 }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish
  2016-09-22 14:45 [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish Richard Biener
@ 2016-09-23  8:22 ` Richard Biener
  2016-09-23 12:09   ` Richard Biener
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2016-09-23  8:22 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason

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;

Richard.

> I believe this is the last part I can reasonably split out (but I'll
> have a second look after merging back from trunk once this and the other
> pending patch is approved).
> 
> Thanks,
> Richard.
> 
> 2016-09-22  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.
> 	(dwarf2out_finish): Move unused type pruning debug_types handling
> 	and breaking out includes ...
> 	(dwarf2out_early_finish): ... here.
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c	(revision 240353)
> +++ gcc/dwarf2out.c	(working copy)
> @@ -2705,6 +2705,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;
> @@ -5098,7 +5102,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
> @@ -5163,7 +5173,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).  */
> @@ -26195,6 +26214,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
> @@ -26224,12 +26253,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)
> @@ -27835,37 +27866,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)
> -    break_out_includes (comp_unit_die ());
> -
>    /* Traverse the DIE's and add sibling attributes to those DIE's that
>       have children.  */
>    add_sibling_attributes (comp_unit_die ());
> @@ -28193,6 +28193,38 @@ 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 ();
> +    }
> +
> +  /* 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 ());
> +
>    /* The early debug phase is now finished.  */
>    early_dwarf_finished = true;
>  }
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish
  2016-09-23  8:22 ` Richard Biener
@ 2016-09-23 12:09   ` Richard Biener
  2016-09-26 14:30     ` Richard Biener
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2016-09-23 12:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason

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.

Thanks,
Richard.

> Richard.
> 
> > I believe this is the last part I can reasonably split out (but I'll
> > have a second look after merging back from trunk once this and the other
> > pending patch is approved).
> > 
> > Thanks,
> > Richard.
> > 
> > 2016-09-22  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.
> > 	(dwarf2out_finish): Move unused type pruning debug_types handling
> > 	and breaking out includes ...
> > 	(dwarf2out_early_finish): ... here.
> > 
> > Index: gcc/dwarf2out.c
> > ===================================================================
> > --- gcc/dwarf2out.c	(revision 240353)
> > +++ gcc/dwarf2out.c	(working copy)
> > @@ -2705,6 +2705,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;
> > @@ -5098,7 +5102,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
> > @@ -5163,7 +5173,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).  */
> > @@ -26195,6 +26214,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
> > @@ -26224,12 +26253,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)
> > @@ -27835,37 +27866,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)
> > -    break_out_includes (comp_unit_die ());
> > -
> >    /* Traverse the DIE's and add sibling attributes to those DIE's that
> >       have children.  */
> >    add_sibling_attributes (comp_unit_die ());
> > @@ -28193,6 +28193,38 @@ 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 ();
> > +    }
> > +
> > +  /* 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 ());
> > +
> >    /* The early debug phase is now finished.  */
> >    early_dwarf_finished = true;
> >  }
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish
  2016-09-23 12:09   ` Richard Biener
@ 2016-09-26 14:30     ` Richard Biener
  2016-09-28 14:40       ` Jason Merrill
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2016-09-26 14:30 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish
  2016-09-26 14:30     ` Richard Biener
@ 2016-09-28 14:40       ` Jason Merrill
  0 siblings, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2016-09-28 14:40 UTC (permalink / raw)
  To: Richard Biener, gcc-patches

OK.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-09-28 14:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-22 14:45 [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish Richard Biener
2016-09-23  8:22 ` Richard Biener
2016-09-23 12:09   ` Richard Biener
2016-09-26 14:30     ` Richard Biener
2016-09-28 14:40       ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).