* [PATCH] Dump BB number when dumping a BB with label. @ 2017-07-27 14:24 Martin Liška 2017-07-28 7:21 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Martin Liška @ 2017-07-27 14:24 UTC (permalink / raw) To: gcc-patches; +Cc: Jan Hubicka [-- Attachment #1: Type: text/plain, Size: 1866 bytes --] 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(-) [-- Attachment #2: 0001-Dump-BB-number-when-dumping-a-BB-with-label.patch --] [-- Type: text/x-patch, Size: 2719 bytes --] 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" } } */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label. 2017-07-27 14:24 [PATCH] Dump BB number when dumping a BB with label Martin Liška @ 2017-07-28 7:21 ` Richard Biener 2017-07-28 7:50 ` Martin Liška 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2017-07-28 7:21 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches, Jan Hubicka 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(-) > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label. 2017-07-28 7:21 ` Richard Biener @ 2017-07-28 7:50 ` Martin Liška 2017-07-28 7:58 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Martin Liška @ 2017-07-28 7:50 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Jan Hubicka 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(-) >> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label. 2017-07-28 7:50 ` Martin Liška @ 2017-07-28 7:58 ` Richard Biener 2017-07-28 10:53 ` Martin Liška 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2017-07-28 7:58 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches, Jan Hubicka 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(-) >>> >>> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label. 2017-07-28 7:58 ` Richard Biener @ 2017-07-28 10:53 ` Martin Liška 2017-07-28 11:21 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Martin Liška @ 2017-07-28 10:53 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Jan Hubicka 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(-) >>>> >>>> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Dump BB number when dumping a BB with label. 2017-07-28 10:53 ` Martin Liška @ 2017-07-28 11:21 ` Richard Biener 2017-07-31 6:44 ` [PATCH v2] " Martin Liška 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2017-07-28 11:21 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches, Jan Hubicka 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(-) >>>>> >>>>> >>> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] Dump BB number when dumping a BB with label. 2017-07-28 11:21 ` Richard Biener @ 2017-07-31 6:44 ` Martin Liška 2017-07-31 8:50 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Martin Liška @ 2017-07-31 6:44 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Jan Hubicka [-- Attachment #1: Type: text/plain, Size: 8265 bytes --] On 07/28/2017 01:21 PM, Richard Biener wrote: > 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. 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? Martin > >> 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(-) >>>>>> >>>>>> >>>> >> [-- Attachment #2: 0001-Learn-GIMPLE-pretty-printer-to-produce-nicer-dump-ou-v2.patch --] [-- Type: text/x-patch, Size: 4276 bytes --] From 09225795a538acd70e72fcb755ece11631660f35 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Fri, 28 Jul 2017 12:53:38 +0200 Subject: [PATCH] Learn GIMPLE pretty printer to produce nicer dump output. gcc/ChangeLog: 2017-07-28 Martin Liska <mliska@suse.cz> * gimple-pretty-print.c (dump_gimple_label): Never dump BB info. (dump_gimple_bb_header): Always dump BB info. (pp_cfg_jump): Do not append info about BB when dumping a jump. gcc/testsuite/ChangeLog: 2017-07-28 Martin Liska <mliska@suse.cz> * gcc.dg/builtin-unreachable-6.c: Update scanned patterns. * gcc.dg/tree-ssa/attr-hotcold-2.c: Likewise. --- gcc/gimple-pretty-print.c | 33 ++++++-------------------- gcc/testsuite/gcc.dg/builtin-unreachable-6.c | 2 +- gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c | 4 ++-- 3 files changed, 10 insertions(+), 29 deletions(-) diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index c8eb9c4a7bf..8b69b72e9e2 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -1120,9 +1120,6 @@ dump_gimple_label (pretty_printer *buffer, glabel *gs, int spc, else { 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)); pp_colon (buffer); } if (flags & TDF_GIMPLE) @@ -2695,16 +2692,12 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent, } else { - gimple *stmt = first_stmt (bb); - if (!stmt || gimple_code (stmt) != GIMPLE_LABEL) - { - if (flags & TDF_GIMPLE) - fprintf (outf, "%*sbb_%d:\n", indent, "", bb->index); - else - fprintf (outf, "%*s<bb %d> %s:\n", - indent, "", bb->index, dump_profile (bb->frequency, - bb->count)); - } + if (flags & TDF_GIMPLE) + fprintf (outf, "%*sbb_%d:\n", indent, "", bb->index); + else + fprintf (outf, "%*s<bb %d> %s:\n", + indent, "", bb->index, dump_profile (bb->frequency, + bb->count)); } } @@ -2760,22 +2753,10 @@ pp_cfg_jump (pretty_printer *buffer, edge e, dump_flags_t flags) } else { - gimple *stmt = first_stmt (e->dest); - pp_string (buffer, "goto <bb "); pp_decimal_int (buffer, e->dest->index); pp_greater (buffer); - if (stmt && gimple_code (stmt) == GIMPLE_LABEL) - { - pp_string (buffer, " ("); - dump_generic_node (buffer, - gimple_label_label (as_a <glabel *> (stmt)), - 0, 0, false); - pp_right_paren (buffer); - pp_semicolon (buffer); - } - else - pp_semicolon (buffer); + pp_semicolon (buffer); dump_edge_probability (buffer, e); } diff --git a/gcc/testsuite/gcc.dg/builtin-unreachable-6.c b/gcc/testsuite/gcc.dg/builtin-unreachable-6.c index d2596e95c3f..2f8ca369546 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:" 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..5f7e3afa2ae 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-times "combined heuristics: 0\\\..*" 1 "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-times "combined heuristics: \[6-9\]\[0-9\]\\\..*" 1 "profile_estimate" } } */ -- 2.13.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Dump BB number when dumping a BB with label. 2017-07-31 6:44 ` [PATCH v2] " Martin Liška @ 2017-07-31 8:50 ` Richard Biener 2017-07-31 8:53 ` Martin Liška 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2017-07-31 8:50 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches, Jan Hubicka On Mon, Jul 31, 2017 at 8:43 AM, Martin Liška <mliska@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 <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. > > 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 <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(-) >>>>>>> >>>>>>> >>>>> >>> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Dump BB number when dumping a BB with label. 2017-07-31 8:50 ` Richard Biener @ 2017-07-31 8:53 ` Martin Liška 0 siblings, 0 replies; 9+ messages in thread From: Martin Liška @ 2017-07-31 8:53 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Jan Hubicka On 07/31/2017 10:50 AM, Richard Biener wrote: > On Mon, Jul 31, 2017 at 8:43 AM, Martin LiÅ¡ka <mliska@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 <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. >> >> 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 <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(-) >>>>>>>> >>>>>>>> >>>>>> >>>> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-07-31 8:53 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-07-27 14:24 [PATCH] Dump BB number when dumping a BB with label Martin Liška 2017-07-28 7:21 ` Richard Biener 2017-07-28 7:50 ` Martin Liška 2017-07-28 7:58 ` Richard Biener 2017-07-28 10:53 ` Martin Liška 2017-07-28 11:21 ` Richard Biener 2017-07-31 6:44 ` [PATCH v2] " Martin Liška 2017-07-31 8:50 ` Richard Biener 2017-07-31 8:53 ` Martin Liška
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).