diff mbox

[PING] DWARF: process all TYPE_DECL nodes when iterating on scopes

Message ID 1e0e170c-9760-ba85-3b75-f1928b6b9fa9@adacore.com
State New
Headers show

Commit Message

Pierre-Marie de Rodat Sept. 27, 2016, 2:01 p.m. UTC
On 09/07/2016 11:30 AM, Richard Biener wrote:
> That said, with the idea of early debug in place and thus giving
> more responsibility to the frontends I wonder in what order the Ada
> FE calls debug_hooks.early_global_decl ()? Maybe it's the middle-end
> and it should arrange for a more natural order on function nests.  So
> I'd appreciate if you can investigate this side of the issue a bit,
> that is, simply avoid the bad ordering.

I investigated this track: it’s not the Ada front-end’s calls to the 
debug_hooks.early_global_decl that matter, but actually the ones in 
cgraphunit.c (symbol_table::finalize_compilation_unit).

The new patch attached tries to fix the call order. Bootstrapping and 
regtesting on x86_64-linux were clean except one testcase where the 
debug output changed legitimally (matcher updated). Ok to commit? Thank 
you in advance!

Comments

Richard Biener Sept. 28, 2016, 7:48 a.m. UTC | #1
On Tue, Sep 27, 2016 at 4:01 PM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> On 09/07/2016 11:30 AM, Richard Biener wrote:
>>
>> That said, with the idea of early debug in place and thus giving
>> more responsibility to the frontends I wonder in what order the Ada
>> FE calls debug_hooks.early_global_decl ()? Maybe it's the middle-end
>> and it should arrange for a more natural order on function nests.  So
>> I'd appreciate if you can investigate this side of the issue a bit,
>> that is, simply avoid the bad ordering.
>
>
> I investigated this track: it’s not the Ada front-end’s calls to the
> debug_hooks.early_global_decl that matter, but actually the ones in
> cgraphunit.c (symbol_table::finalize_compilation_unit).
>
> The new patch attached tries to fix the call order. Bootstrapping and
> regtesting on x86_64-linux were clean except one testcase where the debug
> output changed legitimally (matcher updated). Ok to commit? Thank you in
> advance!

Hmm, interesting approach.  It might work reliably at this point of
the compilation
but we do actually recycle cgraph nodes.  So I wonder if given we do have
->origin / ->next_nested in the cgraph if we can simply perform a walk in the
appropriate order.

But then OTOH in my early LTO debug patchset I have

Index: early-lto-debug/gcc/dwarf2out.c
===================================================================
--- early-lto-debug.orig/gcc/dwarf2out.c        2016-09-26
15:51:37.666113699 +0200
+++ early-lto-debug/gcc/dwarf2out.c     2016-09-27 08:52:20.802507268 +0200
@@ -23837,6 +24261,16 @@ dwarf2out_early_global_decl (tree decl)
          if (!DECL_STRUCT_FUNCTION (decl))
            goto early_decl_exit;

+         /* Emit an abstract origin of a function first.  This happens
+            with C++ constructor clones for example and makes
+            dwarf2out_abstract_function happy which requires the early
+            DIE of the abstract instance to be present.  */
+         if (DECL_ABSTRACT_ORIGIN (decl))
+           {
+             current_function_decl = DECL_ABSTRACT_ORIGIN (decl);
+             dwarf2out_decl (DECL_ABSTRACT_ORIGIN (decl));
+           }
+
          current_function_decl = decl;
        }
       dwarf2out_decl (decl);

which is not 100% equivalent (looking at the abstract origin) but it
sounds like a
similar issue.  So I wonder if doing the same for DECL_CONTEXT being a function
makes sense here and would also fix your particular issue in a more
reliable way.

Thanks,
Richard.

> --
> Pierre-Marie de Rodat
diff mbox

Patch

From 2508b2be4d58ae29cc0093688c50fefea15f6934 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Fri, 23 Sep 2016 18:16:07 +0200
Subject: [PATCH] DWARF: fix scoping for descriptions of local types

In Ada, it is possible to have nested subprograms in the following
configuration:

    procedure Parent is
       type T;
       [...]
       procedure Child (Value : T) is
       begin
          [...]
       end Child;
    begin
       [...]
    end Parent;

As we currently generate debugging information for Child first before
Parent, the debug info for T appears in global scope since the DIE for
Parent does not exist yet.

First, this patch makes sure nested function cgraph_node's are created
after their parents'.  Then, it reverses the iteration on the symbol
table that calls the early_global_decl debug hook so that debug info for
nested functions is generated after their parents'.

gcc/ChangeLog:

	* cgraph.h (symbol_table::last_function_with_gimple_body,
	symbol_table::previous_function_with_gimple_body): New methods.
	(REVERSE_FOR_EACH_FUNCTION_WITH_GIMPLE_BODY): New iteration macro.
	* cgraph.c (symbol_table::call_cgraph_duplication_hooks): Adjust so
	that nested functions have their cgraph_node created after their
	parents'.
	* cgraphunit.c (symbol_table::finalize_compilation_unit): Call the
	early_global_decl debug hook on last functions in the symtab list (i.e.
	older ones first).

gcc/testsuite/

	* gcc/testsuite/g++.dg/debug/dwarf2/auto1.C: Adjust pattern to
	accomodate new DIEs output order.
---
 gcc/cgraph.c                              | 12 ++++++++--
 gcc/cgraph.h                              | 40 +++++++++++++++++++++++++++++++
 gcc/cgraphunit.c                          |  5 ++--
 gcc/testsuite/g++.dg/debug/dwarf2/auto1.C |  2 +-
 4 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index b702a7c..78d1a85 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -492,6 +492,14 @@  symbol_table::call_cgraph_duplication_hooks (cgraph_node *node,
 cgraph_node *
 cgraph_node::create (tree decl)
 {
+  cgraph_node *origin = NULL;
+
+  /* If DECL is local to a function, make sure we have a node for this function
+     first so that the symbol table has parent functions created before their
+     children.  This helps debug information generation.  */
+  if (DECL_CONTEXT (decl) && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
+    origin = cgraph_node::get_create (DECL_CONTEXT (decl));
+
   cgraph_node *node = symtab->create_empty ();
   gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
 
@@ -507,9 +515,9 @@  cgraph_node::create (tree decl)
 
   node->register_symbol ();
 
-  if (DECL_CONTEXT (decl) && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
+  if (origin != NULL)
     {
-      node->origin = cgraph_node::get_create (DECL_CONTEXT (decl));
+      node->origin = origin;
       node->next_nested = node->origin->nested;
       node->origin->nested = node;
     }
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index ecafe63..6b1181c 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -2097,6 +2097,13 @@  public:
   /* Return next reachable static variable with initializer after NODE.  */
   inline cgraph_node *next_function_with_gimple_body (cgraph_node *node);
 
+  /* Return last function with body defined.  */
+  cgraph_node *last_function_with_gimple_body (void);
+
+  /* Return previous reachable static variable with initializer before
+     NODE.  */
+  inline cgraph_node *previous_function_with_gimple_body (cgraph_node *node);
+
   /* Register HOOK to be called with DATA on each removed edge.  */
   cgraph_edge_hook_list *add_edge_removal_hook (cgraph_edge_hook hook,
 						void *data);
@@ -2775,6 +2782,36 @@  symbol_table::next_function_with_gimple_body (cgraph_node *node)
   return NULL;
 }
 
+/* Return last function with body defined.  */
+inline cgraph_node *
+symbol_table::last_function_with_gimple_body (void)
+{
+  cgraph_node *res = NULL;
+  symtab_node *node;
+
+  for (node = nodes; node; node = node->next)
+    {
+      cgraph_node *cn = dyn_cast <cgraph_node *> (node);
+      if (cn && cn->has_gimple_body_p ())
+	res = cn;
+    }
+  return res;
+}
+
+/* Return pverious reachable static variable with initializer before NODE.  */
+inline cgraph_node *
+symbol_table::previous_function_with_gimple_body (cgraph_node *node)
+{
+  symtab_node *node1 = node->previous;
+  for (; node1; node1 = node1->previous)
+    {
+      cgraph_node *cn1 = dyn_cast <cgraph_node *> (node1);
+      if (cn1 && cn1->has_gimple_body_p ())
+	return cn1;
+    }
+  return NULL;
+}
+
 /* Walk all functions.  */
 #define FOR_EACH_FUNCTION(node) \
    for ((node) = symtab->first_function (); (node); \
@@ -2796,6 +2833,9 @@  cgraph_node::has_gimple_body_p (void)
 #define FOR_EACH_FUNCTION_WITH_GIMPLE_BODY(node) \
    for ((node) = symtab->first_function_with_gimple_body (); (node); \
 	(node) = symtab->next_function_with_gimple_body (node))
+#define REVERSE_FOR_EACH_FUNCTION_WITH_GIMPLE_BODY(node) \
+   for ((node) = symtab->last_function_with_gimple_body (); (node); \
+	(node) = symtab->previous_function_with_gimple_body (node))
 
 /* Uniquize all constants that appear in memory.
    Each constant in memory thus far output is recorded
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index e38f0bf..66aa961 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -2562,9 +2562,10 @@  symbol_table::finalize_compilation_unit (void)
   if (!seen_error ())
     {
       /* Emit early debug for reachable functions, and by consequence,
-	 locally scoped symbols.  */
+	 locally scoped symbols.  Process functions in reverse order so that
+	 nested functions come after their parents.  */
       struct cgraph_node *cnode;
-      FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (cnode)
+      REVERSE_FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (cnode)
 	(*debug_hooks->early_global_decl) (cnode->decl);
 
       /* Clean up anything that needs cleaning up after initial debug
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/auto1.C b/gcc/testsuite/g++.dg/debug/dwarf2/auto1.C
index 5daf3cd..6442172 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/auto1.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/auto1.C
@@ -17,7 +17,7 @@ 
 // .long   0x33    # DW_AT_specification
 // .long   0x87    # DW_AT_type
 
-// { dg-final { scan-assembler "a1.*(0x\[0-9a-f]+)\[^\n\r]*DW_AT_type.*\\1. DW_TAG_unspecified_type.*(0x\[0-9a-f]+). DW_TAG_base_type.*DW_AT_specification\[\n\r]{1,2}\[^\n\r]*\\2\[^\n\r]*DW_AT_type" } }
+// { dg-final { scan-assembler "a1.*(0x\[0-9a-f]+)\[^\n\r]*DW_AT_type.*\\1. DW_TAG_unspecified_type.*DW_AT_specification\[\n\r]{1,2}\[^\n\r]*(0x\[0-9a-f]+)\[^\n\r]*DW_AT_type.*\\2. DW_TAG_base_type" } }
 
 struct A
 {
-- 
2.10.0