Message ID | CABu31nNuGOoAXDAbB2cg=yxzPK7sb8mSy1sa1hye=vrR5JwT2Q@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Mon, Dec 3, 2012 at 5:54 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote: > Hello, > > This is the last patch in this series for now, unless I can convince > everyone that some files should be renamed after all these changes :-) > > The attached patch makes graph.c independent of the IR contained in > the CFG it dumps. For this, I made the node label dumping routine a > cfghook. I also changed the dumper to take a struct function rather > than a fndecl tree, and use the funcdef_no as the unique number for > the function. This helps when annotating the graph with profile > information from .gcda files (I have a tool for that :-). > > I also noticed I introduced a bug in the dumping of REG_NOTEs in slim > RTL dumps. Also fixed by this patch. > > Bootstrapped&tested on powerpc64-unknown-linux-gnu. OK? Ok! Please watch for fallout of this series. Thanks, Richard. > Ciao! > Steven
Nice. We used to just do post-processing of the dumps with -blocks option. I assume the graph dump does not support multiple function dump (I noticed that the previous function's dump gets overwritten). This reminds me that I need to resurrect my per-function dump support, and dump-before/after patches at some point. Why can't the dot dumper dump the count/frequency/branch prob information? The attached dot file is generated from an assembly file dumped with -S -dA option. thanks, David On Mon, Dec 3, 2012 at 8:54 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote: > Hello, > > This is the last patch in this series for now, unless I can convince > everyone that some files should be renamed after all these changes :-) > > The attached patch makes graph.c independent of the IR contained in > the CFG it dumps. For this, I made the node label dumping routine a > cfghook. I also changed the dumper to take a struct function rather > than a fndecl tree, and use the funcdef_no as the unique number for > the function. This helps when annotating the graph with profile > information from .gcda files (I have a tool for that :-). > > I also noticed I introduced a bug in the dumping of REG_NOTEs in slim > RTL dumps. Also fixed by this patch. > > Bootstrapped&tested on powerpc64-unknown-linux-gnu. OK? > > Ciao! > Steven
On Tue, Dec 4, 2012 at 9:14 PM, Xinliang David Li wrote: > I assume the graph dump does not support multiple function dump (I > noticed that the previous function's dump gets overwritten). This > reminds me that I need to resurrect my per-function dump support, and > dump-before/after patches at some point. Not sure what you mean with "multiple function dump"...? What my dumper does, is dump each function as a subgraph. If you dump a translation unit with more than one function, all function graphs will be in the same file. With a good DOT viewer, it's still quite readable. I have a perl scriptlet in the pipe line to split one DOT file into per-function DOT files, and to merge per-function DOT files from different passes. > Why can't the dot dumper dump the count/frequency/branch prob > information? The attached dot file is generated from an assembly file > dumped with -S -dA option. It can, and should. I just didn't get around to that yet. I want to add the count//freq/branchprob as edge labels, similar to your dump. I also want to add it to the basic blocks as well but I'm not sure how to pretty-print it such that it doesn't clobber the view of the instructions contained in the basic block. Unfortunately DOT doesn't support edge classes or layers you can toggle. I even had the dumper print the GIMPLE SSA web and DF DU/UD chains, using ports on the fields in the record nodes that I use for dumping the instructions. But that really cluttered the graph too much. I'm still going to add the ports, at least, to use in dumping the scheduler's dependence graph. The whole graph dumping infrastructure will get another overhaul for GCC 4.9 to make it easier to use for non-CFG dumps and to allow passes to add their own meta-data to the dumps. Ciao! Steven
On Tue, Dec 4, 2012 at 12:47 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote: > On Tue, Dec 4, 2012 at 9:14 PM, Xinliang David Li wrote: >> I assume the graph dump does not support multiple function dump (I >> noticed that the previous function's dump gets overwritten). This >> reminds me that I need to resurrect my per-function dump support, and >> dump-before/after patches at some point. > > Not sure what you mean with "multiple function dump"...? > > What my dumper does, is dump each function as a subgraph. If you dump > a translation unit with more than one function, all function graphs > will be in the same file. With a good DOT viewer, it's still quite > readable. Was it a bug then? for this program: typedef unsigned int uint; int foo (uint a, uint b, uint c, uint d, uint e, uint f) { return (a*b + (c << 1) + (d << e) + f + 1); } int foo2 (uint a, uint b, uint c, uint d, uint e, uint f) { return ((f<<e) + (d << 1) + (c*b) + a + 1); } The dot file generated with -fdump-tree-optimized-graph is digraph "" { overlap=false; subgraph "foo2" { color="black"; label="foo2"; fn_1_basic_block_1 [shape=Mdiamond,style=filled,fillcolor=white,label="EXIT"]; fn_1_basic_block_2 [shape=record,style=filled,fillcolor=lightgrey,label="{\<bb\ 2\>:\l\ |e.1_2\ =\ (int)\ e_1(D);\l\ |_4\ =\ f_3(D)\ \<\<\ e.1_2;\l\ |_6\ =\ d_5(D)\ \<\<\ 1;\l\ |_10\ =\ c_8(D)\ *\ b_9(D);\l\ |_7\ =\ a_12(D)\ +\ 1;\l\ |_11\ =\ _7\ +\ _10;\l\ |_13\ =\ _11\ +\ _6;\l\ |_14\ =\ _13\ +\ _4;\l\ |_15\ =\ (int)\ _14;\l\ |return\ _15;\l\ }"]; fn_1_basic_block_0 [shape=Mdiamond,style=filled,fillcolor=white,label="ENTRY"]; fn_1_basic_block_0:s -> fn_1_basic_block_2:n [style="solid,bold",color=blue,weight=100,constraint=true]; fn_1_basic_block_2:s -> fn_1_basic_block_1:n [style="solid,bold",color=black,weight=10,constraint=true]; } } There is no subgraph -- did I miss anything? > > I have a perl scriptlet in the pipe line to split one DOT file into > per-function DOT files, and to merge per-function DOT files from > different passes. It will be nicer to avoid any postprocessing. Once we have the ability to dump one function at a time, the support will be there automatically. > > >> Why can't the dot dumper dump the count/frequency/branch prob >> information? The attached dot file is generated from an assembly file >> dumped with -S -dA option. > > It can, and should. I just didn't get around to that yet. ok. > > I want to add the count//freq/branchprob as edge labels, similar to > your dump. I also want to add it to the basic blocks as well but I'm > not sure how to pretty-print it such that it doesn't clobber the view > of the instructions contained in the basic block. Unfortunately DOT > doesn't support edge classes or layers you can toggle. > > I even had the dumper print the GIMPLE SSA web and DF DU/UD chains, > using ports on the fields in the record nodes that I use for dumping > the instructions. But that really cluttered the graph too much. I'm > still going to add the ports, at least, to use in dumping the > scheduler's dependence graph. DU/UD chains are good source of information -- the problem is it can be too dense. Conceivably, this can be dumped on-demand for selected webs. > > The whole graph dumping infrastructure will get another overhaul for > GCC 4.9 to make it easier to use for non-CFG dumps and to allow passes > to add their own meta-data to the dumps. Do you have a brief design somewhere? thanks, David > > Ciao! > Steven
On Tue, Dec 4, 2012 at 10:18 PM, Xinliang David Li wrote: > Was it a bug then? Yup, a bug. One I introduced myself with the passes.c changes. With the attached patch I get this dump: --------- 8< ----------- digraph "" { overlap=false; subgraph "foo" { color="black"; label="foo"; fn_0_basic_block_1 [shape=Mdiamond,style=filled,fillcolor=white,label="EXIT"]; fn_0_basic_block_2 [shape=record,style=filled,fillcolor=lightgrey,label="{\<bb\ 2\>:\l\ |_3\ =\ a_1(D)\ *\ b_2(D);\l\ |_5\ =\ c_4(D)\ \<\<\ 1;\l\ |e.0_8\ =\ (int)\ e_7(D);\l\ |_10\ =\ d_9(D)\ \<\<\ e.0_8;\l\ |_6\ =\ f_12(D)\ +\ 1;\l\ |_11\ =\ _6\ +\ _5;\l\ |_13\ =\ _11\ +\ _10;\l\ |_14\ =\ _13\ +\ _3;\l\ |_15\ =\ (int)\ _14;\l\ |return\ _15;\l\ }"]; fn_0_basic_block_0 [shape=Mdiamond,style=filled,fillcolor=white,label="ENTRY"]; fn_0_basic_block_0:s -> fn_0_basic_block_2:n [style="solid,bold",color=blue,weight=100,constraint=true]; fn_0_basic_block_2:s -> fn_0_basic_block_1:n [style="solid,bold",color=black,weight=10,constraint=true]; } subgraph "foo2" { color="black"; label="foo2"; fn_1_basic_block_1 [shape=Mdiamond,style=filled,fillcolor=white,label="EXIT"]; fn_1_basic_block_2 [shape=record,style=filled,fillcolor=lightgrey,label="{\<bb\ 2\>:\l\ |e.1_2\ =\ (int)\ e_1(D);\l\ |_4\ =\ f_3(D)\ \<\<\ e.1_2;\l\ |_6\ =\ d_5(D)\ \<\<\ 1;\l\ |_10\ =\ c_8(D)\ *\ b_9(D);\l\ |_7\ =\ a_12(D)\ +\ 1;\l\ |_11\ =\ _7\ +\ _10;\l\ |_13\ =\ _11\ +\ _6;\l\ |_14\ =\ _13\ +\ _4;\l\ |_15\ =\ (int)\ _14;\l\ |return\ _15;\l\ }"]; fn_1_basic_block_0 [shape=Mdiamond,style=filled,fillcolor=white,label="ENTRY"]; fn_1_basic_block_0:s -> fn_1_basic_block_2:n [style="solid,bold",color=blue,weight=100,constraint=true]; fn_1_basic_block_2:s -> fn_1_basic_block_1:n [style="solid,bold",color=black,weight=10,constraint=true]; } } --------- 8< ----------- > It will be nicer to avoid any postprocessing. Once we have the ability > to dump one function at a time, the support will be there > automatically. True. >> The whole graph dumping infrastructure will get another overhaul for >> GCC 4.9 to make it easier to use for non-CFG dumps and to allow passes >> to add their own meta-data to the dumps. > > Do you have a brief design somewhere? Other than in my head? Not really. I'm thinking of something like "graph_traits" and a template class for graph dumping, and make the CFG dumper one specialization of that template (and later add specializations for e.g. the DDG, alias constraint graph, and call graph). But for me, for now, it's more about exploring C++ (which is totally new to me -- I'm a rocket scientist, not a hacker :-) than about having a Grand Design ready. I'll test the patch and commit it if it passes (it partially reverts one of my earlier changes). Thanks for the feedback! Ciao! Steven
Index: rtl.h =================================================================== --- rtl.h (revision 194085) +++ rtl.h (working copy) @@ -2606,7 +2606,7 @@ extern void dump_rtl_slim (FILE *, const_rtx, cons extern void print_value (pretty_printer *, const_rtx, int); extern void print_pattern (pretty_printer *, const_rtx, int); extern void print_insn (pretty_printer *, const_rtx, int); -extern void print_insn_with_notes (pretty_printer *, const_rtx); +extern void rtl_dump_bb_for_graph (pretty_printer *, basic_block); extern const char *str_pattern_slim (const_rtx); /* In function.c */ Index: sched-vis.c =================================================================== --- sched-vis.c (revision 194085) +++ sched-vis.c (working copy) @@ -716,10 +716,10 @@ print_insn (pretty_printer *pp, const_rtx x, int v } } /* print_insn */ -/* Prerry-print a slim dump of X (an insn) to PP, including any register +/* Pretty-print a slim dump of X (an insn) to PP, including any register note attached to the instruction. */ -void +static void print_insn_with_notes (pretty_printer *pp, const_rtx x) { pp_string (pp, print_rtx_head); @@ -728,9 +728,9 @@ print_insn_with_notes (pretty_printer *pp, const_r if (INSN_P (x) && REG_NOTES (x)) for (rtx note = REG_NOTES (x); note; note = XEXP (note, 1)) { - pp_printf (pp, "%s %s", print_rtx_head, + pp_printf (pp, "%s %s ", print_rtx_head, GET_REG_NOTE_NAME (REG_NOTE_KIND (note))); - print_pattern (pp, XEXP (note, 0), 1); + print_pattern (pp, XEXP (note, 0), 1); pp_newline (pp); } } @@ -800,6 +800,29 @@ dump_rtl_slim (FILE *f, const_rtx first, const_rtx pp_flush (pp); } +/* Dumps basic block BB to pretty-printer PP in slim form and without and + no indentation, for use as a label of a DOT graph record-node. */ + +void +rtl_dump_bb_for_graph (pretty_printer *pp, basic_block bb) +{ + rtx insn; + bool first = true; + + /* TODO: inter-bb stuff. */ + FOR_BB_INSNS (bb, insn) + { + if (! first) + { + pp_character (pp, '|'); + pp_write_text_to_stream (pp); + } + first = false; + print_insn_with_notes (pp, insn); + pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/true); + } +} + /* Pretty-print pattern X of some insn in non-verbose mode. Return a string pointer to the pretty-printer buffer. Index: cfghooks.h =================================================================== --- cfghooks.h (revision 194084) +++ cfghooks.h (working copy) @@ -29,6 +29,7 @@ struct cfg_hooks /* Debugging. */ int (*verify_flow_info) (void); void (*dump_bb) (FILE *, basic_block, int, int); + void (dump_bb_for_graph) (pretty_printer *, basic_block); /* Basic CFG manipulation. */ @@ -152,6 +153,8 @@ struct cfg_hooks extern void verify_flow_info (void); extern void dump_bb (FILE *, basic_block, int, int); +extern void dump_bb_for_graph (pretty_printer *, basic_block); + extern edge redirect_edge_and_branch (edge, basic_block); extern basic_block redirect_edge_and_branch_force (edge, basic_block); extern bool can_remove_branch_p (const_edge); Index: cfghooks.c =================================================================== --- cfghooks.c (revision 194084) +++ cfghooks.c (working copy) @@ -280,6 +280,22 @@ dump_bb (FILE *outf, basic_block bb, int indent, i fputc ('\n', outf); } +/* Dumps basic block BB to pretty-printer PP, for use as a label of + a DOT graph record-node. The implementation of this hook is + expected to write the label to the stream that is attached to PP. + Field separators between instructions are pipe characters printed + verbatim. Instructions should be written with some characters + escaped, using pp_write_text_as_dot_label_to_stream(). */ + +void +dump_bb_for_graph (pretty_printer *pp, basic_block bb) +{ + if (!cfg_hooks->dump_bb_for_graph) + internal_error ("%s does not support dump_bb_for_graph", + cfg_hooks->name); + cfg_hooks->dump_bb_for_graph (pp, bb); +} + /* Dump the complete CFG to FILE. FLAGS are the TDF_* flags in dumpfile.h. */ void dump_flow_info (FILE *file, int flags) Index: tree-cfg.c =================================================================== --- tree-cfg.c (revision 194084) +++ tree-cfg.c (working copy) @@ -7620,6 +7620,7 @@ struct cfg_hooks gimple_cfg_hooks = { "gimple", gimple_verify_flow_info, gimple_dump_bb, /* dump_bb */ + gimple_dump_bb_for_graph, /* dump_bb_for_graph */ create_bb, /* create_basic_block */ gimple_redirect_edge_and_branch, /* redirect_edge_and_branch */ gimple_redirect_edge_and_branch_force, /* redirect_edge_and_branch_force */ Index: cfgrtl.c =================================================================== --- cfgrtl.c (revision 194084) +++ cfgrtl.c (working copy) @@ -4529,6 +4529,7 @@ struct cfg_hooks rtl_cfg_hooks = { "rtl", rtl_verify_flow_info, rtl_dump_bb, + rtl_dump_bb_for_graph, rtl_create_basic_block, rtl_redirect_edge_and_branch, rtl_redirect_edge_and_branch_force, @@ -4570,6 +4571,7 @@ struct cfg_hooks cfg_layout_rtl_cfg_hooks = { "cfglayout mode", rtl_verify_flow_info_1, rtl_dump_bb, + rtl_dump_bb_for_graph, cfg_layout_create_basic_block, cfg_layout_redirect_edge_and_branch, cfg_layout_redirect_edge_and_branch_force, Index: graph.c =================================================================== --- graph.c (revision 194085) +++ graph.c (working copy) @@ -24,15 +24,10 @@ along with GCC; see the file COPYING3. If not see #include "system.h" #include "coretypes.h" #include "diagnostic-core.h" /* for fatal_error */ -#include "sbitmap.h" #include "basic-block.h" -#include "rtl.h" -#include "tree.h" -#include "gimple.h" #include "graph.h" #include "dumpfile.h" #include "pretty-print.h" -#include "gimple-pretty-print.h" /* DOT files with the .dot extension are recognized as document templates by a well-known piece of word processing software out of Redmond, WA. @@ -80,10 +75,10 @@ init_graph_slim_pretty_print (FILE *fp) return &graph_slim_pp; } -/* Draw a basic block BB belonging to the function with FNDECL_UID +/* Draw a basic block BB belonging to the function with FUNCDEF_NO as its unique number. */ static void -draw_cfg_node (pretty_printer *pp, int fndecl_uid, basic_block bb) +draw_cfg_node (pretty_printer *pp, int funcdef_no, basic_block bb) { const char *shape; const char *fillcolor; @@ -105,7 +100,7 @@ static void pp_printf (pp, "\tfn_%d_basic_block_%d " "[shape=%s,style=filled,fillcolor=%s,label=\"", - fndecl_uid, bb->index, shape, fillcolor); + funcdef_no, bb->index, shape, fillcolor); if (bb->index == ENTRY_BLOCK) pp_string (pp, "ENTRY"); @@ -115,28 +110,7 @@ static void { pp_character (pp, '{'); pp_write_text_to_stream (pp); - - /* This would be easier if there'd be an IR independent iterator... */ - if (current_ir_type () == IR_GIMPLE) - gimple_dump_bb_for_graph (pp, bb); - else - { - rtx insn; - bool first = true; - - /* TODO: inter-bb stuff. */ - FOR_BB_INSNS (bb, insn) - { - if (! first) - { - pp_character (pp, '|'); - pp_write_text_to_stream (pp); - } - first = false; - print_insn_with_notes (pp, insn); - pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/true); - } - } + dump_bb_for_graph (pp, bb); pp_character (pp, '}'); } @@ -145,9 +119,9 @@ static void } /* Draw all successor edges of a basic block BB belonging to the function - with FNDECL_UID as its unique number. */ + with FUNCDEF_NO as its unique number. */ static void -draw_cfg_node_succ_edges (pretty_printer *pp, int fndecl_uid, basic_block bb) +draw_cfg_node_succ_edges (pretty_printer *pp, int funcdef_no, basic_block bb) { edge e; edge_iterator ei; @@ -181,8 +155,8 @@ static void pp_printf (pp, "\tfn_%d_basic_block_%d:s -> fn_%d_basic_block_%d:n " "[style=%s,color=%s,weight=%d,constraint=%s];\n", - fndecl_uid, e->src->index, - fndecl_uid, e->dest->index, + funcdef_no, e->src->index, + funcdef_no, e->dest->index, style, color, weight, (e->flags & (EDGE_FAKE | EDGE_DFS_BACK)) ? "false" : "true"); } @@ -192,10 +166,10 @@ static void /* Print a graphical representation of the CFG of function FUN. */ void -print_graph_cfg (const char *base, tree fndecl) +print_graph_cfg (const char *base, struct function *fun) { - const char *funcname = fndecl_name (fndecl); - int fndecl_uid = DECL_UID (fndecl); + const char *funcname = function_name (fun); + int funcdef_no = fun->funcdef_no; FILE *fp = open_graph_file (base, "a"); int *rpo = XNEWVEC (int, n_basic_blocks); basic_block bb; @@ -212,7 +186,7 @@ void of the nodes. */ n = pre_and_rev_post_order_compute (NULL, rpo, true); for (i = 0; i < n; i++) - draw_cfg_node (pp, fndecl_uid, BASIC_BLOCK (rpo[i])); + draw_cfg_node (pp, funcdef_no, BASIC_BLOCK (rpo[i])); /* Draw all edges at the end to get subgraphs right for GraphViz, which requires nodes to be defined before edges to cluster @@ -224,7 +198,7 @@ void for ourselves is also not desirable.) */ mark_dfs_back_edges (); FOR_ALL_BB (bb) - draw_cfg_node_succ_edges (pp, fndecl_uid, bb); + draw_cfg_node_succ_edges (pp, funcdef_no, bb); pp_printf (pp, "\t}\n"); pp_flush (pp); Index: graph.h =================================================================== --- graph.h (revision 194085) +++ graph.h (working copy) @@ -20,7 +20,7 @@ along with GCC; see the file COPYING3. If not see #ifndef GCC_GRAPH_H #define GCC_GRAPH_H -extern void print_graph_cfg (const char *, tree); +extern void print_graph_cfg (const char *, struct function *); extern void clean_graph_dump_file (const char *); extern void finish_graph_dump_file (const char *); Index: passes.c =================================================================== --- passes.c (revision 194085) +++ passes.c (working copy) @@ -1770,7 +1770,7 @@ execute_function_dump (void *data ATTRIBUTE_UNUSED if ((cfun->curr_properties & PROP_cfg) && (dump_flags & TDF_GRAPH)) - print_graph_cfg (dump_file_name, cfun->decl); + print_graph_cfg (dump_file_name, cfun); } } Index: Makefile.in =================================================================== --- Makefile.in (revision 194084) +++ Makefile.in (working copy) @@ -1846,7 +1846,7 @@ gcc.srcextra: gengtype-lex.c -cp -p $^ $(srcdir) graph.o: graph.c graph.h $(CONFIG_H) $(SYSTEM_H) coretypes.h \ - $(DIAGNOSTIC_CORE_H) $(TM_H) $(RTL_H) $(BASIC_BLOCK_H) $(PRETTY_PRINT_H) + $(DIAGNOSTIC_CORE_H) $(BASIC_BLOCK_H) $(PRETTY_PRINT_H) dumpfile.h sbitmap.o: sbitmap.c sbitmap.h $(CONFIG_H) $(SYSTEM_H) coretypes.h sparseset.o: sparseset.c $(SYSTEM_H) sparseset.h $(CONFIG_H)