Hookize CFG DOT graph dumping

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
* rtl.h (print_insn_with_notes): Remove prototype.
	(rtl_dump_bb_for_graph): New prototype.
	* sched-vis.c (print_insn_with_notes): Make static again.  Fix
	printing of notes.
	(rtl_dump_bb_for_graph): New function.
	* cfghooks.h (struct cfg_hooks) <dump_bb_for_graph>: New hook.
	(dump_bb_for_graph): New prototype.
	* cfghooks.c (dump_bb_for_graph): New function.
	* tree-cfg.c (gimple_cfg_hooks): Register gimple_dump_bb_for_graph
	as dump_bb_for_graph hook implementation for GIMPLE.
	* cfgrtl.c (rtl_cfg_hooks): Likewise for rtl_dump_bb_for_graph.
	(cfg_layout_rtl_cfg_hooks): Likewise.
	* graph.c (draw_cfg_node): Don't include sbitmap.h, rtl.h, tree.h,
	gimple.h, and gimple-pretty-print.h.
	(draw_cfg_node, draw_cfg_node_succ_edges): Use the uniqe function
	definition number instead of the function declaration UID.
	(print_graph_cfg): Take a struct function instead of a tree.
	Use the dump_bb_for_graph hook to dump the basic block content.
	* graph.h (print_graph_cfg): Update prototype.
	* passes.c (execute_function_dump): Update print_graph_cfg call.
	* Makefile.in (graph.o): Fixup dependencies.

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