[PATCH 1/2] DWARF: process all TYPE_DECL nodes when iterating on scopes

[PATCH 1/2] DWARF: process all TYPE_DECL nodes when iterating on scopes

[Pierre-Marie de Rodat]

Hello,

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;

If we generate debugging information for Child first before Parent, the
debug info for T will be generated at global scope since the DIE for
Parent does not exist yet. It is when generating debug info for Parent
that we are supposed to relocate it thanks to decls_for_scope and
process_scope_var.

However, process_scope_var currently works only on TYPE_DECL nodes that
are stubs, for unknown reasons. This change adapts it to work on all
TYPE_DECL nodes.

It bootstrapped and regtested fine on x86_64-linux and triggered to
regression in the GDB testsuite for Ada, C, C++ and Fortran. Ok to
commit? Thank you in advance!

gcc/ChangeLog:

	* dwarf2out.c (process_scope_var): Relocate TYPE_DECL nodes that
	are not stubs just like stub ones.
---
  gcc/dwarf2out.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 2c0bd63..da5524e 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -22829,8 +22829,7 @@ process_scope_var (tree stmt, tree decl, tree 
origin, dw_die_ref context_die)
     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))
+  else if (TREE_CODE (decl_or_origin) == TYPE_DECL)
      die = lookup_type_die (TREE_TYPE (decl_or_origin));
    else
      die = NULL;
-- 
2.6.4

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

[Pierre-Marie de Rodat]

Hello,

Ping for the patch submitted in 
<https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00160.html>. Thanks!

-- 
Pierre-Marie de Rodat

Re: [PATCH 1/2] DWARF: process all TYPE_DECL nodes when iterating on scopes

[Richard Biener]

On Tue, Jan 5, 2016 at 3:32 PM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> Hello,
>
> 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;
>
> If we generate debugging information for Child first before Parent, the
> debug info for T will be generated at global scope since the DIE for
> Parent does not exist yet. It is when generating debug info for Parent
> that we are supposed to relocate it thanks to decls_for_scope and
> process_scope_var.
>
> However, process_scope_var currently works only on TYPE_DECL nodes that
> are stubs, for unknown reasons. This change adapts it to work on all
> TYPE_DECL nodes.
>
> It bootstrapped and regtested fine on x86_64-linux and triggered to
> regression in the GDB testsuite for Ada, C, C++ and Fortran. Ok to
> commit? Thank you in advance!

Looking for TYPE_DECL_IS_STUB uses I come along dwarf2out_ignore_block
which you'd need to change as well I think.

> gcc/ChangeLog:
>
>         * dwarf2out.c (process_scope_var): Relocate TYPE_DECL nodes that
>         are not stubs just like stub ones.
> ---
>  gcc/dwarf2out.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 2c0bd63..da5524e 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -22829,8 +22829,7 @@ process_scope_var (tree stmt, tree decl, tree
> origin, dw_die_ref context_die)
>     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))
> +  else if (TREE_CODE (decl_or_origin) == TYPE_DECL)
>      die = lookup_type_die (TREE_TYPE (decl_or_origin));

But ... I think this change is wrong.  It is supposed to use the _type_ DIE
in case the FE didn't create a proper TYPE_DECL.  So I think what is
maybe missing is

  else if (TREE_CODE (decl_or_origin) == TYPE_DECL)
    die = lookup_decl_die (decl_or_origin);

?  That is, why should we lookup the type if the type-decl isn't a stub?

Btw, not sure how you get at the "wrong" debug info gen order, I can't seem to
get at it with a C testcase.

As with the other patch this misses a testcase.

Richard.

>    else
>      die = NULL;
> --
> 2.6.4
>

Re: [PATCH 1/2] DWARF: process all TYPE_DECL nodes when iterating on scopes

[Pierre-Marie de Rodat]

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

On 01/18/2016 12:18 PM, Richard Biener wrote:
> Looking for TYPE_DECL_IS_STUB uses I come along dwarf2out_ignore_block
> which you'd need to change as well I think.

That is true, thank you!

First, I’ll answer your last point:
> Btw, not sure how you get at the "wrong" debug info gen order, I
> can't seem to get at it with a C testcase.
>
> As with the other patch this misses a testcase.

That’s true, sorry for that. I could not yield a C reproducer neither, 
so here’s my original Ada testcase: I will include it in the next patch 
under gnat.dg.

So the problem I’m trying to address is the following: Record_Type is 
declared in Debug5, and it is referenced by the R parameter in 
Debug5.Process (so far, so good). Now, because debug info for 
Debug5.Process is emitted before the one for Debug5, Record_Type is 
emitted in the global scope and is not relocated under Debug5 afterwards.

>> -  else if (TREE_CODE (decl_or_origin) == TYPE_DECL
>> -           && TYPE_DECL_IS_STUB (decl_or_origin))
>> +  else if (TREE_CODE (decl_or_origin) == TYPE_DECL)
>>       die = lookup_type_die (TREE_TYPE (decl_or_origin));
>
> But ... I think this change is wrong.  It is supposed to use the _type_ DIE
> in case the FE didn't create a proper TYPE_DECL.  So I think what is
> maybe missing is
>
>    else if (TREE_CODE (decl_or_origin) == TYPE_DECL)
>      die = lookup_decl_die (decl_or_origin);
>
> ?  That is, why should we lookup the type if the type-decl isn't a stub?

While I agree that for non-stub TYPE_DECL, it is sound to look for the 
decl itself rather than the type, this does not fix the bug I intended 
to fix. Indeed, when we reach this point for the TYPE_DECL corresponding 
to Record_Type, add_type_attribute just creates a DW_TAG_typedef for it, 
and the call it does to modified_type just returns the existing 
DW_TAG_record_type (still in the global scope).

So assuming your proposal of the change is correct, should we then use 
TYPE_CONTEXT in order to (potentially) relocate TREE_TYPE (decl) in 
add_type_attribute?

-- 
Pierre-Marie de Rodat

[-- Attachment #2: debug5.adb --]
[-- Type: text/x-adasrc, Size: 1563 bytes --]

--  The aim of this test is to check that Ada types appear in the proper
--  context in the debug info.
-- 
--  Checking this directly would be really tedious just scanning for assembly
--  lines, so instead we rely on DWARFv4's .debug_types sections, which must be
--  created only for global-scope types. Checking the number of .debug_types is
--  some hackish way to check that types are output in the proper context (i.e.
--  at global or local scope).
--
--  { dg-options "-g -gdwarf-4 -cargs -fdebug-types-section -dA" }
--  { dg-final { scan-assembler-times "\\(DIE \\(0x\[a-f0-9\]*\\) DW_TAG_type_unit\\)" 0 } }

procedure Debug5 is
   type Array_Type is array (Natural range <>) of Integer;
   type Record_Type (L1, L2 : Natural) is record
      I1 : Integer;
      A1 : Array_Type (1 .. L1);
      I2 : Integer;
      A2 : Array_Type (1 .. L2);
      I3 : Integer;
   end record;

   function Get (L1, L2 : Natural) return Record_Type is
      Result : Record_Type (L1, L2);
   begin
      Result.I1 := 1;
      for I in Result.A1'Range loop
         Result.A1 (I) := I;
      end loop;
      Result.I2 := 2;
      for I in Result.A2'Range loop
         Result.A2 (I) := I;
      end loop;
      Result.I3 := 3;
      return Result;
   end Get;

   R1 : Record_Type := Get (0, 0);
   R2 : Record_Type := Get (1, 0);
   R3 : Record_Type := Get (0, 1);
   R4 : Record_Type := Get (2, 2);

   procedure Process (R : Record_Type) is
   begin
      null;
   end Process;

begin
   Process (R1);
   Process (R2);
   Process (R3);
   Process (R4);
end Debug5;

Re: [PATCH 1/2] DWARF: process all TYPE_DECL nodes when iterating on scopes

[Pierre-Marie de Rodat]

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

Here is another attempt to solve the original issue. This time, with a 
proper testcase. ;-)

Rebased against trunk, boostrapped and regtested on x86_64-linux.

gcc/

     * dwarf2out.c (process_scope_var): Relocate DIEs for TYPE_DECL
     nodes that are not stubs.
     (gen_decl_die): For TYPE_DECLs that have the same scope as their
     type, relocate the type DIE if it was generated without a scope.

gcc/testsuite/

     * gnat.dg/debug9.adb: New testcase.

-- 
Pierre-Marie de Rodat

[-- Attachment #2: 0001-DWARF-process-all-TYPE_DECL-nodes-when-iterating-on-.patch --]
[-- Type: text/x-diff, Size: 4700 bytes --]

From 7b6288889a631b532d561ed66c0dc973dfbf1f5d Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Tue, 5 Jan 2016 15:32:34 +0100
Subject: [PATCH] DWARF: process all TYPE_DECL nodes when iterating on scopes

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;

If we generate debugging information for Child first before Parent, the
debug info for T will be generated at global scope since the DIE for
Parent does not exist yet.  It is when generating debug info for Parent
that we are supposed to relocate it.  This change enhances the
relocation machinery to handle the above case.

gcc/

	* dwarf2out.c (process_scope_var): Relocate DIEs for TYPE_DECL
	nodes that are not stubs.
	(gen_decl_die): For TYPE_DECLs that have the same scope as their
	type, relocate the type DIE if it was generated without a scope.

gcc/testsuite/

	* gnat.dg/debug9.adb: New testcase.
---
 gcc/dwarf2out.c                  | 18 ++++++++++++--
 gcc/testsuite/gnat.dg/debug9.adb | 53 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gnat.dg/debug9.adb

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 45eb684..2e3ca71 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -23248,6 +23248,8 @@ process_scope_var (tree stmt, tree decl, tree origin, dw_die_ref context_die)
   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)
+    die = lookup_decl_die (decl_or_origin);
   else
     die = NULL;
 
@@ -23732,8 +23734,20 @@ gen_decl_die (tree decl, tree origin, struct vlr_context *ctx,
       if (is_redundant_typedef (decl))
 	gen_type_die (TREE_TYPE (decl), context_die);
       else
-	/* Output a DIE to represent the typedef itself.  */
-	gen_typedef_die (decl, context_die);
+	{
+	  /* Output a DIE to represent the typedef itself.  */
+	  gen_typedef_die (decl, context_die);
+
+	  /* The above may create a typedef in the proper scope, but the
+	     underlying type itself could have been created earlier, at a point
+	     when the scope was not available yet.  If it's the case, relocate
+	     it.  This is analogous to what is done in process_scope_var,
+	     except we deal with a TYPE and not a DECL, here.  */
+	  dw_die_ref type_die = lookup_type_die (TREE_TYPE (decl));
+	  if (type_die != NULL && type_die->die_parent == NULL
+	      && DECL_CONTEXT (decl) == TYPE_CONTEXT (TREE_TYPE (decl)))
+	    add_child_die (context_die, type_die);
+	}
       break;
 
     case LABEL_DECL:
diff --git a/gcc/testsuite/gnat.dg/debug9.adb b/gcc/testsuite/gnat.dg/debug9.adb
new file mode 100644
index 0000000..dd14210
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug9.adb
@@ -0,0 +1,53 @@
+--  The aim of this test is to check that Ada types appear in the proper
+--  context in the debug info.
+-- 
+--  Checking this directly would be really tedious just scanning for assembly
+--  lines, so instead we rely on DWARFv4's .debug_types sections, which must be
+--  created only for global-scope types. Checking the number of .debug_types is
+--  some hackish way to check that types are output in the proper context (i.e.
+--  at global or local scope).
+--
+--  { dg-options "-g -gdwarf-4 -cargs -fdebug-types-section -dA" }
+--  { dg-final { scan-assembler-times "\\(DIE \\(0x\[a-f0-9\]*\\) DW_TAG_type_unit\\)" 0 } }
+
+procedure Debug9 is
+   type Array_Type is array (Natural range <>) of Integer;
+   type Record_Type (L1, L2 : Natural) is record
+      I1 : Integer;
+      A1 : Array_Type (1 .. L1);
+      I2 : Integer;
+      A2 : Array_Type (1 .. L2);
+      I3 : Integer;
+   end record;
+
+   function Get (L1, L2 : Natural) return Record_Type is
+      Result : Record_Type (L1, L2);
+   begin
+      Result.I1 := 1;
+      for I in Result.A1'Range loop
+         Result.A1 (I) := I;
+      end loop;
+      Result.I2 := 2;
+      for I in Result.A2'Range loop
+         Result.A2 (I) := I;
+      end loop;
+      Result.I3 := 3;
+      return Result;
+   end Get;
+
+   R1 : Record_Type := Get (0, 0);
+   R2 : Record_Type := Get (1, 0);
+   R3 : Record_Type := Get (0, 1);
+   R4 : Record_Type := Get (2, 2);
+
+   procedure Process (R : Record_Type) is
+   begin
+      null;
+   end Process;
+
+begin
+   Process (R1);
+   Process (R2);
+   Process (R3);
+   Process (R4);
+end Debug9;
-- 
2.3.3.199.g52cae64

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

[Pierre-Marie de Rodat]

Hello,

On 08/29/2016 11:03 AM, Pierre-Marie de Rodat wrote:
> Here is another attempt to solve the original issue. This time, with a
> proper testcase. ;-)
>
> Rebased against trunk, boostrapped and regtested on x86_64-linux.

Ping for the patch submitted at 
<https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01934.html>. Thank you in 
advance!

-- 
Pierre-Marie de Rodat

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

[Richard Biener]

On Tue, Sep 6, 2016 at 10:43 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> Hello,
>
> On 08/29/2016 11:03 AM, Pierre-Marie de Rodat wrote:
>>
>> Here is another attempt to solve the original issue. This time, with a
>> proper testcase. ;-)
>>
>> Rebased against trunk, boostrapped and regtested on x86_64-linux.
>
>
> Ping for the patch submitted at
> <https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01934.html>. Thank you in
> advance!

Ok, had time to look at this issue again.  I see the patch works like dwarf2out
works currently with respect to DIE creation order and re-location.

-       /* Output a DIE to represent the typedef itself.  */
-       gen_typedef_die (decl, context_die);
+       {
+         /* Output a DIE to represent the typedef itself.  */
+         gen_typedef_die (decl, context_die);
+
+         /* The above may create a typedef in the proper scope, but the
+            underlying type itself could have been created earlier, at a point
+            when the scope was not available yet.  If it's the case, relocate
+            it.  This is analogous to what is done in process_scope_var,
+            except we deal with a TYPE and not a DECL, here.  */
+         dw_die_ref type_die = lookup_type_die (TREE_TYPE (decl));
+         if (type_die != NULL && type_die->die_parent == NULL
+             && DECL_CONTEXT (decl) == TYPE_CONTEXT (TREE_TYPE (decl)))
+           add_child_die (context_die, type_die);
+       }

this might be incomplete though for the case where it's say

 typedef const T X;

thus the type of decl is a qualified type?  In this case the qualification might
be at the correct scope but the actual type not or you just relocate the
qualification but not the type DIE it refers to?

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.

Richard.

> --
> Pierre-Marie de Rodat

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

[Pierre-Marie de Rodat]

On 09/07/2016 11:30 AM, Richard Biener wrote:
> Ok, had time to look at this issue again.  I see the patch works like dwarf2out
> works currently with respect to DIE creation order and re-location.

Thank you very much for helping me with this again!

So yes, that was the intent of the patch.

> this might be incomplete though for the case where it's say
>
>  typedef const T X;
>
> thus the type of decl is a qualified type?  In this case the qualification might
> be at the correct scope but the actual type not or you just relocate the
> qualification but not the type DIE it refers to?

I haven’t tested this yet but I guess you are right. A complete patch 
should also probably see if the unqualifier type should be relocated.

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

Reordering compilation of function nests was the first idea I had in 
mind when I first worked on this issue, maybe two years ago. I thought 
it would make sense debug info generation-wise, but I wondered if this 
specific order was motivated by code generation concerns.

Anyway I agree it would be a more elegant way out. As the GNU cauldron 
is going to keep me busy, I think I’ll investigate this way next week. 
Thanks again! :-)

-- 
Pierre-Marie de Rodat

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

[Pierre-Marie de Rodat]

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

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!

-- 
Pierre-Marie de Rodat

[-- Attachment #2: 0001-DWARF-fix-scoping-for-descriptions-of-local-types.patch --]
[-- Type: text/x-diff, Size: 6737 bytes --]

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

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

[Richard Biener]

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

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

[Pierre-Marie de Rodat]

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

On 09/28/2016 09:48 AM, Richard Biener wrote:
> Hmm, interesting approach.  It might work reliably at this point of
> the compilation
> but we do actually recycle cgraph nodes.

Arf, what a pity. ;-)

> 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
>[…]
> 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.

I think it makes sense, and it seems to work, so many thanks for the 
suggestion! Bootstrapped and regtested successfuly on x86_64-linux. And 
this time, I did not forgot to attach the new testcase!

-- 
Pierre-Marie de Rodat

[-- Attachment #2: 0001-DWARF-fix-scoping-for-descriptions-of-local-types.patch --]
[-- Type: text/x-diff, Size: 3687 bytes --]

From ad9eeef49da48786caabe9ef529127d6f93ea766 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.

This patch makes sure that when we generate early debug info for a
nested function, we trigger generation for the parent function first.

gcc/ChangeLog:

	* dwarf2out.c (dwarf2out_early_global_decl): For nested
	functions, call dwarf2out_decl on the parent function first.

gcc/testsuite/

	* gcc/testsuite/gnat.dg/debug9.adb: New testcase.
---
 gcc/dwarf2out.c                  | 10 ++++++++
 gcc/testsuite/gnat.dg/debug9.adb | 53 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)
 create mode 100644 gcc/testsuite/gnat.dg/debug9.adb

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 4a3df33..413106d 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -23867,6 +23867,16 @@ dwarf2out_early_global_decl (tree decl)
 	  if (!DECL_STRUCT_FUNCTION (decl))
 	    goto early_decl_exit;
 
+	  /* For nested functions, emit DIEs for the parents first so that all
+	     nested DIEs are generated at the proper scope in the first
+	     shot.  */
+	  tree context = decl_function_context (decl);
+	  if (context != NULL)
+	    {
+	      current_function_decl = context;
+	      dwarf2out_decl (context);
+	    }
+
 	  current_function_decl = decl;
 	}
       dwarf2out_decl (decl);
diff --git a/gcc/testsuite/gnat.dg/debug9.adb b/gcc/testsuite/gnat.dg/debug9.adb
new file mode 100644
index 0000000..dd14210
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug9.adb
@@ -0,0 +1,53 @@
+--  The aim of this test is to check that Ada types appear in the proper
+--  context in the debug info.
+-- 
+--  Checking this directly would be really tedious just scanning for assembly
+--  lines, so instead we rely on DWARFv4's .debug_types sections, which must be
+--  created only for global-scope types. Checking the number of .debug_types is
+--  some hackish way to check that types are output in the proper context (i.e.
+--  at global or local scope).
+--
+--  { dg-options "-g -gdwarf-4 -cargs -fdebug-types-section -dA" }
+--  { dg-final { scan-assembler-times "\\(DIE \\(0x\[a-f0-9\]*\\) DW_TAG_type_unit\\)" 0 } }
+
+procedure Debug9 is
+   type Array_Type is array (Natural range <>) of Integer;
+   type Record_Type (L1, L2 : Natural) is record
+      I1 : Integer;
+      A1 : Array_Type (1 .. L1);
+      I2 : Integer;
+      A2 : Array_Type (1 .. L2);
+      I3 : Integer;
+   end record;
+
+   function Get (L1, L2 : Natural) return Record_Type is
+      Result : Record_Type (L1, L2);
+   begin
+      Result.I1 := 1;
+      for I in Result.A1'Range loop
+         Result.A1 (I) := I;
+      end loop;
+      Result.I2 := 2;
+      for I in Result.A2'Range loop
+         Result.A2 (I) := I;
+      end loop;
+      Result.I3 := 3;
+      return Result;
+   end Get;
+
+   R1 : Record_Type := Get (0, 0);
+   R2 : Record_Type := Get (1, 0);
+   R3 : Record_Type := Get (0, 1);
+   R4 : Record_Type := Get (2, 2);
+
+   procedure Process (R : Record_Type) is
+   begin
+      null;
+   end Process;
+
+begin
+   Process (R1);
+   Process (R2);
+   Process (R3);
+   Process (R4);
+end Debug9;
-- 
2.10.0

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

[Richard Biener]

On Tue, Oct 11, 2016 at 4:39 PM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> On 09/28/2016 09:48 AM, Richard Biener wrote:
>>
>> Hmm, interesting approach.  It might work reliably at this point of
>> the compilation
>> but we do actually recycle cgraph nodes.
>
>
> Arf, what a pity. ;-)
>
>> 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
>> […]
>> 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.
>
>
> I think it makes sense, and it seems to work, so many thanks for the
> suggestion! Bootstrapped and regtested successfuly on x86_64-linux. And this
> time, I did not forgot to attach the new testcase!

Ok.

Thanks,
Richard.

> --
> Pierre-Marie de Rodat

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

[Pierre-Marie de Rodat]

On 10/12/2016 10:15 AM, Richard Biener wrote:
> Ok.
>
> Thanks,

Committed. Thank you!

-- 
Pierre-Marie de Rodat