On 07/31/2017 10:50 AM, Richard Biener wrote:
> 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.
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 <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(-)
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
