| Message ID | 09f05b6e-99bd-0238-e39b-185c5df95fe5@suse.cz |
|---|---|
| State | New |
| Headers | show |
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(-) > >
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(-) >> >>
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(-) >>> >>> >
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(-) >>>> >>>> >>
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(-) >>>>> >>>>> >>> >
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" } } */
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(-)