On Mon, Jul 31, 2017 at 8:43 AM, Martin Liška <mli...@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 <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?
Ok. Nice that it also simplifies code.
Thanks,
Richard.
> 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(-)
>>>>>>>
>>>>>>>
>>>>>
>>>
>
