public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR83941
@ 2018-09-25  7:31 Richard Biener
  2018-09-25 13:05 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Richard Biener @ 2018-09-25  7:31 UTC (permalink / raw)
  To: gcc-patches


This gets rid of 99% of the useless late DIE to early DIE forwarders
(those which do not have any attributes besides the 
DW_AT_abstract_origin).  I've wanted to do this for a long time but
only now was able to verify that creating the concrete instances
lazily in lookup_decl_die and friend works.

So the patch replaces BLOCK_DIE uses in dwarf2out.c with lookup_block_die
and equate_block_to_die "forwarders" that do the lazy concrete instance
DIE creation and adjusts lookup_decl_die accordingly.  It then switches
dwarf2out_register_external_die to populate a decl -> { symbol, offset }
map rather than using DIEs for the recording (and adjusts
dwarf2out_die_ref_for_decl to look into that map in WPA phase).

The rest of the patch deals with cases where we are outputting references
to DIEs and replaces existing hacks with easier querying of the new map.
It also makes sure to output DW_TAG_imported_unit DIEs first rather than
spread over the DIE tree and consistently so.

LTO bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

I do consider backporting this given it makes the debug info so much
easier to read (actual size only shriks by about 10%).  A backport of
the patch passed LTO bootstrap and regtest but I'll leave this on
trunk for a while to spot issues.

I said 99% above because I did spot some stray remains when digging
through cc1 debug info.  Have to see if I can easily debug that.

Richard.

2018-09-25  Richard Biener  <rguenther@suse.de>

	PR debug/83941
	* dwarf2out.c (struct sym_off_pair): New.
	(external_die_map): New global.
	(lookup_decl_die): When in LTO create DIEs lazily from the
	external_die_map.
	(lookup_block_die): New function, create DIEs lazily in LTO.
	(equate_block_to_die): New function.
	(dwarf2out_die_ref_for_decl): During WPA get the association
	from the external DIE map.
	(dwarf2out_register_external_die): Record mapping into the
	external DIE map.
	(maybe_create_die_with_external_ref): New function split out from
	DIE generation part of old dwarf2out_register_external_die.
	(add_abstract_origin_attribute): Do not return the DIE.  When
	in LTO reference externals directly.
	(dwarf2out_abstract_function): When in LTO ignore calls for
	decls with external DIEs (already present abstract instances).
	(gen_call_site_die): Adjust.
	(add_high_low_attributes): Likewise.
	(gen_lexical_block_die): Likewise.
	(gen_inlined_subroutine_die): Likewie.
	(gen_block_die): Likewise.
	(dwarf2out_inline_entry): Likewise.
	(dwarf2out_early_finish): In LTRANS phase create DW_TAG_imported_unit
	DIEs.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 264526)
+++ gcc/dwarf2out.c	(working copy)
@@ -3831,7 +3831,7 @@ static inline void add_bit_offset_attrib
 					     struct vlr_context *);
 static void add_bit_size_attribute (dw_die_ref, tree);
 static void add_prototyped_attribute (dw_die_ref, tree);
-static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
+static void add_abstract_origin_attribute (dw_die_ref, tree);
 static void add_pure_or_virtual_attribute (dw_die_ref, tree);
 static void add_src_coords_attributes (dw_die_ref, tree);
 static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = false);
@@ -5830,6 +5830,14 @@ equate_type_number_to_die (tree type, dw
   TYPE_SYMTAB_DIE (type) = type_die;
 }
 
+static dw_die_ref maybe_create_die_with_external_ref (tree);
+struct GTY(()) sym_off_pair 
+{
+  const char * GTY((skip)) sym;
+  unsigned HOST_WIDE_INT off;
+};
+static GTY(()) hash_map<tree, sym_off_pair> *external_die_map;
+
 /* Returns a hash value for X (which really is a die_struct).  */
 
 inline hashval_t
@@ -5854,7 +5862,11 @@ lookup_decl_die (tree decl)
   dw_die_ref *die = decl_die_table->find_slot_with_hash (decl, DECL_UID (decl),
 							 NO_INSERT);
   if (!die)
-    return NULL;
+    {
+      if (in_lto_p)
+	return maybe_create_die_with_external_ref (decl);
+      return NULL;
+    }
   if ((*die)->removed)
     {
       decl_die_table->clear_slot (die);
@@ -5864,6 +5876,27 @@ lookup_decl_die (tree decl)
 }
 
 
+/* Return the DIE associated with BLOCK.  */
+
+static inline dw_die_ref
+lookup_block_die (tree block)
+{
+  dw_die_ref die = BLOCK_DIE (block);
+  if (!die && in_lto_p)
+    return maybe_create_die_with_external_ref (block);
+  return die;
+}
+
+/* Associate DIE with BLOCK.  */
+
+static inline void
+equate_block_to_die (tree block, dw_die_ref die)
+{
+  BLOCK_DIE (block) = die;
+}
+#undef BLOCK_DIE
+
+
 /* For DECL which might have early dwarf output query a SYMBOL + OFFSET
    style reference.  Return true if we found one refering to a DIE for
    DECL, otherwise return false.  */
@@ -5874,32 +5907,27 @@ dwarf2out_die_ref_for_decl (tree decl, c
 {
   dw_die_ref die;
 
-  if (in_lto_p && !decl_die_table)
-    return false;
+  if (in_lto_p)
+    {
+      /* During WPA stage and incremental linking we use a hash-map
+	 to store the decl <-> label + offset map.  */
+      if (!external_die_map)
+	return false;
+      sym_off_pair *desc = external_die_map->get (decl);
+      if (!desc)
+	return false;
+      *sym = desc->sym;
+      *off = desc->off;
+      return true;
+    }
 
   if (TREE_CODE (decl) == BLOCK)
-    die = BLOCK_DIE (decl);
+    die = lookup_block_die (decl);
   else
     die = lookup_decl_die (decl);
   if (!die)
     return false;
 
-  /* During WPA stage and incremental linking we currently use DIEs
-     to store the decl <-> label + offset map.  That's quite inefficient
-     but it works for now.  */
-  if (in_lto_p)
-    {
-      dw_die_ref ref = get_AT_ref (die, DW_AT_abstract_origin);
-      if (!ref)
-	{
-	  gcc_assert (die == comp_unit_die ());
-	  return false;
-	}
-      *off = ref->die_offset;
-      *sym = ref->die_id.die_symbol;
-      return true;
-    }
-
   /* Similar to get_ref_die_offset_label, but using the "correct"
      label.  */
   *off = die->die_offset;
@@ -5921,6 +5949,8 @@ add_AT_external_die_ref (dw_die_ref die,
 {
   /* Create a fake DIE that contains the reference.  Don't use
      new_die because we don't want to end up in the limbo list.  */
+  /* ???  We probably want to share these, thus put a ref to the DIE
+     we create here to the external_die_map entry.  */
   dw_die_ref ref = new_die_raw (die->die_tag);
   ref->die_id.die_symbol = IDENTIFIER_POINTER (get_identifier (symbol));
   ref->die_offset = offset;
@@ -5938,13 +5968,33 @@ dwarf2out_register_external_die (tree de
   if (debug_info_level == DINFO_LEVEL_NONE)
     return;
 
-  if ((flag_wpa
-       || flag_incremental_link == INCREMENTAL_LINK_LTO) && !decl_die_table)
-    decl_die_table = hash_table<decl_die_hasher>::create_ggc (1000);
+  if (!external_die_map)
+    external_die_map = hash_map<tree, sym_off_pair>::create_ggc (1000);
+  gcc_checking_assert (!external_die_map->get (decl));
+  sym_off_pair p = { IDENTIFIER_POINTER (get_identifier (sym)), off };
+  external_die_map->put (decl, p);
+}
+
+/* If we have a registered external DIE for DECL return a new DIE for
+   the concrete instance with an appropriate abstract origin.  */
+
+static dw_die_ref
+maybe_create_die_with_external_ref (tree decl)
+{
+  if (!external_die_map)
+    return NULL;
+  sym_off_pair *desc = external_die_map->get (decl);
+  if (!desc)
+    return NULL;
+
+  const char *sym = desc->sym;
+  unsigned HOST_WIDE_INT off = desc->off;
 
-  dw_die_ref die
-    = TREE_CODE (decl) == BLOCK ? BLOCK_DIE (decl) : lookup_decl_die (decl);
+  in_lto_p = false;
+  dw_die_ref die = (TREE_CODE (decl) == BLOCK
+		    ? lookup_block_die (decl) : lookup_decl_die (decl));
   gcc_assert (!die);
+  in_lto_p = true;
 
   tree ctx;
   dw_die_ref parent = NULL;
@@ -5956,7 +6006,7 @@ dwarf2out_register_external_die (tree de
       /* ???  We do not output DIEs for all scopes thus skip as
 	 many DIEs as needed.  */
       while (TREE_CODE (ctx) == BLOCK
-	     && !BLOCK_DIE (ctx))
+	     && !lookup_block_die (ctx))
 	ctx = BLOCK_SUPERCONTEXT (ctx);
     }
   else
@@ -5971,7 +6021,7 @@ dwarf2out_register_external_die (tree de
   if (ctx)
     {
       if (TREE_CODE (ctx) == BLOCK)
-	parent = BLOCK_DIE (ctx);
+	parent = lookup_block_die (ctx);
       else if (TREE_CODE (ctx) == TRANSLATION_UNIT_DECL
 	       /* Keep the 1:1 association during WPA.  */
 	       && !flag_wpa
@@ -5998,18 +6048,14 @@ dwarf2out_register_external_die (tree de
   switch (TREE_CODE (decl))
     {
     case TRANSLATION_UNIT_DECL:
-      if (! flag_wpa && flag_incremental_link != INCREMENTAL_LINK_LTO)
-	{
-	  die = comp_unit_die ();
-	  dw_die_ref import = new_die (DW_TAG_imported_unit, die, NULL_TREE);
-	  add_AT_external_die_ref (import, DW_AT_import, sym, off);
-	  /* We re-target all CU decls to the LTRANS CU DIE, so no need
-	     to create a DIE for the original CUs.  */
-	  return;
-	}
-      /* Keep the 1:1 association during WPA.  */
-      die = new_die (DW_TAG_compile_unit, NULL, decl);
-      break;
+      {
+	die = comp_unit_die ();
+	dw_die_ref import = new_die (DW_TAG_imported_unit, die, NULL_TREE);
+	add_AT_external_die_ref (import, DW_AT_import, sym, off);
+	/* We re-target all CU decls to the LTRANS CU DIE, so no need
+	   to create a DIE for the original CUs.  */
+	return die;
+      }
     case NAMESPACE_DECL:
       if (is_fortran (decl))
 	die = new_die (DW_TAG_module, parent, decl);
@@ -6041,7 +6087,7 @@ dwarf2out_register_external_die (tree de
       gcc_unreachable ();
     }
   if (TREE_CODE (decl) == BLOCK)
-    BLOCK_DIE (decl) = die;
+    equate_block_to_die (decl, die);
   else
     equate_decl_number_to_die (decl, die);
 
@@ -6049,6 +6095,8 @@ dwarf2out_register_external_die (tree de
 
   /* Add a reference to the DIE providing early debug at $sym + off.  */
   add_AT_external_die_ref (die, DW_AT_abstract_origin, sym, off);
+
+  return die;
 }
 
 /* Returns a hash value for X (which really is a var_loc_list).  */
@@ -21222,28 +21270,28 @@ add_prototyped_attribute (dw_die_ref die
    by looking in the type declaration, the object declaration equate table or
    the block mapping.  */
 
-static inline dw_die_ref
+static inline void
 add_abstract_origin_attribute (dw_die_ref die, tree origin)
 {
   dw_die_ref origin_die = NULL;
 
   if (DECL_P (origin))
     {
-      dw_die_ref c;
-      origin_die = lookup_decl_die (origin);
-      /* "Unwrap" the decls DIE which we put in the imported unit context.
-         We are looking for the abstract copy here.  */
+      sym_off_pair *desc;
       if (in_lto_p
-	  && origin_die
-	  && (c = get_AT_ref (origin_die, DW_AT_abstract_origin))
-	  /* ???  Identify this better.  */
-	  && c->with_offset)
-	origin_die = c;
+	  && external_die_map
+	  && (desc = external_die_map->get (origin)))
+	{
+	  add_AT_external_die_ref (die, DW_AT_abstract_origin,
+				   desc->sym, desc->off);
+	  return;
+	}
+      origin_die = lookup_decl_die (origin);
     }
   else if (TYPE_P (origin))
     origin_die = lookup_type_die (origin);
   else if (TREE_CODE (origin) == BLOCK)
-    origin_die = BLOCK_DIE (origin);
+    origin_die = lookup_block_die (origin);
 
   /* XXX: Functions that are never lowered don't always have correct block
      trees (in the case of java, they simply have no block tree, in some other
@@ -21256,7 +21304,6 @@ add_abstract_origin_attribute (dw_die_re
 
   if (origin_die)
     add_AT_die_ref (die, DW_AT_abstract_origin, origin_die);
-  return origin_die;
 }
 
 /* We do not currently support the pure_virtual attribute.  */
@@ -22552,6 +22599,13 @@ dwarf2out_abstract_function (tree decl)
   if (DECL_IGNORED_P (decl))
     return;
 
+  /* Do not lazily create a DIE for decl here just because we
+     got called via debug_hooks->outlining_inline_function.  */
+  if (in_lto_p
+      && external_die_map
+      && external_die_map->get (decl))
+    return;
+
   old_die = lookup_decl_die (decl);
   /* With early debug we always have an old DIE unless we are in LTO
      and the user did not compile but only link with debug.  */
@@ -22670,7 +22724,7 @@ gen_call_site_die (tree decl, dw_die_ref
 	 && block != DECL_INITIAL (decl)
 	 && TREE_CODE (block) == BLOCK)
     {
-      stmt_die = BLOCK_DIE (block);
+      stmt_die = lookup_block_die (block);
       if (stmt_die)
 	break;
       block = BLOCK_SUPERCONTEXT (block);
@@ -24152,12 +24206,12 @@ add_high_low_attributes (tree stmt, dw_d
 static void
 gen_lexical_block_die (tree stmt, dw_die_ref context_die)
 {
-  dw_die_ref old_die = BLOCK_DIE (stmt);
+  dw_die_ref old_die = lookup_block_die (stmt);
   dw_die_ref stmt_die = NULL;
   if (!old_die)
     {
       stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
-      BLOCK_DIE (stmt) = stmt_die;
+      equate_block_to_die (stmt, stmt_die);
     }
 
   if (BLOCK_ABSTRACT (stmt))
@@ -24185,7 +24239,7 @@ gen_lexical_block_die (tree stmt, dw_die
       if (old_die)
 	{
 	  stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
-	  BLOCK_DIE (stmt) = stmt_die;
+	  equate_block_to_die (stmt, stmt_die);
 	  old_die = NULL;
 	}
 
@@ -24232,7 +24286,7 @@ gen_inlined_subroutine_die (tree stmt, d
 	= new_die (DW_TAG_inlined_subroutine, context_die, stmt);
 
       if (call_arg_locations || debug_inline_points)
-	BLOCK_DIE (stmt) = subr_die;
+	equate_block_to_die (stmt, subr_die);
       add_abstract_origin_attribute (subr_die, decl);
       if (TREE_ASM_WRITTEN (stmt))
         add_high_low_attributes (stmt, subr_die);
@@ -25732,7 +25786,7 @@ gen_block_die (tree stmt, dw_die_ref con
     /* The outer scopes for inlinings *must* always be represented.  We
        generate DW_TAG_inlined_subroutine DIEs for them.  (See below.) */
     must_output_die = 1;
-  else if (BLOCK_DIE (stmt))
+  else if (lookup_block_die (stmt))
     /* If we already have a DIE then it was filled early.  Meanwhile
        we might have pruned all BLOCK_VARS as optimized out but we
        still want to generate high/low PC attributes so output it.  */
@@ -27563,7 +27617,7 @@ dwarf2out_inline_entry (tree block)
 				      true));
 
   gcc_assert (inlined_function_outer_scope_p (block));
-  gcc_assert (!BLOCK_DIE (block));
+  gcc_assert (!lookup_block_die (block));
 
   if (BLOCK_FRAGMENT_ORIGIN (block))
     block = BLOCK_FRAGMENT_ORIGIN (block);
@@ -31987,6 +32041,17 @@ dwarf2out_early_finish (const char *file
      sure to adjust the phase after annotating the LTRANS CU DIE.  */
   if (in_lto_p)
     {
+      /* Force DW_TAG_imported_unit to be created now, otherwise
+	 we might end up without it or ordered after DW_TAG_inlined_subroutine
+	 referencing DIEs from it.  */
+      if (! flag_wpa && flag_incremental_link != INCREMENTAL_LINK_LTO)
+	{
+	  unsigned i;
+	  tree tu;
+	  FOR_EACH_VEC_SAFE_ELT (all_translation_units, i, tu)
+	    maybe_create_die_with_external_ref (tu);
+	}
+
       early_dwarf_finished = true;
       if (dump_file)
 	{

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

* Re: [PATCH] Fix PR83941
  2018-09-25  7:31 [PATCH] Fix PR83941 Richard Biener
@ 2018-09-25 13:05 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2018-09-25 13:05 UTC (permalink / raw)
  To: gcc-patches

On Tue, 25 Sep 2018, Richard Biener wrote:

> 
> This gets rid of 99% of the useless late DIE to early DIE forwarders
> (those which do not have any attributes besides the 
> DW_AT_abstract_origin).  I've wanted to do this for a long time but
> only now was able to verify that creating the concrete instances
> lazily in lookup_decl_die and friend works.
> 
> So the patch replaces BLOCK_DIE uses in dwarf2out.c with lookup_block_die
> and equate_block_to_die "forwarders" that do the lazy concrete instance
> DIE creation and adjusts lookup_decl_die accordingly.  It then switches
> dwarf2out_register_external_die to populate a decl -> { symbol, offset }
> map rather than using DIEs for the recording (and adjusts
> dwarf2out_die_ref_for_decl to look into that map in WPA phase).
> 
> The rest of the patch deals with cases where we are outputting references
> to DIEs and replaces existing hacks with easier querying of the new map.
> It also makes sure to output DW_TAG_imported_unit DIEs first rather than
> spread over the DIE tree and consistently so.
> 
> LTO bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> 
> I do consider backporting this given it makes the debug info so much
> easier to read (actual size only shriks by about 10%).  A backport of
> the patch passed LTO bootstrap and regtest but I'll leave this on
> trunk for a while to spot issues.
> 
> I said 99% above because I did spot some stray remains when digging
> through cc1 debug info.  Have to see if I can easily debug that.

The following gets rid of the remaining issues (also fixing a mistake
I made with DW_TAG_imported_unit which is now created over and over
again).  It also does a few simplifications where possible.

LTO bootstrap and regtest in progress on x86_64-unknown-linux-gnu.

Richard.

2018-09-25  Richard Biener  <rguenther@suse.de>

	PR debug/83941
	* dwarf2out.c (add_AT_external_die_ref): Remove now redundant
	GC-ification.
	(maybe_create_die_with_external_ref): Do not create
	DW_TAG_imported_unit here.
	(add_abstract_origin_attribute): Handle external BLOCK refs.
	(dwarf2out_abstract_function): Simplify LTO case.
	(dwarf2out_early_finish): Create DW_TAG_imported_unit explicitely
	rather than using maybe_create_die_with_external_ref.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 264564)
+++ gcc/dwarf2out.c	(working copy)
@@ -5868,7 +5868,7 @@ add_AT_external_die_ref (dw_die_ref die,
   /* ???  We probably want to share these, thus put a ref to the DIE
      we create here to the external_die_map entry.  */
   dw_die_ref ref = new_die_raw (die->die_tag);
-  ref->die_id.die_symbol = IDENTIFIER_POINTER (get_identifier (symbol));
+  ref->die_id.die_symbol = symbol;
   ref->die_offset = offset;
   ref->with_offset = 1;
   add_AT_die_ref (die, attr_kind, ref);
@@ -5966,8 +5966,6 @@ maybe_create_die_with_external_ref (tree
     case TRANSLATION_UNIT_DECL:
       {
 	die = comp_unit_die ();
-	dw_die_ref import = new_die (DW_TAG_imported_unit, die, NULL_TREE);
-	add_AT_external_die_ref (import, DW_AT_import, sym, off);
 	/* We re-target all CU decls to the LTRANS CU DIE, so no need
 	   to create a DIE for the original CUs.  */
 	return die;
@@ -21134,19 +21132,21 @@ add_abstract_origin_attribute (dw_die_re
 {
   dw_die_ref origin_die = NULL;
 
-  if (DECL_P (origin))
+  /* For late LTO debug output we want to refer directly to the abstract
+     DIE in the early debug rather to the possibly existing concrete
+     instance and avoid creating that just for this purpose.  */
+  sym_off_pair *desc;
+  if (in_lto_p
+      && external_die_map
+      && (desc = external_die_map->get (origin)))
     {
-      sym_off_pair *desc;
-      if (in_lto_p
-	  && external_die_map
-	  && (desc = external_die_map->get (origin)))
-	{
-	  add_AT_external_die_ref (die, DW_AT_abstract_origin,
-				   desc->sym, desc->off);
-	  return;
-	}
-      origin_die = lookup_decl_die (origin);
+      add_AT_external_die_ref (die, DW_AT_abstract_origin,
+			       desc->sym, desc->off);
+      return;
     }
+
+  if (DECL_P (origin))
+    origin_die = lookup_decl_die (origin);
   else if (TYPE_P (origin))
     origin_die = lookup_type_die (origin);
   else if (TREE_CODE (origin) == BLOCK)
@@ -22458,21 +22458,15 @@ dwarf2out_abstract_function (tree decl)
   if (DECL_IGNORED_P (decl))
     return;
 
-  /* Do not lazily create a DIE for decl here just because we
-     got called via debug_hooks->outlining_inline_function.  */
-  if (in_lto_p
-      && external_die_map
-      && external_die_map->get (decl))
+  /* In LTO we're all set.  We already created abstract instances
+     early and we want to avoid creating a concrete instance of that
+     if we don't output it.  */
+  if (in_lto_p)
     return;
 
   old_die = lookup_decl_die (decl);
-  /* With early debug we always have an old DIE unless we are in LTO
-     and the user did not compile but only link with debug.  */
-  if (in_lto_p && ! old_die)
-    return;
   gcc_assert (old_die != NULL);
-  if (get_AT (old_die, DW_AT_inline)
-      || get_AT (old_die, DW_AT_abstract_origin))
+  if (get_AT (old_die, DW_AT_inline))
     /* We've already generated the abstract instance.  */
     return;
 
@@ -31908,7 +31910,13 @@ dwarf2out_early_finish (const char *file
 	  unsigned i;
 	  tree tu;
 	  FOR_EACH_VEC_SAFE_ELT (all_translation_units, i, tu)
-	    maybe_create_die_with_external_ref (tu);
+	    if (sym_off_pair *desc = external_die_map->get (tu))
+	      {
+		dw_die_ref import = new_die (DW_TAG_imported_unit,
+					     comp_unit_die (), NULL_TREE);
+		add_AT_external_die_ref (import, DW_AT_import,
+					 desc->sym, desc->off);
+	      }
 	}
 
       early_dwarf_finished = true;

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

end of thread, other threads:[~2018-09-25 13:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-25  7:31 [PATCH] Fix PR83941 Richard Biener
2018-09-25 13:05 ` Richard Biener

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).