diff mbox

[stage1] Make function names visible in -fdump-rtl-*-graph

Message ID 000601d05d5e$d106d7e0$731487a0$@arm.com
State New
Headers show

Commit Message

Thomas Preud'homme March 13, 2015, 7:24 a.m. UTC
Hi,

The description is longer than the patch so you might want to skip directly to it.

The dot file generated by -fdump-rtl-*-graph switches group basic blocks for a given function together in a subgraph and use the function name as the label. However, when generating an image (for instance a svg with "dot -Tsvg") the label does not appear. This makes analyzing the resulting file more difficult than it should be.

The section "Subgraphs and clusters" of "The DOT language" document contains the following excerpt:
 
"The third role for subgraphs directly involves how the graph will be laid out by certain layout engines. If the name of the subgraph begins with cluster, Graphviz notes the subgraph as a special cluster subgraph. If supported, the layout engine will do the layout so that the nodes belonging to the cluster are drawn together, with the entire drawing of the cluster contained within a bounding rectangle. Note that, for good and bad, cluster subgraphs are not part of the DOT language, but solely a syntactic convention adhered to by certain of the layout engines."

Hence prepending cluster_ to subgraph id (not its label) would improve the output image with many layout engines while no doing any difference for other layout engines. The patch also make the subgraph boudary visible with dashed lines and add "()" to the label of the subgraph (so for a function f the label would be "f ()").

ChangeLog entry is as follows:

*** gcc/ChangeLog ***

2015-03-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * graph.c (print_graph_cfg): Make function names visible and append
        parenthesis to it.  Also make groups of basic blocks belonging to the
        same function visible.



Is this ok for stage1? It's not a bug but it helps debuggability so is this something we might consider backporting?

Best regards,

Thomas

Comments

Richard Biener March 13, 2015, 9:01 a.m. UTC | #1
On Fri, 13 Mar 2015, Thomas Preud'homme wrote:

> Hi,
> 
> The description is longer than the patch so you might want to skip directly to it.
> 
> The dot file generated by -fdump-rtl-*-graph switches group basic blocks for a given function together in a subgraph and use the function name as the label. However, when generating an image (for instance a svg with "dot -Tsvg") the label does not appear. This makes analyzing the resulting file more difficult than it should be.
> 
> The section "Subgraphs and clusters" of "The DOT language" document contains the following excerpt:
>  
> "The third role for subgraphs directly involves how the graph will be laid out by certain layout engines. If the name of the subgraph begins with cluster, Graphviz notes the subgraph as a special cluster subgraph. If supported, the layout engine will do the layout so that the nodes belonging to the cluster are drawn together, with the entire drawing of the cluster contained within a bounding rectangle. Note that, for good and bad, cluster subgraphs are not part of the DOT language, but solely a syntactic convention adhered to by certain of the layout engines."
> 
> Hence prepending cluster_ to subgraph id (not its label) would improve the output image with many layout engines while no doing any difference for other layout engines. The patch also make the subgraph boudary visible with dashed lines and add "()" to the label of the subgraph (so for a function f the label would be "f ()").
> 
> ChangeLog entry is as follows:
> 
> *** gcc/ChangeLog ***
> 
> 2015-03-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * graph.c (print_graph_cfg): Make function names visible and append
>         parenthesis to it.  Also make groups of basic blocks belonging to the
>         same function visible.
> 
> 
> diff --git a/gcc/graph.c b/gcc/graph.c
> index a1eb24c..5fb0d78 100644
> --- a/gcc/graph.c
> +++ b/gcc/graph.c
> @@ -292,9 +292,10 @@ print_graph_cfg (const char *base, struct function *fun)
>    pretty_printer graph_slim_pp;
>    graph_slim_pp.buffer->stream = fp;
>    pretty_printer *const pp = &graph_slim_pp;
> -  pp_printf (pp, "subgraph \"%s\" {\n"
> -	         "\tcolor=\"black\";\n"
> -		 "\tlabel=\"%s\";\n",
> +  pp_printf (pp, "subgraph \"cluster_%s\" {\n"
> +		 "\tstyle=\"dashed\";\n"
> +		 "\tcolor=\"black\";\n"
> +		 "\tlabel=\"%s ()\";\n",
>  		 funcname, funcname);
>    draw_cfg_nodes (pp, fun);
>    draw_cfg_edges (pp, fun);
> 
> Is this ok for stage1? It's not a bug but it helps debuggability so is 
> this something we might consider backporting?

It's ok now given you bootstrapped the change.

Thanks,
Richard.
Thomas Preud'homme March 13, 2015, 9:46 a.m. UTC | #2
> From: Richard Biener [mailto:rguenther@suse.de]
> Sent: Friday, March 13, 2015 5:02 PM
> >
> > Is this ok for stage1? It's not a bug but it helps debuggability so is
> > this something we might consider backporting?
> 
> It's ok now given you bootstrapped the change.

I did + regression testsuite on both arm-none-eabi and
x86_64-linux-gnu. I forgot to mentioned it sorry.

I just committed it.

Best regards,

Thomas
diff mbox

Patch

diff --git a/gcc/graph.c b/gcc/graph.c
index a1eb24c..5fb0d78 100644
--- a/gcc/graph.c
+++ b/gcc/graph.c
@@ -292,9 +292,10 @@  print_graph_cfg (const char *base, struct function *fun)
   pretty_printer graph_slim_pp;
   graph_slim_pp.buffer->stream = fp;
   pretty_printer *const pp = &graph_slim_pp;
-  pp_printf (pp, "subgraph \"%s\" {\n"
-	         "\tcolor=\"black\";\n"
-		 "\tlabel=\"%s\";\n",
+  pp_printf (pp, "subgraph \"cluster_%s\" {\n"
+		 "\tstyle=\"dashed\";\n"
+		 "\tcolor=\"black\";\n"
+		 "\tlabel=\"%s ()\";\n",
 		 funcname, funcname);
   draw_cfg_nodes (pp, fun);
   draw_cfg_edges (pp, fun);