On 07/28/2017 01:21 PM, Richard Biener wrote:
> On Fri, Jul 28, 2017 at 12:52 PM, Martin Liška <mli...@suse.cz> wrote:
>> On 07/28/2017 09:58 AM, Richard Biener wrote:
>>> On Fri, Jul 28, 2017 at 9:50 AM, Martin Liška <mli...@suse.cz> wrote:
>>>> On 07/28/2017 09:21 AM, Richard Biener wrote:
>>>>> On Thu, Jul 27, 2017 at 4:24 PM, Martin Liška <mli...@suse.cz> wrote:
>>>>>> Hi.
>>>>>>
>>>>>> Following simple patch adds support for dumping of BBs when it's a BB
>>>>>> that contains a label. That makes it easier for debugging as one can
>>>>>> find destination for an edge in dump file.
>>>>>>
>>>>>> Sample, before:
>>>>>>
>>>>>> foo (int a)
>>>>>> {
>>>>>> int D.1821;
>>>>>> int _1;
>>>>>> int _4;
>>>>>> int _5;
>>>>>>
>>>>>> <bb 2> [0.00%] [count: INV]:
>>>>>> switch (a_2(D)) <default: <L2> [INV] [count: INV], case 0: <L0> [INV]
>>>>>> [count: INV], case 1: <L1> [INV] [count: INV]>
>>>>>>
>>>>>> <L0> [0.00%] [count: INV]:
>>>>>> a_3 = a_2(D) + 2;
>>>>>>
>>>>>> <L1> [0.00%] [count: INV]:
>>>>>> _4 = 2;
>>>>>> goto <bb 6> (<L3>); [INV] [count: INV]
>>>>>>
>>>>>> <L2> [0.00%] [count: INV]:
>>>>>> _5 = 123;
>>>>>>
>>>>>> # _1 = PHI <_4(4), _5(5)>
>>>>>> <L3> [0.00%] [count: INV]:
>>>>>> return _1;
>>>>>>
>>>>>> }
>>>>>>
>>>>>> After:
>>>>>>
>>>>>> foo (int a)
>>>>>> {
>>>>>> int D.1821;
>>>>>> int _1;
>>>>>> int _4;
>>>>>> int _5;
>>>>>>
>>>>>> <bb 2> [0.00%] [count: INV]:
>>>>>> switch (a_2(D)) <default: <L2> [INV] [count: INV], case 0: <L0> [INV]
>>>>>> [count: INV], case 1: <L1> [INV] [count: INV]>
>>>>>>
>>>>>> <L0> (<bb 3>) [0.00%] [count: INV]:
>>>>>> a_3 = a_2(D) + 2;
>>>>>>
>>>>>> <L1> (<bb 4>) [0.00%] [count: INV]:
>>>>>> _4 = 2;
>>>>>> goto <bb 6> (<L3>); [INV] [count: INV]
>>>>>>
>>>>>> <L2> (<bb 5>) [0.00%] [count: INV]:
>>>>>> _5 = 123;
>>>>>>
>>>>>> # _1 = PHI <_4(4), _5(5)>
>>>>>> <L3> (<bb 6>) [0.00%] [count: INV]:
>>>>>> return _1;
>>>>>>
>>>>>> }
>>>>>>
>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression
>>>>>> tests.
>>>>>>
>>>>>> Thoughts?
>>>>>
>>>>> I think I prefer to always see
>>>>>
>>>>> <bb 3> ....:
>>>>>
>>>>> and if there's a label just dump that as well, thus
>>>>>
>>>>> <bb 3> ....:
>>>>> L0:
>>>>>
>>>>> I think that's how we dump the case with multiple labels. And always use
>>>>> the
>>>>> implicit bb N when dumping destinations (in gotos, switches, etc).
>>>>>
>>>>> That is, what we have now is IMHO premature prettifying losing BB
>>>>> indices in the dumps
>>>>> unnecessarily.
>>>>>
>>>>> Richard.
>>>>
>>>> Hi.
>>>>
>>>> I like your ideas, there's difference in between 7.1 and modified trunk:
>>>>
>>>> foo (int a)
>>>> {
>>>> int D.1824;
>>>> int _1;
>>>> int _4;
>>>> int _6;
>>>>
>>>> <bb 2> [0.00%] [count: INV]:
>>>> switch (a_2(D)) <default: <L2> [INV] [count: INV], case 0: <L0> [INV]
>>>> [count: INV], case 1: <L1> [INV] [count: INV]>
>>>>
>>>> <L0> [0.00%] [count: INV]:
>>>> a_3 = a_2(D) + 2;
>>>>
>>>> <L1> [0.00%] [count: INV]:
>>>> _4 = 2;
>>>> goto <bb 8> (<L6>); [INV] [count: INV]
>>>>
>>>> <L2> [0.00%] [count: INV]:
>>>>
>>>> <bb 6> [0.00%] [count: INV]:
>>>> a_5 = a_2(D) + 2;
>>>>
>>>> label_XXX [0.00%] [count: INV]:
>>>> label_YYY [0.00%] [count: INV]:
>>>> _6 = 101;
>>>>
>>>> # _1 = PHI <_4(4), _6(7)>
>>>> <L6> [0.00%] [count: INV]:
>>>> return _1;
>>>>
>>>> }
>>>>
>>>> after:
>>>>
>>>> foo (int a)
>>>> {
>>>> int D.1824;
>>>> int _1;
>>>> int _4;
>>>> int _6;
>>>>
>>>> <bb 2> [0.00%] [count: INV]:
>>>> switch (a_2(D)) <default: <bb 5> [INV] [count: INV], case 0: <bb 3>
>>>> [INV] [count: INV], case 1: <bb 4> [INV] [count: INV]>
>>>>
>>>> <bb 3> [0.00%] [count: INV]:
>>>> <L0>:
>>>> a_3 = a_2(D) + 2;
>>>>
>>>> <bb 4> [0.00%] [count: INV]:
>>>> <L1>:
>>>> _4 = 2;
>>>> goto <bb 8>; [INV] [count: INV]
>>>>
>>>> <bb 5> [0.00%] [count: INV]:
>>>> <L2>:
>>>>
>>>> <bb 6> [0.00%] [count: INV]:
>>>> a_5 = a_2(D) + 2;
>>>>
>>>> <bb 7> [0.00%] [count: INV]:
>>>> label_XXX:
>>>> label_YYY:
>>>> _6 = 101;
>>>>
>>>> <bb 8> [0.00%] [count: INV]:
>>>> # _1 = PHI <_4(4), _6(7)>
>>>> <L6>:
>>>> return _1;
>>>>
>>>> }
>>>>
>>>> Do you like it? What about indentation of labels, should I increase it or
>>>> leave it?
>>>
>>> Leave it.
>>>
>>>> I guess there will be some tests that will need to be adjusted.
>>>
>>> I guess so.
>>>
>>> I think <L0>: and friends are all DECL_ARTIFICIAL -- maybe we can avoid
>>> dumping
>>> them? Hmm, I guess doing it like above, while it preserves BB numbering,
>>> does
>>> reflect the actual IL a bit less so I guess I'd leave the <L0>s in
>>> switches (those
>>> have labels) and gotos if there's still the label args (not in case of
>>> we are just
>>> dumping CFG edges).
>>
>> Good, thus said there's how it will look like:
>>
>> $ cat /tmp/switch.c
>> int c;
>>
>> int foo(int a)
>> {
>> switch (a)
>> {
>> case 0:
>> a += 2;
>> case 1:
>> if (c)
>> goto label_XXX;
>> return 2;
>> default:
>> break;
>> }
>>
>> a += 2;
>>
>> label_XXX:
>> label_YYY:
>> return 99 + 2;
>> }
>>
>> $ ./xgcc -B. /tmp/switch.c -fdump-tree-optimized=/dev/stdout
>>
>> ;; Function foo (foo, funcdef_no=0, decl_uid=1816, cgraph_uid=0,
>> symbol_order=1)
>>
>> foo (int a)
>> {
>> int D.1827;
>> int c.0_1;
>> int _2;
>> int _6;
>> int _8;
>>
>> <bb 2> [0.00%] [count: INV]:
>> switch (a_3(D)) <default: <L4> [INV] [count: INV], case 0: <L0> [INV]
>> [count: INV], case 1: <L1> [INV] [count: INV]>
>>
>> <bb 3> [0.00%] [count: INV]:
>> <L0>:
>> a_4 = a_3(D) + 2;
>>
>> <bb 4> [0.00%] [count: INV]:
>> <L1>:
>> c.0_1 = c;
>> if (c.0_1 != 0)
>> goto <bb 5>; [INV] [count: INV]
>> else
>> goto <bb 6>; [INV] [count: INV]
>>
>> <bb 5> [0.00%] [count: INV]:
>> goto <bb 9>; [INV] [count: INV]
>>
>> <bb 6> [0.00%] [count: INV]:
>> _6 = 2;
>> goto <bb 10>; [INV] [count: INV]
>>
>> <bb 7> [0.00%] [count: INV]:
>> <L4>:
>>
>> <bb 8> [0.00%] [count: INV]:
>> a_7 = a_3(D) + 2;
>>
>> <bb 9> [0.00%] [count: INV]:
>> label_XXX:
>> label_YYY:
>> _8 = 101;
>>
>> <bb 10> [0.00%] [count: INV]:
>> # _2 = PHI <_6(6), _8(9)>
>> <L8>:
>> return _2;
>>
>> }
>>
>>
>> Note that edge bb_5->bb_9 is represented after gimplification by implicit
>> edge, not by goto. But:
>>
>> ./xgcc -B. /tmp/switch.c -fdump-tree-lower=/dev/stdout
>>
>> ;; Function foo (foo, funcdef_no=0, decl_uid=1816, cgraph_uid=0,
>> symbol_order=1)
>>
>> foo (int a)
>> {
>> int D.1827;
>>
>> switch (a) <default: <D.1821>, case 0: <D.1818>, case 1: <D.1819>>
>> <D.1818>:
>> a = a + 2;
>> <D.1819>:
>> c.0_1 = c;
>> if (c.0_1 != 0) goto <D.1825>; else goto <D.1826>;
>> <D.1825>:
>> goto label_XXX;
>> <D.1826>:
>> D.1827 = 2;
>> goto <D.1828>;
>> <D.1821>:
>> goto <D.1822>;
>> <D.1822>:
>> a = a + 2;
>> label_XXX:
>> label_YYY:
>> D.1827 = 101;
>> goto <D.1828>;
>> <D.1828>:
>> return D.1827;
>> }
>>
>> There labels are dumped properly. If it's ok I'll start working on
>> test-suite transition.
>
> Yes. Looks good to me now.
>
> That said... if the fallout is very big we might consider switching to
> -gimple style dumping
> unconditionally?
>
> Richard.
Hello.
Sending second version of the patch. Eventually it shows that fallout for test
suite was minimal.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Ready to be installed?
Martin
>
>> Martin
>>
>>>
>>> Richard.
>>>
>>>> Martin
>>>>
>>>>>
>>>>>> Martin
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>> 2017-07-27 Martin Liska <mli...@suse.cz>
>>>>>>
>>>>>> * gcc.dg/builtin-unreachable-6.c: Update scanned pattern.
>>>>>> * gcc.dg/tree-ssa/attr-hotcold-2.c: Likewise.
>>>>>> * gcc.dg/tree-ssa/ssa-ccp-18.c: Likewise.
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2017-07-27 Martin Liska <mli...@suse.cz>
>>>>>>
>>>>>> * gimple-pretty-print.c (dump_gimple_label): Dump BB number.
>>>>>> ---
>>>>>> gcc/gimple-pretty-print.c | 6 +++++-
>>>>>> gcc/testsuite/gcc.dg/builtin-unreachable-6.c | 2 +-
>>>>>> gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c | 4 ++--
>>>>>> gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-18.c | 3 +--
>>>>>> 4 files changed, 9 insertions(+), 6 deletions(-)
>>>>>>
>>>>>>
>>>>
>>
>From 09225795a538acd70e72fcb755ece11631660f35 Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Fri, 28 Jul 2017 12:53:38 +0200
Subject: [PATCH] Learn GIMPLE pretty printer to produce nicer dump output.
gcc/ChangeLog:
2017-07-28 Martin Liska <mli...@suse.cz>
* gimple-pretty-print.c (dump_gimple_label): Never dump
BB info.
(dump_gimple_bb_header): Always dump BB info.
(pp_cfg_jump): Do not append info about BB when dumping a jump.
gcc/testsuite/ChangeLog:
2017-07-28 Martin Liska <mli...@suse.cz>
* gcc.dg/builtin-unreachable-6.c: Update scanned patterns.
* gcc.dg/tree-ssa/attr-hotcold-2.c: Likewise.
---
gcc/gimple-pretty-print.c | 33 ++++++--------------------
gcc/testsuite/gcc.dg/builtin-unreachable-6.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c | 4 ++--
3 files changed, 10 insertions(+), 29 deletions(-)
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index c8eb9c4a7bf..8b69b72e9e2 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -1120,9 +1120,6 @@ dump_gimple_label (pretty_printer *buffer, glabel *gs, int spc,
else
{
dump_generic_node (buffer, label, spc, flags, false);
- basic_block bb = gimple_bb (gs);
- if (bb && !(flags & TDF_GIMPLE))
- pp_scalar (buffer, " %s", dump_profile (bb->frequency, bb->count));
pp_colon (buffer);
}
if (flags & TDF_GIMPLE)
@@ -2695,16 +2692,12 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent,
}
else
{
- gimple *stmt = first_stmt (bb);
- if (!stmt || gimple_code (stmt) != GIMPLE_LABEL)
- {
- if (flags & TDF_GIMPLE)
- fprintf (outf, "%*sbb_%d:\n", indent, "", bb->index);
- else
- fprintf (outf, "%*s<bb %d> %s:\n",
- indent, "", bb->index, dump_profile (bb->frequency,
- bb->count));
- }
+ if (flags & TDF_GIMPLE)
+ fprintf (outf, "%*sbb_%d:\n", indent, "", bb->index);
+ else
+ fprintf (outf, "%*s<bb %d> %s:\n",
+ indent, "", bb->index, dump_profile (bb->frequency,
+ bb->count));
}
}
@@ -2760,22 +2753,10 @@ pp_cfg_jump (pretty_printer *buffer, edge e, dump_flags_t flags)
}
else
{
- gimple *stmt = first_stmt (e->dest);
-
pp_string (buffer, "goto <bb ");
pp_decimal_int (buffer, e->dest->index);
pp_greater (buffer);
- if (stmt && gimple_code (stmt) == GIMPLE_LABEL)
- {
- pp_string (buffer, " (");
- dump_generic_node (buffer,
- gimple_label_label (as_a <glabel *> (stmt)),
- 0, 0, false);
- pp_right_paren (buffer);
- pp_semicolon (buffer);
- }
- else
- pp_semicolon (buffer);
+ pp_semicolon (buffer);
dump_edge_probability (buffer, e);
}
diff --git a/gcc/testsuite/gcc.dg/builtin-unreachable-6.c b/gcc/testsuite/gcc.dg/builtin-unreachable-6.c
index d2596e95c3f..2f8ca369546 100644
--- a/gcc/testsuite/gcc.dg/builtin-unreachable-6.c
+++ b/gcc/testsuite/gcc.dg/builtin-unreachable-6.c
@@ -16,5 +16,5 @@ lab2:
goto *x;
}
-/* { dg-final { scan-tree-dump-times "lab \\\[\[0-9.\]+%\\\]" 1 "fab1" } } */
+/* { dg-final { scan-tree-dump-times "lab:" 1 "fab1" } } */
/* { dg-final { scan-tree-dump-times "__builtin_unreachable" 1 "fab1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c b/gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c
index 184dd10ddae..5f7e3afa2ae 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c
@@ -20,9 +20,9 @@ void f(int x, int y)
/* { dg-final { scan-tree-dump-times "hot label heuristics" 1 "profile_estimate" } } */
/* { dg-final { scan-tree-dump-times "cold label heuristics" 1 "profile_estimate" } } */
-/* { dg-final { scan-tree-dump "A \\\[0\\\..*\\\]" "profile_estimate" } } */
+/* { dg-final { scan-tree-dump-times "combined heuristics: 0\\\..*" 1 "profile_estimate" } } */
/* Note: we're attempting to match some number > 6000, i.e. > 60%.
The exact number ought to be tweekable without having to juggle
the testcase around too much. */
-/* { dg-final { scan-tree-dump "B \\\[\[6-9\]\[0-9\]\\\..*\\\]" "profile_estimate" } } */
+/* { dg-final { scan-tree-dump-times "combined heuristics: \[6-9\]\[0-9\]\\\..*" 1 "profile_estimate" } } */
--
2.13.3
