Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'

Message ID 87fsosmb17.fsf@dem-tschwing-1.ger.mentorg.com
State New
Headers
Series Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node' |

Commit Message

Thomas Schwinge Feb. 9, 2022, 1:20 p.m. UTC
  Hi!

OK to push (now, or in next development stage 1?) the attached
"Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'",
or should that be done differently -- or, per the current state (why?)
not at all?

This does work for my current debugging task, but I've not yet run
'make check' in case anything needs to be adjusted there.


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  

Comments

Richard Biener Feb. 10, 2022, 9:42 a.m. UTC | #1
On Wed, Feb 9, 2022 at 2:21 PM Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> Hi!
>
> OK to push (now, or in next development stage 1?) the attached
> "Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'",
> or should that be done differently -- or, per the current state (why?)
> not at all?
>
> This does work for my current debugging task, but I've not yet run
> 'make check' in case anything needs to be adjusted there.

Hmm, I wonder if we shouldn't simply dump DECL_UID as

 'uid NNN'

somewhere.  For example after or before DECL_NAME?

>
> Grüße
>  Thomas
>
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  
Michael Matz Feb. 10, 2022, 4:36 p.m. UTC | #2
Hi,

On Thu, 10 Feb 2022, Richard Biener via Gcc-patches wrote:

> On Wed, Feb 9, 2022 at 2:21 PM Thomas Schwinge <thomas@codesourcery.com> wrote:
> >
> > Hi!
> >
> > OK to push (now, or in next development stage 1?) the attached
> > "Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'",
> > or should that be done differently -- or, per the current state (why?)
> > not at all?
> >
> > This does work for my current debugging task, but I've not yet run
> > 'make check' in case anything needs to be adjusted there.
> 
> Hmm, I wonder if we shouldn't simply dump DECL_UID as
> 
>  'uid NNN'

Yes, much better in line with the normal dump_tree output.


Ciao,
Michael.

> 
> somewhere.  For example after or before DECL_NAME?
> 
> >
> > Grüße
> >  Thomas
> >
> >
> > -----------------
> > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
>
  
Thomas Schwinge Feb. 10, 2022, 10:19 p.m. UTC | #3
Hi!

On 2022-02-10T16:36:51+0000, Michael Matz via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> On Thu, 10 Feb 2022, Richard Biener via Gcc-patches wrote:
>> On Wed, Feb 9, 2022 at 2:21 PM Thomas Schwinge <thomas@codesourcery.com> wrote:
>> > OK to push (now, or in next development stage 1?) the attached
>> > "Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'",
>> > or should that be done differently -- or, per the current state (why?)
>> > not at all?

First, thanks for (indirectly) having confirmed that my confusion is not
completely off, why this is currently missing.  ;-)

>> Hmm, I wonder if we shouldn't simply dump DECL_UID as
>>
>>  'uid NNN'
>
> Yes, much better in line with the normal dump_tree output.

>> somewhere.  For example after or before DECL_NAME?

Heh -- that's what I wanted to do initially, but then I saw that we've
currently got in 'print_node_brief' (and very similar in 'print_node'):

    [...]
      fprintf (file, "%s <%s", prefix, get_tree_code_name (TREE_CODE (node)));
      dump_addr (file, " ", node);

      if (tclass == tcc_declaration)
        {
          if (DECL_NAME (node))
            fprintf (file, " %s", IDENTIFIER_POINTER (DECL_NAME (node)));
          else if (TREE_CODE (node) == LABEL_DECL
                   && LABEL_DECL_UID (node) != -1)
            {
              if (dump_flags & TDF_NOUID)
                fprintf (file, " L.xxxx");
              else
                fprintf (file, " L.%d", (int) LABEL_DECL_UID (node));
            }
          else
            {
              if (dump_flags & TDF_NOUID)
                fprintf (file, " %c.xxxx",
                         TREE_CODE (node) == CONST_DECL ? 'C' : 'D');
              else
                fprintf (file, " %c.%u",
                         TREE_CODE (node) == CONST_DECL ? 'C' : 'D',
                         DECL_UID (node));
            }
        }
    [...]

That is, if there's no 'DECL_NAME', we print 'L.[UID]', 'C.[UID]',
'D.[UID]'.  The same we do in 'gcc/tree-pretty-print.cc:dump_decl_name',
I found.  But in the latter function, we also do it that same way if
there is a 'DECL_NAME' ('i' -> 'iD.4249', for example), so that's why I
copied that style back to my proposed 'print_node_brief'/'print_node'
change.

Are you now suggesting to only print 'DECL_NAME' as '[NAME] uid [UID]',
but keep 'L.[UID]', 'C.[UID]', 'D.[UID]' in the "dot" form, or change
these to 'L uid [UID]', 'C uid [UID]', 'D uid [UID]' correspondingly?
And also do the similar changes in
'gcc/tree-pretty-print.cc:dump_decl_name' (as well as another dozen or so
places where such things are printed...), or don't change those?

I don't care very much which way, just have some slight preference to
keep things similar.


Grüße
 Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  
Richard Biener Feb. 11, 2022, 7:02 a.m. UTC | #4
On Thu, Feb 10, 2022 at 11:20 PM Thomas Schwinge
<thomas@codesourcery.com> wrote:
>
> Hi!
>
> On 2022-02-10T16:36:51+0000, Michael Matz via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> > On Thu, 10 Feb 2022, Richard Biener via Gcc-patches wrote:
> >> On Wed, Feb 9, 2022 at 2:21 PM Thomas Schwinge <thomas@codesourcery.com> wrote:
> >> > OK to push (now, or in next development stage 1?) the attached
> >> > "Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'",
> >> > or should that be done differently -- or, per the current state (why?)
> >> > not at all?
>
> First, thanks for (indirectly) having confirmed that my confusion is not
> completely off, why this is currently missing.  ;-)
>
> >> Hmm, I wonder if we shouldn't simply dump DECL_UID as
> >>
> >>  'uid NNN'
> >
> > Yes, much better in line with the normal dump_tree output.
>
> >> somewhere.  For example after or before DECL_NAME?
>
> Heh -- that's what I wanted to do initially, but then I saw that we've
> currently got in 'print_node_brief' (and very similar in 'print_node'):
>
>     [...]
>       fprintf (file, "%s <%s", prefix, get_tree_code_name (TREE_CODE (node)));
>       dump_addr (file, " ", node);
>
>       if (tclass == tcc_declaration)
>         {
>           if (DECL_NAME (node))
>             fprintf (file, " %s", IDENTIFIER_POINTER (DECL_NAME (node)));
>           else if (TREE_CODE (node) == LABEL_DECL
>                    && LABEL_DECL_UID (node) != -1)
>             {
>               if (dump_flags & TDF_NOUID)
>                 fprintf (file, " L.xxxx");
>               else
>                 fprintf (file, " L.%d", (int) LABEL_DECL_UID (node));
>             }
>           else
>             {
>               if (dump_flags & TDF_NOUID)
>                 fprintf (file, " %c.xxxx",
>                          TREE_CODE (node) == CONST_DECL ? 'C' : 'D');
>               else
>                 fprintf (file, " %c.%u",
>                          TREE_CODE (node) == CONST_DECL ? 'C' : 'D',
>                          DECL_UID (node));
>             }
>         }
>     [...]
>
> That is, if there's no 'DECL_NAME', we print 'L.[UID]', 'C.[UID]',
> 'D.[UID]'.  The same we do in 'gcc/tree-pretty-print.cc:dump_decl_name',
> I found.  But in the latter function, we also do it that same way if
> there is a 'DECL_NAME' ('i' -> 'iD.4249', for example), so that's why I
> copied that style back to my proposed 'print_node_brief'/'print_node'
> change.
>
> Are you now suggesting to only print 'DECL_NAME' as '[NAME] uid [UID]',
> but keep 'L.[UID]', 'C.[UID]', 'D.[UID]' in the "dot" form, or change
> these to 'L uid [UID]', 'C uid [UID]', 'D uid [UID]' correspondingly?

I'd say these should then be 'D.[UID] uid [UID]' even if that's
somewhat redundant.

> And also do the similar changes in
> 'gcc/tree-pretty-print.cc:dump_decl_name' (as well as another dozen or so
> places where such things are printed...), or don't change those?

Don't change those - you were targeting the tree dumper, not the
pretty printers.
The tree dumpers generally dump attributes separately.


>
> I don't care very much which way, just have some slight preference to
> keep things similar.
>
>
> Grüße
>  Thomas
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  

Patch

From e655409cf9154ac72194dd55f7f80cb5ed3137fc Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 9 Feb 2022 12:51:39 +0100
Subject: [PATCH] Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief',
 'print_node'

Running GCC with '-fdump-tree-all-uid' (so that 'TDF_UID' is set in
'dump_flags') and '-wrapper gdb,--args', then for a 'call debug_tree(decl)',
that does (pretty-)print all kinds of things -- but not the 'DECL_UID':

    [...]
    (gdb) print dump_flags & TDF_UID
    $1 = 256
    (gdb) call debug_tree(decl)
     <var_decl 0x7ffff7fc3f30 i
        type <integer_type 0x7ffff75555e8 integer(kind=4) public SI
            size <integer_cst 0x7ffff753ee40 constant 32>
            unit-size <integer_cst 0x7ffff753ee58 constant 4>
            align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff75555e8 precision:32 min <integer_cst 0x7ffff753edf8 -2147483648> max <integer_cst 0x7ffff753ee10 2147483647>
            pointer_to_this <pointer_type 0x7ffff755da80>>
        used SI source-gcc/gcc/testsuite/gfortran.dg/goacc-gomp/pr102330-3.f90:10:3 size <integer_cst 0x7ffff753ee40 32> unit-size <integer_cst 0x7ffff753ee58 4>
        align:32 warn_if_not_align:0 context <function_decl 0x7ffff7730d00 p>>
    (gdb) print decl.decl_minimal.uid
    $3 = 4249

In my opinion, that's a bit unfortunate, as the 'DECL_UID' is very important
for debugging certain classes of issues.

With this patch, there is no change if 'TDF_UID' isn't set, but if it is, we
now use the same syntax as 'gcc/tree-pretty-print.cc:dump_decl_name', for
example:

     <var_decl 0x7ffff7fc3f30 iD.4249
        type <integer_type 0x7ffff75555e8 integer(kind=4) public SI
            size <integer_cst 0x7ffff753ee40 constant 32>
            unit-size <integer_cst 0x7ffff753ee58 constant 4>
            align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff75555e8 precision:32 min <integer_cst 0x7ffff753edf8 -2147483648> max <integer_cst 0x7ffff753ee10 2147483647>
            pointer_to_this <pointer_type 0x7ffff755da80>>
        used SI source-gcc/gcc/testsuite/gfortran.dg/goacc-gomp/pr102330-3.f90:10:3 size <integer_cst 0x7ffff753ee40 32> unit-size <integer_cst 0x7ffff753ee58 4>
        align:32 warn_if_not_align:0 context <function_decl 0x7ffff7730d00 pD.4227>>

Notice 'var_decl': 'i' vs. 'iD.4249', and 'function_decl': 'p' vs. 'pD.4227'.
Or 'iD.xxxx', 'pD.xxxx' if 'TDF_NOUID' is set ('-fdump-tree-all-uid-nouid', for
example).

	gcc/
	* print-tree.cc (print_node_brief, print_node): Consider
	'TDF_UID', 'TDF_NOUID'.
---
 gcc/print-tree.cc | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
index 0876da873a9..f2da5187293 100644
--- a/gcc/print-tree.cc
+++ b/gcc/print-tree.cc
@@ -139,7 +139,17 @@  print_node_brief (FILE *file, const char *prefix, const_tree node, int indent)
   if (tclass == tcc_declaration)
     {
       if (DECL_NAME (node))
-	fprintf (file, " %s", IDENTIFIER_POINTER (DECL_NAME (node)));
+	{
+	  fprintf (file, " %s", IDENTIFIER_POINTER (DECL_NAME (node)));
+	  if (dump_flags & TDF_UID)
+	    {
+	      char c = TREE_CODE (node) == CONST_DECL ? 'C' : 'D';
+	      if (dump_flags & TDF_NOUID)
+		fprintf (file, "%c.xxxx", c);
+	      else
+		fprintf (file, "%c.%d", c, DECL_UID (node));
+	    }
+	}
       else if (TREE_CODE (node) == LABEL_DECL
 	       && LABEL_DECL_UID (node) != -1)
 	{
@@ -284,7 +294,17 @@  print_node (FILE *file, const char *prefix, tree node, int indent,
   if (tclass == tcc_declaration)
     {
       if (DECL_NAME (node))
-	fprintf (file, " %s", IDENTIFIER_POINTER (DECL_NAME (node)));
+	{
+	  fprintf (file, " %s", IDENTIFIER_POINTER (DECL_NAME (node)));
+	  if (dump_flags & TDF_UID)
+	    {
+	      char c = TREE_CODE (node) == CONST_DECL ? 'C' : 'D';
+	      if (dump_flags & TDF_NOUID)
+		fprintf (file, "%c.xxxx", c);
+	      else
+		fprintf (file, "%c.%d", c, DECL_UID (node));
+	    }
+	}
       else if (code == LABEL_DECL
 	       && LABEL_DECL_UID (node) != -1)
 	{
-- 
2.25.1