This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][2/2] early LTO debug, main part


On 05/19/2017 06:42 AM, Richard Biener wrote:
+     /* ???  In some cases the C++ FE (at least) fails to
+        set DECL_CONTEXT properly.  Simply globalize stuff
+        in this case.  For example
+        __dso_handle created via iostream line 74 col 25.  */
+     parent = comp_unit_die ();

I've now fixed __dso_handle, so that can be removed from the comment, but it still makes sense to have this fall-back for the (permitted) case of null DECL_CONTEXT.

+       /* ???  LANG issue - DW_TAG_module for fortran.  Either look
+ 	 at the input language (if we have enough DECL_CONTEXT to follow)
+ 	 or use a bit in tree_decl_with_vis to record the distinction.  */

Sure, you should be able to look at TRANSLATION_UNIT_LANGUAGE.

! 		  /* ???  We cannot unconditionally output die_offset if
! 		     non-zero - at least -feliminate-dwarf2-dups will
! 		     create references to those DIEs via symbols.  And we
! 		     do not clear its DIE offset after outputting it
! 		     (and the label refers to the actual DIEs, not the
! 		     DWARF CU unit header which is when using label + offset
! 		     would be the correct thing to do).

As in our previous discussion, I think -feliminate-dwarf2-dups can go away now. But this is a more general issue: die_offset has meant the offset from the beginning of the CU, but if with_offset is set it means an offset from die_symbol. Since with_offset changes the meaning of die_symbol and die_offset, having different code here depending on that flag makes sense.

It seems likely that when -fEDD goes away, we will never again want to do direct symbolic references to DIEs, in which case we could drop the current meaning of die_symbol, and so we wouldn't need the with_offset flag.

!   unit_die->comdat_type_p = comdat_p;
! }
! ! static void
! compute_section_prefix (dw_die_ref unit_die)
! {
!   compute_section_prefix_1 (unit_die, true);
!   comdat_symbol_id = unit_die->die_id.die_symbol;
    comdat_symbol_number = 0;
  }

Let's set the comdat_type_p flag in this function rather than add a parameter to the existing function. And when -fEDD goes away, we don't need this entry point at all.

Also, for LTO debug, it seems you aren't actually using the symbol as a section prefix, so the name becomes inaccurate. Maybe compute_comp_unit_symbol rather than compute_section_prefix_1?

+   /* For LTO cross unit DIE refs we want a symbol on the start of the
+      debuginfo section, not on the CU DIE.
+      ???  We could simply use the symbol as it would be output by output_die
+      and account for the extra offset produced by the CU header which has fixed
+      size.  OTOH it currently only supports linkonce globals which would
+      be less than ideal?.  */

I think the way you're doing it now is better than this alternative, since die_offset is relative to the beginning of the CU header.

!       /* Don't output the symbol twice.  For LTO we want the label
!          on the section beginning, not on the actual DIE.  */
!       && (!flag_generate_lto
! 	      || die->die_tag != DW_TAG_compile_unit))

I think this check should just be !with_offset; if that flag is set the DIE doesn't actually have its own symbol.

!       if (old_die
! 	  && (c = get_AT_ref (old_die, DW_AT_abstract_origin))
! 	  /* ???  In LTO all origin DIEs still refer to the early
! 	     debug copy.  Detect that.  */
! 	  && get_AT (c, DW_AT_inline))
...
!       /* "Unwrap" the decls DIE which we put in the imported unit context.
!           ???  If we finish dwarf2out_function_decl refactoring we can
! 	  do this in a better way from the start and only lazily emit
! 	  the early DIE references.  */

It seems like in gen_subprogram_die you deliberately avoid reusing the DIE from dwarf2out_register_external_die (since it doesn't have DW_AT_inline), and then in add_abstract_origin_attribute you need to look through that redundant die. Why not reuse it?

! 	  /* ???  In LTO we do not see any of the location attributes.  */
! 	   && ((DECL_ARTIFICIAL (decl) || in_lto_p)

Perhaps get_AT_ref (old_die, DW_AT_abstract_origin) instead of in_lto_p?

And you don't need the added parentheses here.

     if (parm_die && parm_die->die_parent != context_die)
  	{
! 	  /* ???  The DIE parent is the "abstract" copy and the context_die
! 	     is the specification "copy".  */
! 	  if (!DECL_ABSTRACT_P (node) && !in_lto_p)

And a bit below in the current source,

/* FIXME: Reuse DIE even with a differing context. This can happen when calling dwarf2out_abstract_function to build debug info for the abstract instance of a function for which we have already generated a DIE in dwarf2out_early_global_decl. Once we remove dwarf2out_abstract_function, we should have a call to gcc_unreachable here. */

Now that you've done away with the old dwarf2out_abstract_function, I think dwarf2out shouldn't need to consider DECL_ABSTRACT_P at all.

In the hunk above, it seems that you want to reuse the parm DIEs from d*_register_external_die if origin is NULL, which seems appropriate. So, are there any cases remaining where we actually want to clear parm_die there? I suspect not, in which case we should be able to remove the entire "if (parm_die && parm_die->die_parent != context_die)" block.

! 	  /* Early created DIEs do not have a parent as the decls refer
! 	     to the function as DECL_CONTEXT rather than the BLOCK.  */
! 	  if (in_lto_p
! 	      && die && die->die_parent == NULL)
! 	    add_child_die (context_die, die);

Perhaps in_lto_p should be asserted rather than a condition?

! 	       /* Do not generate stray type DIEs in late LTO dumping.  */
! 	       && early_dwarf)
...
!       /* Avoid generating stray type DIEs during late dwarf dumping.
!          All types have been dumped early.  */
!       if (! lookup_decl_die (decl_or_origin)
...
!       /* Avoid generating stray type DIEs during late dwarf dumping.
!          All types have been dumped early.  */
!       if (! lookup_decl_die (decl_or_origin)

Should these use the same test (early_dwarf vs. lookup_decl_die)?

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]