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? 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(-) >>>>>> >>>>>> >>>> >>
>From 09225795a538acd70e72fcb755ece11631660f35 Mon Sep 17 00:00:00 2001 From: marxin <mli...@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 <mli...@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 <mli...@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