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