* [PATCH] Dump BB number when dumping a BB with label.
@ 2017-07-27 14:24 Martin Liška
2017-07-28 7:21 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2017-07-27 14:24 UTC (permalink / raw)
To: gcc-patches; +Cc: Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]
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?
Martin
gcc/testsuite/ChangeLog:
2017-07-27 Martin Liska <mliska@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 <mliska@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(-)
[-- Attachment #2: 0001-Dump-BB-number-when-dumping-a-BB-with-label.patch --]
[-- Type: text/x-patch, Size: 2719 bytes --]
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index c8eb9c4a7bf..6b272286714 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -1122,7 +1122,11 @@ dump_gimple_label (pretty_printer *buffer, glabel *gs, int spc,
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));
+ {
+ if (gimple_bb (gs))
+ pp_scalar (buffer, " (<bb %d>)", gimple_bb (gs)->index);
+ pp_scalar (buffer, " %s", dump_profile (bb->frequency, bb->count));
+ }
pp_colon (buffer);
}
if (flags & TDF_GIMPLE)
diff --git a/gcc/testsuite/gcc.dg/builtin-unreachable-6.c b/gcc/testsuite/gcc.dg/builtin-unreachable-6.c
index d2596e95c3f..040917f29b0 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 \\\(<bb .>\\\) \\\[\[0-9.\]+%\\\]" 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..67eb9163684 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 "A \\\(<bb .>\\\) \\\[0\\\..*\\\]" "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 "B \\\(<bb .>\\\) \\\[\[6-9\]\[0-9\]\\\..*\\\]" "profile_estimate" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-18.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-18.c
index 2ab12626088..78d53520395 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-18.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-18.c
@@ -15,5 +15,4 @@ void func2(int* val)
d: d(val);
}
-/* { dg-final { scan-tree-dump-not "a \\\(" "ccp1" } } */
-/* { dg-final { scan-tree-dump-not "b \\\(" "ccp1" } } */
+/* { dg-final { scan-tree-dump-not "goto" "ccp1" } } */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label.
2017-07-27 14:24 [PATCH] Dump BB number when dumping a BB with label Martin Liška
@ 2017-07-28 7:21 ` Richard Biener
2017-07-28 7:50 ` Martin Liška
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2017-07-28 7:21 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches, Jan Hubicka
On Thu, Jul 27, 2017 at 4:24 PM, Martin Liška <mliska@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.
> Martin
>
> gcc/testsuite/ChangeLog:
>
> 2017-07-27 Martin Liska <mliska@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 <mliska@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(-)
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label.
2017-07-28 7:21 ` Richard Biener
@ 2017-07-28 7:50 ` Martin Liška
2017-07-28 7:58 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2017-07-28 7:50 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Jan Hubicka
On 07/28/2017 09:21 AM, Richard Biener wrote:
> On Thu, Jul 27, 2017 at 4:24 PM, Martin Liška <mliska@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?
I guess there will be some tests that will need to be adjusted.
Martin
>
>> Martin
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2017-07-27 Martin Liska <mliska@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 <mliska@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(-)
>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label.
2017-07-28 7:50 ` Martin Liška
@ 2017-07-28 7:58 ` Richard Biener
2017-07-28 10:53 ` Martin Liška
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2017-07-28 7:58 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches, Jan Hubicka
On Fri, Jul 28, 2017 at 9:50 AM, Martin Liška <mliska@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 <mliska@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).
Richard.
> Martin
>
>>
>>> Martin
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2017-07-27 Martin Liska <mliska@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 <mliska@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(-)
>>>
>>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label.
2017-07-28 7:58 ` Richard Biener
@ 2017-07-28 10:53 ` Martin Liška
2017-07-28 11:21 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2017-07-28 10:53 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Jan Hubicka
On 07/28/2017 09:58 AM, Richard Biener wrote:
> On Fri, Jul 28, 2017 at 9:50 AM, Martin Liška <mliska@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 <mliska@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.
Martin
>
> Richard.
>
>> Martin
>>
>>>
>>>> Martin
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2017-07-27 Martin Liska <mliska@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 <mliska@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(-)
>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label.
2017-07-28 10:53 ` Martin Liška
@ 2017-07-28 11:21 ` Richard Biener
2017-07-31 6:44 ` [PATCH v2] " Martin Liška
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2017-07-28 11:21 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches, Jan Hubicka
On Fri, Jul 28, 2017 at 12:52 PM, Martin Liška <mliska@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 <mliska@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 <mliska@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.
> Martin
>
>>
>> Richard.
>>
>>> Martin
>>>
>>>>
>>>>> Martin
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 2017-07-27 Martin Liska <mliska@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 <mliska@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(-)
>>>>>
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] Dump BB number when dumping a BB with label.
2017-07-28 11:21 ` Richard Biener
@ 2017-07-31 6:44 ` Martin Liška
2017-07-31 8:50 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2017-07-31 6:44 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 8265 bytes --]
On 07/28/2017 01:21 PM, Richard Biener wrote:
> On Fri, Jul 28, 2017 at 12:52 PM, Martin Liška <mliska@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 <mliska@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 <mliska@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 <mliska@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 <mliska@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(-)
>>>>>>
>>>>>>
>>>>
>>
[-- Attachment #2: 0001-Learn-GIMPLE-pretty-printer-to-produce-nicer-dump-ou-v2.patch --]
[-- Type: text/x-patch, Size: 4276 bytes --]
From 09225795a538acd70e72fcb755ece11631660f35 Mon Sep 17 00:00:00 2001
From: marxin <mliska@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 <mliska@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 <mliska@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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Dump BB number when dumping a BB with label.
2017-07-31 6:44 ` [PATCH v2] " Martin Liška
@ 2017-07-31 8:50 ` Richard Biener
2017-07-31 8:53 ` Martin Liška
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2017-07-31 8:50 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches, Jan Hubicka
On Mon, Jul 31, 2017 at 8:43 AM, Martin Liška <mliska@suse.cz> wrote:
> On 07/28/2017 01:21 PM, Richard Biener wrote:
>> On Fri, Jul 28, 2017 at 12:52 PM, Martin Liška <mliska@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 <mliska@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 <mliska@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?
Ok. Nice that it also simplifies code.
Thanks,
Richard.
> Martin
>
>>
>>> Martin
>>>
>>>>
>>>> Richard.
>>>>
>>>>> Martin
>>>>>
>>>>>>
>>>>>>> Martin
>>>>>>>
>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>
>>>>>>> 2017-07-27 Martin Liska <mliska@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 <mliska@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(-)
>>>>>>>
>>>>>>>
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Dump BB number when dumping a BB with label.
2017-07-31 8:50 ` Richard Biener
@ 2017-07-31 8:53 ` Martin Liška
0 siblings, 0 replies; 9+ messages in thread
From: Martin Liška @ 2017-07-31 8:53 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Jan Hubicka
On 07/31/2017 10:50 AM, Richard Biener wrote:
> On Mon, Jul 31, 2017 at 8:43 AM, Martin Liška <mliska@suse.cz> wrote:
>> On 07/28/2017 01:21 PM, Richard Biener wrote:
>>> On Fri, Jul 28, 2017 at 12:52 PM, Martin Liška <mliska@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 <mliska@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 <mliska@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?
>
> Ok. Nice that it also simplifies code.
Yes. To be honest I also like code removal (simplification) :)
Martin
>
> Thanks,
> Richard.
>
>> Martin
>>
>>>
>>>> Martin
>>>>
>>>>>
>>>>> Richard.
>>>>>
>>>>>> Martin
>>>>>>
>>>>>>>
>>>>>>>> Martin
>>>>>>>>
>>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>>
>>>>>>>> 2017-07-27 Martin Liska <mliska@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 <mliska@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(-)
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-07-31 8:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-27 14:24 [PATCH] Dump BB number when dumping a BB with label Martin Liška
2017-07-28 7:21 ` Richard Biener
2017-07-28 7:50 ` Martin Liška
2017-07-28 7:58 ` Richard Biener
2017-07-28 10:53 ` Martin Liška
2017-07-28 11:21 ` Richard Biener
2017-07-31 6:44 ` [PATCH v2] " Martin Liška
2017-07-31 8:50 ` Richard Biener
2017-07-31 8:53 ` Martin Liška
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).