Re: [PATCH] Dump BB number when dumping a BB with label.

[Richard Biener <richard dot guenther at gmail dot com>]

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