public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'
@ 2022-02-09 13:20 Thomas Schwinge
  2022-02-10  9:42 ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Schwinge @ 2022-02-09 13:20 UTC (permalink / raw)
  To: gcc-patches

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

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Consider-TDF_UID-TDF_NOUID-in-print_node_brief-print.patch --]
[-- Type: text/x-diff, Size: 4090 bytes --]

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


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

* Re: Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'
  2022-02-09 13:20 Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node' Thomas Schwinge
@ 2022-02-10  9:42 ` Richard Biener
  2022-02-10 16:36   ` Michael Matz
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2022-02-10  9:42 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: GCC Patches

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

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

* Re: Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'
  2022-02-10  9:42 ` Richard Biener
@ 2022-02-10 16:36   ` Michael Matz
  2022-02-10 22:19     ` Thomas Schwinge
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Matz @ 2022-02-10 16:36 UTC (permalink / raw)
  To: Richard Biener; +Cc: Thomas Schwinge, GCC Patches

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
> 

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

* Re: Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'
  2022-02-10 16:36   ` Michael Matz
@ 2022-02-10 22:19     ` Thomas Schwinge
  2022-02-11  7:02       ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Schwinge @ 2022-02-10 22:19 UTC (permalink / raw)
  To: Michael Matz, Richard Biener; +Cc: GCC Patches

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

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

* Re: Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'
  2022-02-10 22:19     ` Thomas Schwinge
@ 2022-02-11  7:02       ` Richard Biener
  2022-02-17 12:23         ` Thomas Schwinge
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2022-02-11  7:02 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: Michael Matz, GCC Patches

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

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

* Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'
  2022-02-11  7:02       ` Richard Biener
@ 2022-02-17 12:23         ` Thomas Schwinge
  2022-02-17 15:21           ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Schwinge @ 2022-02-17 12:23 UTC (permalink / raw)
  To: Richard Biener, gcc-patches; +Cc: Michael Matz

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

Hi!

On 2022-02-11T08:02:20+0100, Richard Biener <richard.guenther@gmail.com> wrote:
> On Thu, Feb 10, 2022 at 11:20 PM Thomas Schwinge
> <thomas@codesourcery.com> wrote:
>> 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.

Sure, that's fine for me.  So, like in the attached
"Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'";
OK to push?  (...  which evidently I forgot to send last week...)


Grüße
 Thomas


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Consider-TDF_UID-TDF_NOUID-in-print_node_brief-print.patch --]
[-- Type: text/x-diff, Size: 4548 bytes --]

From 0a39ef2415e5b43755556e5554533b33ff86f16d Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Fri, 11 Feb 2022 10:10:25 +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 append 'uid [DECL_UID]':

     <var_decl 0x7ffff7fc3f30 i uid 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:6:3 size <integer_cst 0x7ffff753ee40 32> unit-size <integer_cst 0x7ffff753ee58 4>
        align:32 warn_if_not_align:0 context <function_decl 0x7ffff7730d00 p uid 4227>>

Notice 'var_decl': 'i uid 4249', and 'function_decl': 'p uid 4227'.

A few 'print_node_brief' examples:

'!TDF_UID', '!TDF_NOUID' (no change):

    <var_decl 0x7ffff76bc120 iftmp.1>

    <function_decl 0x7ffff7692400 main>

    <label_decl 0x7ffff76b9000 labl>
    <label_decl 0x7ffff76b7d00 L.0>
    <label_decl 0x7ffff76b9100 D.2138>

'!TDF_UID', 'TDF_NOUID' (no change):

    <var_decl 0x7ffff76bc120 iftmp.1>

    <function_decl 0x7ffff7692400 main>

    <label_decl 0x7ffff76b9000 labl>
    <label_decl 0x7ffff76b7d00 L.xxxx>
    <label_decl 0x7ffff76b9100 D.xxxx>

'TDF_UID', '!TDF_NOUID' (now appends 'uid [DECL_UID]'):

    <var_decl 0x7ffff76bc120 iftmp.1 uid 2136>

    <function_decl 0x7ffff7692400 main uid 2124>

    <label_decl 0x7ffff76b9000 labl uid 2129>
    <label_decl 0x7ffff76b7d00 L.0 uid 2243
    <label_decl 0x7ffff76b9100 D.2138 uid 2138>

'TDF_UID', 'TDF_NOUID' (now appends 'uid xxxx'):

    <var_decl 0x7ffff76bc120 iftmp.1 uid xxxx>

    <function_decl 0x7ffff7692400 main uid xxxx>

    <label_decl 0x7ffff76b9000 labl uid xxxx>
    <label_decl 0x7ffff76b7d00 L.xxxx uid xxxx>
    <label_decl 0x7ffff76b9100 D.xxxx uid xxxx>

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

diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
index 0876da873a9..38dd032fbf7 100644
--- a/gcc/print-tree.cc
+++ b/gcc/print-tree.cc
@@ -158,6 +158,14 @@ print_node_brief (FILE *file, const char *prefix, const_tree node, int indent)
 		     TREE_CODE (node) == CONST_DECL ? 'C' : 'D',
 		     DECL_UID (node));
 	}
+
+      if (dump_flags & TDF_UID)
+	{
+	  if (dump_flags & TDF_NOUID)
+	    fprintf (file, " uid xxxx");
+	  else
+	    fprintf (file, " uid %d", DECL_UID (node));
+	}
     }
   else if (tclass == tcc_type)
     {
@@ -301,6 +309,14 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
 	    fprintf (file, " %c.%u", code == CONST_DECL ? 'C' : 'D',
 		     DECL_UID (node));
 	}
+
+      if (dump_flags & TDF_UID)
+	{
+	  if (dump_flags & TDF_NOUID)
+	    fprintf (file, " uid xxxx");
+	  else
+	    fprintf (file, " uid %d", DECL_UID (node));
+	}
     }
   else if (tclass == tcc_type)
     {
-- 
2.34.1


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

* Re: Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'
  2022-02-17 12:23         ` Thomas Schwinge
@ 2022-02-17 15:21           ` Richard Biener
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Biener @ 2022-02-17 15:21 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: GCC Patches, Michael Matz

On Thu, Feb 17, 2022 at 1:23 PM Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> Hi!
>
> On 2022-02-11T08:02:20+0100, Richard Biener <richard.guenther@gmail.com> wrote:
> > On Thu, Feb 10, 2022 at 11:20 PM Thomas Schwinge
> > <thomas@codesourcery.com> wrote:
> >> 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.
>
> Sure, that's fine for me.  So, like in the attached
> "Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'";
> OK to push?  (...  which evidently I forgot to send last week...)

'TDF_UID', 'TDF_NOUID' (now appends 'uid xxxx'):

    <var_decl 0x7ffff76bc120 iftmp.1 uid xxxx>

    <function_decl 0x7ffff7692400 main uid xxxx>

    <label_decl 0x7ffff76b9000 labl uid xxxx>
    <label_decl 0x7ffff76b7d00 L.xxxx uid xxxx>
    <label_decl 0x7ffff76b9100 D.xxxx uid xxxx>

just not append 'uid ...' for NOUID, it doesn't add any useful information here.

So

diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
index 0876da873a9..38dd032fbf7 100644
--- a/gcc/print-tree.cc
+++ b/gcc/print-tree.cc
@@ -158,6 +158,14 @@ print_node_brief (FILE *file, const char *prefix,
const_tree node, int indent)
                     TREE_CODE (node) == CONST_DECL ? 'C' : 'D',
                     DECL_UID (node));
        }
+
+      if (dump_flags & TDF_UID)
+       {
+         if (dump_flags & TDF_NOUID)
+           fprintf (file, " uid xxxx");
+         else
+           fprintf (file, " uid %d", DECL_UID (node));
+       }

just

    if (dump_flags & TDF_UID)
      fprintf (file, " uid %d", DECL_UID (node));

Asking for both at the same time is odd and I'd really not expect
that.  It should be a tri-state, UID (everywhere), default (in some
places), NOUID (nowhere).

+      if (dump_flags & TDF_UID)
+       {
+         if (dump_flags & TDF_NOUID)
+           fprintf (file, " uid xxxx");
+         else
+           fprintf (file, " uid %d", DECL_UID (node));
+       }

same here.  But for print_node I think we should default to printing
the UID.  So if would be

   if (!(dump_flags & TDF_NOUID))
    fprintf (file, " uid %d", DECL_UID (node));

note that UIDs can be negative, but decl_minimal.uid is unsigned but
you are using
a signed format.  See DEBUG_TEMP_UID in tree.h.  I don't have a well thought out
opinion on how to present the uid here for debug temps, signed works for me
but then I think you should have (int) DECL_UID (node) for the prints?

Thanks,
Richard.

>
> Grüße
>  Thomas
>
>
> >> 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

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

end of thread, other threads:[~2022-02-17 15:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09 13:20 Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node' Thomas Schwinge
2022-02-10  9:42 ` Richard Biener
2022-02-10 16:36   ` Michael Matz
2022-02-10 22:19     ` Thomas Schwinge
2022-02-11  7:02       ` Richard Biener
2022-02-17 12:23         ` Thomas Schwinge
2022-02-17 15:21           ` Richard Biener

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