Consider 'TDF_UID', 'TDF_NOUID' in 'print_node_brief', 'print_node'
Commit Message
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
Comments
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
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(+)
@@ -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