public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
@ 2015-06-12  1:25 David Edelsohn
  2015-06-12  5:45 ` Richard Biener
  0 siblings, 1 reply; 16+ messages in thread
From: David Edelsohn @ 2015-06-12  1:25 UTC (permalink / raw)
  To: Pierre-Marie de Rodat; +Cc: Richard Biener, GCC Patches

This patch broke AIX bootstrap because dbxout.c was not updated for
the XCOFF_DEBUGGING_INFO case.

- David

* dbxout.c (xcoff_debug_hooks): Provide a function for
register_main_translation_unit hook.

Index: dbxout.c
===================================================================
--- dbxout.c    (revision 224402)
+++ dbxout.c    (working copy)
@@ -419,6 +419,7 @@
   xcoffout_end_epilogue,
   debug_nothing_tree,                   /* begin_function */
   xcoffout_end_function,
+  debug_nothing_tree,                   /* register_main_translation_unit */
   debug_nothing_tree,                   /* function_decl */
   dbxout_early_global_decl,             /* early_global_decl */
   dbxout_late_global_decl,              /* late_global_decl */

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-12  1:25 [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites David Edelsohn
@ 2015-06-12  5:45 ` Richard Biener
  2015-06-12 10:15   ` Pierre-Marie de Rodat
  2015-06-12 10:32   ` Pierre-Marie de Rodat
  0 siblings, 2 replies; 16+ messages in thread
From: Richard Biener @ 2015-06-12  5:45 UTC (permalink / raw)
  To: David Edelsohn, Pierre-Marie de Rodat; +Cc: GCC Patches

On June 12, 2015 2:30:36 AM GMT+02:00, David Edelsohn <dje.gcc@gmail.com> wrote:
>This patch broke AIX bootstrap because dbxout.c was not updated for
>the XCOFF_DEBUGGING_INFO case.

OK.
Thanks
Richard.

>- David
>
>* dbxout.c (xcoff_debug_hooks): Provide a function for
>register_main_translation_unit hook.
>
>Index: dbxout.c
>===================================================================
>--- dbxout.c    (revision 224402)
>+++ dbxout.c    (working copy)
>@@ -419,6 +419,7 @@
>   xcoffout_end_epilogue,
>   debug_nothing_tree,                   /* begin_function */
>   xcoffout_end_function,
>+  debug_nothing_tree,                   /*
>register_main_translation_unit */
>   debug_nothing_tree,                   /* function_decl */
>   dbxout_early_global_decl,             /* early_global_decl */
>   dbxout_late_global_decl,              /* late_global_decl */


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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-12  5:45 ` Richard Biener
@ 2015-06-12 10:15   ` Pierre-Marie de Rodat
  2015-06-12 10:32   ` Pierre-Marie de Rodat
  1 sibling, 0 replies; 16+ messages in thread
From: Pierre-Marie de Rodat @ 2015-06-12 10:15 UTC (permalink / raw)
  To: Richard Biener, David Edelsohn; +Cc: GCC Patches

On 06/12/2015 07:41 AM, Richard Biener wrote:
> On June 12, 2015 2:30:36 AM GMT+02:00, David Edelsohn <dje.gcc@gmail.com> wrote:
>> This patch broke AIX bootstrap because dbxout.c was not updated for
>> the XCOFF_DEBUGGING_INFO case.
>
> OK.
> Thanks
> Richard.

Sorry about that and thank you for the fix!

-- 
Pierre-Marie de Rodat

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-12  5:45 ` Richard Biener
  2015-06-12 10:15   ` Pierre-Marie de Rodat
@ 2015-06-12 10:32   ` Pierre-Marie de Rodat
  2015-06-12 11:17     ` Richard Biener
  1 sibling, 1 reply; 16+ messages in thread
From: Pierre-Marie de Rodat @ 2015-06-12 10:32 UTC (permalink / raw)
  To: Richard Biener, David Edelsohn; +Cc: GCC Patches

On 06/12/2015 07:41 AM, Richard Biener wrote:
> On June 12, 2015 2:30:36 AM GMT+02:00, David Edelsohn <dje.gcc@gmail.com> wrote:
>> This patch broke AIX bootstrap because dbxout.c was not updated for
>> the XCOFF_DEBUGGING_INFO case.
>
> OK.
> Thanks
> Richard.

By the way, I think we should apply this fix to the GCC 5 branch as 
well. May I?

-- 
Pierre-Marie de Rodat

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-12 10:32   ` Pierre-Marie de Rodat
@ 2015-06-12 11:17     ` Richard Biener
  2015-06-12 13:56       ` Pierre-Marie de Rodat
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Biener @ 2015-06-12 11:17 UTC (permalink / raw)
  To: Pierre-Marie de Rodat, David Edelsohn; +Cc: GCC Patches

On June 12, 2015 12:31:02 PM GMT+02:00, Pierre-Marie de Rodat <derodat@adacore.com> wrote:
>On 06/12/2015 07:41 AM, Richard Biener wrote:
>> On June 12, 2015 2:30:36 AM GMT+02:00, David Edelsohn
><dje.gcc@gmail.com> wrote:
>>> This patch broke AIX bootstrap because dbxout.c was not updated for
>>> the XCOFF_DEBUGGING_INFO case.
>>
>> OK.
>> Thanks
>> Richard.
>
>By the way, I think we should apply this fix to the GCC 5 branch as 
>well. May I?

Yes


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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-12 11:17     ` Richard Biener
@ 2015-06-12 13:56       ` Pierre-Marie de Rodat
  0 siblings, 0 replies; 16+ messages in thread
From: Pierre-Marie de Rodat @ 2015-06-12 13:56 UTC (permalink / raw)
  To: Richard Biener, David Edelsohn; +Cc: GCC Patches

On 06/12/2015 01:11 PM, Richard Biener wrote:
>> By the way, I think we should apply this fix to the GCC 5 branch as
>> well. May I?
>
> Yes

Done. Thanks again!

-- 
Pierre-Marie de Rodat

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-11  9:25             ` Richard Biener
@ 2015-06-11 13:49               ` Pierre-Marie de Rodat
  0 siblings, 0 replies; 16+ messages in thread
From: Pierre-Marie de Rodat @ 2015-06-11 13:49 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches, Jakub Jelinek

On 06/11/2015 11:21 AM, Richard Biener wrote:
>> That sounds perfect: thank you Richard! I am about to commit the original fix
>> on mainline: should I still open a bugreport before commiting it to the GCC 5
>> branch?
>
> Yes please.

For the record, as you already noticed, I opened PR debug/66503. 
Original fix pushed on both trunk and the GCC 5 branch.

Thank you very much for your help!

Should I close the PR, now?

-- 
Pierre-Marie de Rodat

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-11  9:21           ` Pierre-Marie de Rodat
@ 2015-06-11  9:25             ` Richard Biener
  2015-06-11 13:49               ` Pierre-Marie de Rodat
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Biener @ 2015-06-11  9:25 UTC (permalink / raw)
  To: Pierre-Marie de Rodat; +Cc: GCC Patches, Jakub Jelinek

On Thu, 11 Jun 2015, Pierre-Marie de Rodat wrote:

> On 06/11/2015 11:06 AM, Richard Biener wrote:
> > FYI, I decided to backport the fix causing this regression to the
> > 4.8 branch today, guarded with in_lto_p, thus eliminating the effect
> > on non-LTO links.  The adjusted patch looks like the following and
> > I'll also adjust the 4.9 branch accordingly, leaving the GCC 5 branch
> > with the original fix.
> 
> That sounds perfect: thank you Richard! I am about to commit the original fix
> on mainline: should I still open a bugreport before commiting it to the GCC 5
> branch?

Yes please.

Thanks,
Richard.

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-11  9:10         ` Richard Biener
@ 2015-06-11  9:21           ` Pierre-Marie de Rodat
  2015-06-11  9:25             ` Richard Biener
  0 siblings, 1 reply; 16+ messages in thread
From: Pierre-Marie de Rodat @ 2015-06-11  9:21 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches, Jakub Jelinek

On 06/11/2015 11:06 AM, Richard Biener wrote:
> FYI, I decided to backport the fix causing this regression to the
> 4.8 branch today, guarded with in_lto_p, thus eliminating the effect
> on non-LTO links.  The adjusted patch looks like the following and
> I'll also adjust the 4.9 branch accordingly, leaving the GCC 5 branch
> with the original fix.

That sounds perfect: thank you Richard! I am about to commit the 
original fix on mainline: should I still open a bugreport before 
commiting it to the GCC 5 branch?

-- 
Pierre-Marie de Rodat

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-10 14:31       ` Pierre-Marie de Rodat
  2015-06-11  8:02         ` Richard Biener
@ 2015-06-11  9:10         ` Richard Biener
  2015-06-11  9:21           ` Pierre-Marie de Rodat
  1 sibling, 1 reply; 16+ messages in thread
From: Richard Biener @ 2015-06-11  9:10 UTC (permalink / raw)
  To: Pierre-Marie de Rodat; +Cc: GCC Patches, Jakub Jelinek

On Wed, 10 Jun 2015, Pierre-Marie de Rodat wrote:

> On 06/10/2015 03:36 PM, Richard Biener wrote:
> > Hmm, yes.  It meant to break after the first ;)  (without LTO
> > there usually is only one TU decl, apart from Java I think).
> > The hunk isn't in mainline because it was part of an experimental patch I
> > did on the early-debug branch.
> 
> Understood, thanks!
> 
> > Yeah, that looks great!
> > 
> > Of course I wonder about Java (builds multiple ones, one for each
> > input file) and Go (no idea).  I suppose Java would need to build
> > another one where all the "defaults" go (or it doesn't have any
> > such entities).
> 
> Yeah, I'm not familiar with them neither...
> 
> > In theory we could have changed dwarf2out_init to get a
> > translation-unit-decl argument as well.  But your patch looks like
> > we don't have such at the point of dwarf2out_init in all frontends.
> > 
> > Your patch is ok (and ok to backport) IMHO, though please give
> > others the chance to chime in.
> 
> Thank you very much. :-) I will soon become unavailable until July, so I think
> I'll wait until then to commit anyway.

FYI, I decided to backport the fix causing this regression to the
4.8 branch today, guarded with in_lto_p, thus eliminating the effect
on non-LTO links.  The adjusted patch looks like the following and
I'll also adjust the 4.9 branch accordingly, leaving the GCC 5 branch
with the original fix.

Richard.

2015-06-11  Richard Biener  <rguenther@suse.de>

	Backport from mainline, guarded with in_lto_p
	2015-06-02  Richard Biener  <rguenther@suse.de>

	PR debug/65549
	* dwarf2out.c (lookup_context_die): New function.
	(resolve_addr): Avoid forcing a full DIE for the
	target of a DW_TAG_GNU_call_site during late compilation.
	Instead create a stub DIE without a type if we have a
	context DIE present.

	* g++.dg/lto/pr65549_0.C: New testcase.

Index: gcc/testsuite/g++.dg/lto/pr65549_0.C
===================================================================
*** gcc/testsuite/g++.dg/lto/pr65549_0.C	(revision 0)
--- gcc/testsuite/g++.dg/lto/pr65549_0.C	(working copy)
***************
*** 0 ****
--- 1,144 ----
+ // { dg-lto-do link }
+ // { dg-lto-options { { -std=gnu++14 -flto -g } { -std=gnu++14 -flto -g -O2 -fno-inline -flto-partition=max } } }
+ // { dg-extra-ld-options "-r -nostdlib" }
+ 
+ namespace std {
+ inline namespace __cxx11 {}
+ template <typename _Tp, _Tp> struct integral_constant {
+   static constexpr _Tp value = 0;
+ };
+ template <typename> struct __and_;
+ struct is_member_object_pointer : integral_constant<bool, false> {};
+ template <typename>
+ struct is_member_function_pointer : integral_constant<bool, false> {};
+ template <typename> struct remove_reference { typedef int type; };
+ template <typename> class C;
+ template <bool, int, typename...> struct __result_of_impl;
+ template <typename _Functor, typename... _ArgTypes>
+ struct __result_of_impl<false, 0, _Functor, _ArgTypes...> {
+   typedef decltype(0) type;
+ };
+ template <typename _Functor, typename... _ArgTypes>
+ struct C<_Functor(_ArgTypes...)>
+     : __result_of_impl<is_member_object_pointer::value,
+                        is_member_function_pointer<
+                            typename remove_reference<_Functor>::type>::value,
+                        _Functor> {};
+ template <typename _Tp> using result_of_t = typename C<_Tp>::type;
+ template <typename> void forward();
+ template <typename _Tp> _Tp move(_Tp) {}
+ namespace __cxx11 {
+ class basic_string typedef string;
+ }
+ template <typename> struct allocator_traits { typedef decltype(0) pointer; };
+ }
+ struct F : std::allocator_traits<int> {};
+ namespace std {
+ namespace __cxx11 {
+ class basic_string {
+ public:
+   struct _Alloc_hider : F {
+     _Alloc_hider(pointer);
+   } _M_dataplus;
+   basic_string(int) : _M_dataplus(0) {}
+   ~basic_string();
+ };
+ }
+ template <typename> class function;
+ template <typename _Functor> class _Base_manager {
+ protected:
+   static _Functor *_M_get_pointer(int) {}
+ };
+ template <typename, typename> class _Function_handler;
+ template <typename _Res, typename _Functor, typename... _ArgTypes>
+ class _Function_handler<_Res(_ArgTypes...), _Functor>
+     : _Base_manager<_Functor> {
+ public:
+   static _Res _M_invoke(const int &) {
+     (*_Base_manager<_Functor>::_M_get_pointer(0))();
+   }
+ };
+ template <typename, typename> using __check_func_return_type = int;
+ template <typename _Res, typename... _ArgTypes>
+ class function<_Res(_ArgTypes...)> {
+   template <typename> using _Invoke = decltype(0);
+   template <typename _Functor>
+   using _Callable = __and_<__check_func_return_type<_Invoke<_Functor>, _Res>>;
+   template <typename, typename> using _Requires = int;
+ 
+ public:
+   template <typename _Functor, typename = _Requires<_Callable<_Functor>, void>>
+   function(_Functor);
+   using _Invoker_type = _Res (*)(const int &);
+   _Invoker_type _M_invoker;
+ };
+ template <typename _Res, typename... _ArgTypes>
+ template <typename _Functor, typename>
+ function<_Res(_ArgTypes...)>::function(_Functor) {
+   _M_invoker = _Function_handler<_Res(), _Functor>::_M_invoke;
+ }
+ class unique_ptr {
+ public:
+   ~unique_ptr();
+ };
+ template <typename _Tp, typename... _Args> _Tp make_unique(_Args... __args) {
+   _Tp(__args...);
+ }
+ }
+ class A {
+ public:
+   template <class T> T as();
+ };
+ class variables_map {
+ public:
+   A operator[](std::basic_string);
+ };
+ class B {
+ public:
+   variables_map configuration();
+   void run(int, int, std::function<void()>);
+ };
+ class H;
+ struct G {
+   enum {} _state;
+ };
+ class D {
+   G _local_state;
+   std::unique_ptr _task;
+   template <typename Func> void schedule(Func func) {
+     struct task_with_state {
+       task_with_state(Func func) : _func(func) {}
+       Func _func;
+     } tws = std::make_unique<task_with_state>(std::move(func));
+   }
+   friend H;
+ };
+ template <typename> using futurize_t = H;
+ class H {
+   D *_promise;
+   template <typename Func> void schedule(Func func) {
+     G __trans_tmp_1;
+     struct task_with_ready_state {
+       task_with_ready_state(Func, G);
+     };
+     std::make_unique<task_with_ready_state>(std::move(func), __trans_tmp_1);
+     _promise->schedule(std::move(func));
+   }
+   template <typename Func, typename Param> void then(Func func, Param) {
+     using P = D;
+     P pr;
+     schedule([ pr = std::move(pr), func, param = std::forward<Param> ]{});
+   }
+ 
+ public:
+   template <typename Func> futurize_t<std::result_of_t<Func()>> then(Func) {
+     then(0, [] {});
+   }
+ } clients;
+ main() {
+   B app;
+   app.run(0, 0, [&] {
+     auto config = app.configuration()[0].as<std::string>();
+     clients.then([] {});
+   });
+ }
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 224363)
+++ gcc/dwarf2out.c	(working copy)
@@ -19697,6 +19697,28 @@ is_naming_typedef_decl (const_tree decl)
 	      != TYPE_NAME (TREE_TYPE (decl))));
 }
 
+/* Looks up the DIE for a context.  */
+
+static inline dw_die_ref
+lookup_context_die (tree context)
+{
+  if (context)
+    {
+      /* Find die that represents this context.  */
+      if (TYPE_P (context))
+	{
+	  context = TYPE_MAIN_VARIANT (context);
+	  dw_die_ref ctx = lookup_type_die (context);
+	  if (!ctx)
+	    return NULL;
+	  return strip_naming_typedef (context, ctx);
+	}
+      else
+	return lookup_decl_die (context);
+    }
+  return comp_unit_die ();
+}
+
 /* Returns the DIE for a context.  */
 
 static inline dw_die_ref
@@ -22751,8 +22773,25 @@ resolve_addr (dw_die_ref die)
 		&& DECL_EXTERNAL (tdecl)
 		&& DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE)
 	      {
-		force_decl_die (tdecl);
-		tdie = lookup_decl_die (tdecl);
+		dw_die_ref cdie;
+		if (!in_lto_p)
+		  {
+		    force_decl_die (tdecl);
+		    tdie = lookup_decl_die (tdecl);
+		  }
+		else if ((cdie = lookup_context_die (DECL_CONTEXT (tdecl))))
+		  {
+		    /* Creating a full DIE for tdecl is overly expensive and
+		       at this point even wrong when in the LTO phase
+		       as it can end up generating new type DIEs we didn't
+		       output and thus optimize_external_refs will crash.  */
+		    tdie = new_die (DW_TAG_subprogram, cdie, NULL_TREE);
+		    add_AT_flag (tdie, DW_AT_external, 1);
+		    add_AT_flag (tdie, DW_AT_declaration, 1);
+		    add_linkage_attr (tdie, tdecl);
+		    add_name_and_src_coords_attributes (tdie, tdecl);
+		    equate_decl_number_to_die (tdecl, tdie);
+		  }
 	      }
 	    if (tdie)
 	      {

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-10 14:31       ` Pierre-Marie de Rodat
@ 2015-06-11  8:02         ` Richard Biener
  2015-06-11  9:10         ` Richard Biener
  1 sibling, 0 replies; 16+ messages in thread
From: Richard Biener @ 2015-06-11  8:02 UTC (permalink / raw)
  To: Pierre-Marie de Rodat; +Cc: GCC Patches

On Wed, 10 Jun 2015, Pierre-Marie de Rodat wrote:

> On 06/10/2015 03:36 PM, Richard Biener wrote:
> > Hmm, yes.  It meant to break after the first ;)  (without LTO
> > there usually is only one TU decl, apart from Java I think).
> > The hunk isn't in mainline because it was part of an experimental patch I
> > did on the early-debug branch.
> 
> Understood, thanks!
> 
> > Yeah, that looks great!
> > 
> > Of course I wonder about Java (builds multiple ones, one for each
> > input file) and Go (no idea).  I suppose Java would need to build
> > another one where all the "defaults" go (or it doesn't have any
> > such entities).
> 
> Yeah, I'm not familiar with them neither...
> 
> > In theory we could have changed dwarf2out_init to get a
> > translation-unit-decl argument as well.  But your patch looks like
> > we don't have such at the point of dwarf2out_init in all frontends.
> > 
> > Your patch is ok (and ok to backport) IMHO, though please give
> > others the chance to chime in.
> 
> Thank you very much. :-) I will soon become unavailable until July, so I think
> I'll wait until then to commit anyway.

Ok - can you please open a bugreport for the regression on the 4.9
branch so we don't miss the fix for the next 4.9 release which should
be in about 3 weeks from now (in fact you might miss the RC deadline
for that release).

So I suggest you commit the patch to trunk this week so others can
take care of backporting.

Thanks,
Richard.

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

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-10 13:37     ` Richard Biener
@ 2015-06-10 14:31       ` Pierre-Marie de Rodat
  2015-06-11  8:02         ` Richard Biener
  2015-06-11  9:10         ` Richard Biener
  0 siblings, 2 replies; 16+ messages in thread
From: Pierre-Marie de Rodat @ 2015-06-10 14:31 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On 06/10/2015 03:36 PM, Richard Biener wrote:
> Hmm, yes.  It meant to break after the first ;)  (without LTO
> there usually is only one TU decl, apart from Java I think).
> The hunk isn't in mainline because it was part of an experimental patch I
> did on the early-debug branch.

Understood, thanks!

> Yeah, that looks great!
>
> Of course I wonder about Java (builds multiple ones, one for each
> input file) and Go (no idea).  I suppose Java would need to build
> another one where all the "defaults" go (or it doesn't have any
> such entities).

Yeah, I'm not familiar with them neither...

> In theory we could have changed dwarf2out_init to get a
> translation-unit-decl argument as well.  But your patch looks like
> we don't have such at the point of dwarf2out_init in all frontends.
>
> Your patch is ok (and ok to backport) IMHO, though please give
> others the chance to chime in.

Thank you very much. :-) I will soon become unavailable until July, so I 
think I'll wait until then to commit anyway.

-- 
Pierre-Marie de Rodat

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-10 13:09   ` Pierre-Marie de Rodat
@ 2015-06-10 13:37     ` Richard Biener
  2015-06-10 14:31       ` Pierre-Marie de Rodat
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Biener @ 2015-06-10 13:37 UTC (permalink / raw)
  To: Pierre-Marie de Rodat; +Cc: GCC Patches

On Wed, 10 Jun 2015, Pierre-Marie de Rodat wrote:

> Thank you for your answer, Richard!
> 
> On 06/10/2015 08:58 AM, Richard Biener wrote:
> > Hmm, so the underlying issue is that we don't associate comp_unit_die ()
> > with any TRANSLATION_UNIT_DECL.
> 
> Indeed.
> 
> > For the LTO early debug work I did the following at the very
> > beginning of dwarf2out_early_finish:
> > 
> >    /* Pick the first TRANSLATION_UNIT_DECL we didn't create a DIE for
> >       and equate it with our default CU DIE.  LTO output needs to be
> >       able to lookup DIEs for translation unit decls.  */
> >    unsigned i;
> >    tree decl;
> >    FOR_EACH_VEC_SAFE_ELT (all_translation_units, i, decl)
> >      if (!lookup_decl_die (decl))
> >        equate_decl_number_to_die (decl, comp_unit_die ());
> 
> If I understand correctly, this does not only "pick the first" (as the comment
> says) but do equate for all DIE-less units, right? Why isn't this hunk in
> mainline yet by the way?

Hmm, yes.  It meant to break after the first ;)  (without LTO
there usually is only one TU decl, apart from Java I think).
The hunk isn't in mainline because it was part of an experimental patch I 
did on the early-debug branch.

> 
> > We create some DIEs for builtin types too early before frontends
> > got a chance to build their TRANSLATION_UNIT_DECL, so comp_unit_die
> > gets created before there is any TRANSLATION_UNIT_DECL.  Another
> > approach would of course be that the Frontends register their main
> > TRANSLATION_UNIT_DECL with dwarf2out via a debug hook.
> > 
> > But - does the above work for you and fix the regression?  If so
> > adding a debug hook would probably be the cleaner solution still.
> 
> Yes it does, thanks! However we need to fix this on the 4.9 branch as well and
> this patch would need reworking to be applied there (no debug early). So
> here's a patch that introduce a register_main_translation_unit debug hook: is
> this what you had in mind? This works for me on the 4.9 branch and on mainline
> as well, regtested on x86_64-linux.

Yeah, that looks great!

Of course I wonder about Java (builds multiple ones, one for each
input file) and Go (no idea).  I suppose Java would need to build
another one where all the "defaults" go (or it doesn't have any
such entities).

In theory we could have changed dwarf2out_init to get a
translation-unit-decl argument as well.  But your patch looks like
we don't have such at the point of dwarf2out_init in all frontends.

Your patch is ok (and ok to backport) IMHO, though please give
others the chance to chime in.

Thanks,
Richard.

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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-10  7:06 ` Richard Biener
@ 2015-06-10 13:09   ` Pierre-Marie de Rodat
  2015-06-10 13:37     ` Richard Biener
  0 siblings, 1 reply; 16+ messages in thread
From: Pierre-Marie de Rodat @ 2015-06-10 13:09 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]

Thank you for your answer, Richard!

On 06/10/2015 08:58 AM, Richard Biener wrote:
> Hmm, so the underlying issue is that we don't associate comp_unit_die ()
> with any TRANSLATION_UNIT_DECL.

Indeed.

> For the LTO early debug work I did the following at the very
> beginning of dwarf2out_early_finish:
>
>    /* Pick the first TRANSLATION_UNIT_DECL we didn't create a DIE for
>       and equate it with our default CU DIE.  LTO output needs to be
>       able to lookup DIEs for translation unit decls.  */
>    unsigned i;
>    tree decl;
>    FOR_EACH_VEC_SAFE_ELT (all_translation_units, i, decl)
>      if (!lookup_decl_die (decl))
>        equate_decl_number_to_die (decl, comp_unit_die ());

If I understand correctly, this does not only "pick the first" (as the 
comment says) but do equate for all DIE-less units, right? Why isn't 
this hunk in mainline yet by the way?

> We create some DIEs for builtin types too early before frontends
> got a chance to build their TRANSLATION_UNIT_DECL, so comp_unit_die
> gets created before there is any TRANSLATION_UNIT_DECL.  Another
> approach would of course be that the Frontends register their main
> TRANSLATION_UNIT_DECL with dwarf2out via a debug hook.
>
> But - does the above work for you and fix the regression?  If so
> adding a debug hook would probably be the cleaner solution still.

Yes it does, thanks! However we need to fix this on the 4.9 branch as 
well and this patch would need reworking to be applied there (no debug 
early). So here's a patch that introduce a 
register_main_translation_unit debug hook: is this what you had in mind? 
This works for me on the 4.9 branch and on mainline as well, regtested 
on x86_64-linux.

-- 
Pierre-Marie de Rodat

[-- Attachment #2: 0001-Restore-DW_AT_abstract_origin-for-cross-unit-call-si.patch --]
[-- Type: text/x-diff, Size: 8604 bytes --]

From 285ea98dfdf3d45f32fc0141aa182f07d172612d Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Wed, 10 Jun 2015 10:59:40 +0200
Subject: [PATCH] Restore DW_AT_abstract_origin for cross-unit call sites

gcc/ChangeLog:
	* debug.h (struct gcc_debug_hooks): Add a
	register_main_translation_unit hook.
	* debug.c (do_nothing_debug_hooks): Provide a function for this
	new hook.
	* dbxout.c (dbx_debug_hooks): Likewise.
	* sdbout.c (sdb_debug_hooks): Likewise.
	* vmsdbgout.c (vmsdbg_debug_hooks): Likewise.
	* dwarf2out.c (main_translation_unit): New global variable.
	(dwarf2out_register_main_translation_unit): New function
	implementing the new hook.
	(dwarf2_debug_hooks): Assign
	dwarf2out_register_main_translation_unit to this new hook.
	(dwarf2out_init): Associate any main translation unit to
	comp_unit_die ().
	* c/c-decl.c (pop_scope): Register the main translation unit
	through the new debug hook.
	* cp/decl.c (cxx_init_decl_processing): Likewise.

gcc/ada/ChangeLog:
	* gcc-interface/utils.c (get_global_context): Register the main
	translation unit through the new debug hook.

gcc/fortran/ChangeLog:
	* f95-lang.c (gfc_create_decls): Register the main translation
	unit through the new debug hook.
---
 gcc/ada/gcc-interface/utils.c |  5 ++++-
 gcc/c/c-decl.c                |  1 +
 gcc/cp/decl.c                 |  2 ++
 gcc/dbxout.c                  |  1 +
 gcc/debug.c                   |  1 +
 gcc/debug.h                   |  4 ++++
 gcc/dwarf2out.c               | 27 +++++++++++++++++++++++++++
 gcc/fortran/f95-lang.c        |  1 +
 gcc/sdbout.c                  |  1 +
 gcc/vmsdbgout.c               |  1 +
 10 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index 9076529..655bfa1 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -667,7 +667,10 @@ static tree
 get_global_context (void)
 {
   if (!global_context)
-    global_context = build_translation_unit_decl (NULL_TREE);
+    {
+      global_context = build_translation_unit_decl (NULL_TREE);
+      debug_hooks->register_main_translation_unit (global_context);
+    }
   return global_context;
 }
 
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 7fd662d..3fde22f 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1196,6 +1196,7 @@ pop_scope (void)
     {
       tree file_decl = build_translation_unit_decl (NULL_TREE);
       context = file_decl;
+      debug_hooks->register_main_translation_unit (file_decl);
     }
   else
     context = block;
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 3bed538..ffd068a 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -3831,6 +3831,8 @@ cxx_init_decl_processing (void)
   global_namespace = build_lang_decl (NAMESPACE_DECL, global_scope_name,
 				      void_type_node);
   DECL_CONTEXT (global_namespace) = build_translation_unit_decl (NULL_TREE);
+  debug_hooks->register_main_translation_unit
+    (DECL_CONTEXT (global_namespace));
   TREE_PUBLIC (global_namespace) = 1;
   begin_scope (sk_namespace, global_namespace);
 
diff --git a/gcc/dbxout.c b/gcc/dbxout.c
index 48b5065..94fac42 100644
--- a/gcc/dbxout.c
+++ b/gcc/dbxout.c
@@ -380,6 +380,7 @@ const struct gcc_debug_hooks dbx_debug_hooks =
   debug_nothing_tree,		         /* begin_function */
 #endif
   debug_nothing_int,		         /* end_function */
+  debug_nothing_tree,			 /* register_main_translation_unit */
   dbxout_function_decl,
   dbxout_early_global_decl,		 /* early_global_decl */
   dbxout_late_global_decl,		 /* late_global_decl */
diff --git a/gcc/debug.c b/gcc/debug.c
index 9c621f8..ab92cc8 100644
--- a/gcc/debug.c
+++ b/gcc/debug.c
@@ -46,6 +46,7 @@ const struct gcc_debug_hooks do_nothing_debug_hooks =
   debug_nothing_int_charstar,	         /* end_epilogue */
   debug_nothing_tree,		         /* begin_function */
   debug_nothing_int,		         /* end_function */
+  debug_nothing_tree,		         /* register_main_translation_unit */
   debug_nothing_tree,		         /* function_decl */
   debug_nothing_tree,	         	 /* early_global_decl */
   debug_nothing_tree,	         	 /* late_global_decl */
diff --git a/gcc/debug.h b/gcc/debug.h
index e7e1334..269c4d8 100644
--- a/gcc/debug.h
+++ b/gcc/debug.h
@@ -89,6 +89,10 @@ struct gcc_debug_hooks
   /* Record end of function.  LINE is highest line number in function.  */
   void (* end_function) (unsigned int line);
 
+  /* Register UNIT as the main translation unit.  Called from front-ends when
+     they create their main translation unit.  */
+  void (* register_main_translation_unit) (tree);
+
   /* Debug information for a function DECL.  This might include the
      function name (a symbol), its parameters, and the block that
      makes up the function's body, and the local variables of the
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index ee2bcb1..8a36fe8 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -2446,6 +2446,7 @@ static void dwarf2out_abstract_function (tree);
 static void dwarf2out_var_location (rtx_insn *);
 static void dwarf2out_begin_function (tree);
 static void dwarf2out_end_function (unsigned int);
+static void dwarf2out_register_main_translation_unit (tree unit);
 static void dwarf2out_set_name (tree, tree);
 
 /* The debug hooks structure.  */
@@ -2475,6 +2476,7 @@ const struct gcc_debug_hooks dwarf2_debug_hooks =
   dwarf2out_end_epilogue,
   dwarf2out_begin_function,
   dwarf2out_end_function,	/* end_function */
+  dwarf2out_register_main_translation_unit,
   dwarf2out_function_decl,	/* function_decl */
   dwarf2out_early_global_decl,
   dwarf2out_late_global_decl,
@@ -22505,6 +22507,26 @@ dwarf2out_end_function (unsigned int)
   maybe_at_text_label_p = false;
 }
 
+/* Temporary holder for dwarf2out_register_main_translation_unit.  Used to let
+   front-ends register a translation unit even before dwarf2out_init is
+   called.  */
+static tree main_translation_unit = NULL_TREE;
+
+/* Hook called by front-ends after they built their main translation unit.
+   Associate comp_unit_die to UNIT.  */
+
+static void
+dwarf2out_register_main_translation_unit (tree unit)
+{
+  gcc_assert (TREE_CODE (unit) == TRANSLATION_UNIT_DECL
+	      && main_translation_unit == NULL_TREE);
+  main_translation_unit = unit;
+  /* If dwarf2out_init has not been called yet, it will perform the association
+     itself looking at main_translation_unit.  */
+  if (decl_die_table != NULL)
+    equate_decl_number_to_die (unit, comp_unit_die ());
+}
+
 /* Add OPCODE+VAL as an entry at the end of the opcode array in TABLE.  */
 
 static void
@@ -23242,6 +23264,11 @@ dwarf2out_init (const char *filename ATTRIBUTE_UNUSED)
   /* Make sure the line number table for .text always exists.  */
   text_section_line_info = new_line_info_table ();
   text_section_line_info->end_label = text_end_label;
+
+  /* If front-ends already registered a main translation unit but we were not
+     ready to perform the association, do this now.  */
+  if (main_translation_unit != NULL_TREE)
+    equate_decl_number_to_die (main_translation_unit, comp_unit_die ());
 }
 
 /* Called before compile () starts outputtting functions, variables
diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
index 6daac83..2c055f5 100644
--- a/gcc/fortran/f95-lang.c
+++ b/gcc/fortran/f95-lang.c
@@ -204,6 +204,7 @@ gfc_create_decls (void)
 
   /* Build our translation-unit decl.  */
   current_translation_unit = build_translation_unit_decl (NULL_TREE);
+  debug_hooks->register_main_translation_unit (current_translation_unit);
 }
 
 
diff --git a/gcc/sdbout.c b/gcc/sdbout.c
index f5671c6..033886a 100644
--- a/gcc/sdbout.c
+++ b/gcc/sdbout.c
@@ -296,6 +296,7 @@ const struct gcc_debug_hooks sdb_debug_hooks =
   sdbout_end_epilogue,		         /* end_epilogue */
   sdbout_begin_function,	         /* begin_function */
   sdbout_end_function,		         /* end_function */
+  debug_nothing_tree,		         /* register_main_translation_unit */
   debug_nothing_tree,		         /* function_decl */
   sdbout_early_global_decl,		 /* early_global_decl */
   sdbout_late_global_decl,		 /* late_global_decl */
diff --git a/gcc/vmsdbgout.c b/gcc/vmsdbgout.c
index 8297e02..8c917e0 100644
--- a/gcc/vmsdbgout.c
+++ b/gcc/vmsdbgout.c
@@ -194,6 +194,7 @@ const struct gcc_debug_hooks vmsdbg_debug_hooks
    vmsdbgout_end_epilogue,
    vmsdbgout_begin_function,
    vmsdbgout_end_function,
+   debug_nothing_tree,		  /* register_main_translation_unit */
    vmsdbgout_function_decl,
    vmsdbgout_early_global_decl,
    vmsdbgout_late_global_decl,
-- 
2.3.3.199.g52cae64


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

* Re: [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
  2015-06-09 21:39 Pierre-Marie de Rodat
@ 2015-06-10  7:06 ` Richard Biener
  2015-06-10 13:09   ` Pierre-Marie de Rodat
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Biener @ 2015-06-10  7:06 UTC (permalink / raw)
  To: Pierre-Marie de Rodat; +Cc: GCC Patches

On Tue, 9 Jun 2015, Pierre-Marie de Rodat wrote:

> Hello,
> 
> With the recent work for PR debug/65549, we observed a regression in the
> generated debugging information. Take the attached reproducer, build it and
> look at the output DWARF:
> 
>      $ gcc -c -O1 -g foo.c
>      $ objdump --dwarf=info foo.o
>      [...]
>       <2><4e>: Abbrev Number: 3 (DW_TAG_GNU_call_site)
>          <4f>   DW_AT_low_pc      : 0x9
>       <2><57>: Abbrev Number: 0
> 
> There is no DW_AT_abstract_origin attribute anymore, whereas there used to be
> one.
> 
> On PowerPC with -mlongcall, for instance, call instructions are indirect and
> we (at AdaCore) rely on this attribute to determine what function is actually
> called (it's easier this way than interpreting machine code...). The DWARF
> proposal for call sites also say debuggers should be able to use this
> attribute (to build virtual call stacks in the presence of tail calls), but I
> could not observe this in practice with GDB.
> 
> The attached patch is an attempt to restore this attribute. The logic behind
> it is: this is what we used to do previously for contexts that are alien
> translation unit (through the call to force_decl_die), so this should not
> harm.
> 
> Bootstrapped and regtested on x86_64-linux. Ok to commit?

Hmm, so the underlying issue is that we don't associate comp_unit_die ()
with any TRANSLATION_UNIT_DECL.  For the LTO early debug work I did
the following at the very beginning of dwarf2out_early_finish:

  /* Pick the first TRANSLATION_UNIT_DECL we didn't create a DIE for
     and equate it with our default CU DIE.  LTO output needs to be
     able to lookup DIEs for translation unit decls.  */
  unsigned i;
  tree decl;
  FOR_EACH_VEC_SAFE_ELT (all_translation_units, i, decl)
    if (!lookup_decl_die (decl))
      equate_decl_number_to_die (decl, comp_unit_die ());

We create some DIEs for builtin types too early before frontends
got a chance to build their TRANSLATION_UNIT_DECL, so comp_unit_die
gets created before there is any TRANSLATION_UNIT_DECL.  Another
approach would of course be that the Frontends register their main
TRANSLATION_UNIT_DECL with dwarf2out via a debug hook.

But - does the above work for you and fix the regression?  If so
adding a debug hook would probably be the cleaner solution still.

Thanks,
Richard.

> Thank you in advance!
> 
> gcc/ChangeLog
>         * dwarf2out.c (lookup_context_die): Return the DIE for the
>         current compilation unit when the input context is a
>         TRANSLATION_UNIT_DECL.
> 
> 

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

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

* [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites
@ 2015-06-09 21:39 Pierre-Marie de Rodat
  2015-06-10  7:06 ` Richard Biener
  0 siblings, 1 reply; 16+ messages in thread
From: Pierre-Marie de Rodat @ 2015-06-09 21:39 UTC (permalink / raw)
  To: GCC Patches, Richard Biener

[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]

Hello,

With the recent work for PR debug/65549, we observed a regression in the 
generated debugging information. Take the attached reproducer, build it 
and look at the output DWARF:

      $ gcc -c -O1 -g foo.c
      $ objdump --dwarf=info foo.o
      [...]
       <2><4e>: Abbrev Number: 3 (DW_TAG_GNU_call_site)
          <4f>   DW_AT_low_pc      : 0x9
       <2><57>: Abbrev Number: 0

There is no DW_AT_abstract_origin attribute anymore, whereas there used 
to be one.

On PowerPC with -mlongcall, for instance, call instructions are indirect 
and we (at AdaCore) rely on this attribute to determine what function is 
actually called (it's easier this way than interpreting machine 
code...). The DWARF proposal for call sites also say debuggers should be 
able to use this attribute (to build virtual call stacks in the presence 
of tail calls), but I could not observe this in practice with GDB.

The attached patch is an attempt to restore this attribute. The logic 
behind it is: this is what we used to do previously for contexts that 
are alien translation unit (through the call to force_decl_die), so this 
should not harm.

Bootstrapped and regtested on x86_64-linux. Ok to commit?
Thank you in advance!

gcc/ChangeLog
         * dwarf2out.c (lookup_context_die): Return the DIE for the
         current compilation unit when the input context is a
         TRANSLATION_UNIT_DECL.

-- 
Pierre-Marie de Rodat


[-- Attachment #2: foo.c --]
[-- Type: text/x-csrc, Size: 96 bytes --]

extern int bar(int a);

int foo(int a)
{
  a += 1;
  return a;
}

int main(void)
{
  bar(1);
}


[-- Attachment #3: 0001-DWARF-restore-DW_AT_abstract_origin-for-cross-unit-c.patch --]
[-- Type: text/x-patch, Size: 1586 bytes --]

From b2ededa4a5eac77fc0d5d95011de935a1dff7eba Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Mon, 8 Jun 2015 16:27:04 +0200
Subject: [PATCH] DWARF: restore DW_AT_abstract_origin for cross-unit call
 sites

gcc/ChangeLog
	* dwarf2out.c (lookup_context_die): Return the DIE for the
	current compilation unit when the input context is a
	TRANSLATION_UNIT_DECL.
---
 gcc/dwarf2out.c |   25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index ee2bcb1..4e204df 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -21053,21 +21053,20 @@ is_naming_typedef_decl (const_tree decl)
 static inline dw_die_ref
 lookup_context_die (tree context)
 {
-  if (context)
+  if (context == NULL_TREE || TREE_CODE (context) == TRANSLATION_UNIT_DECL)
+    return comp_unit_die ();
+
+  /* Find die that represents this context.  */
+  if (TYPE_P (context))
     {
-      /* Find die that represents this context.  */
-      if (TYPE_P (context))
-	{
-	  context = TYPE_MAIN_VARIANT (context);
-	  dw_die_ref ctx = lookup_type_die (context);
-	  if (!ctx)
-	    return NULL;
-	  return strip_naming_typedef (context, ctx);
-	}
-      else
-	return lookup_decl_die (context);
+      context = TYPE_MAIN_VARIANT (context);
+      dw_die_ref ctx = lookup_type_die (context);
+      if (!ctx)
+	return NULL;
+      return strip_naming_typedef (context, ctx);
     }
-  return comp_unit_die ();
+  else
+    return lookup_decl_die (context);
 }
 
 /* Returns the DIE for a context.  */
-- 
1.7.10.4


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

end of thread, other threads:[~2015-06-12 13:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-12  1:25 [PATCH][PR debug/65549] Restore DW_AT_abstract_origin for cross-unit call sites David Edelsohn
2015-06-12  5:45 ` Richard Biener
2015-06-12 10:15   ` Pierre-Marie de Rodat
2015-06-12 10:32   ` Pierre-Marie de Rodat
2015-06-12 11:17     ` Richard Biener
2015-06-12 13:56       ` Pierre-Marie de Rodat
  -- strict thread matches above, loose matches on Subject: below --
2015-06-09 21:39 Pierre-Marie de Rodat
2015-06-10  7:06 ` Richard Biener
2015-06-10 13:09   ` Pierre-Marie de Rodat
2015-06-10 13:37     ` Richard Biener
2015-06-10 14:31       ` Pierre-Marie de Rodat
2015-06-11  8:02         ` Richard Biener
2015-06-11  9:10         ` Richard Biener
2015-06-11  9:21           ` Pierre-Marie de Rodat
2015-06-11  9:25             ` Richard Biener
2015-06-11 13:49               ` Pierre-Marie de Rodat

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