Hi, Liška
I incorporated all your suggestions to the patch, but before sending the
v2 I want to discuss some errors reported by the style script.
>=== ERROR type #1: blocks of 8 spaces should be replaced with tabs (16
>error(s)) ===
>gcc/lto/lto-dump.c:263:22: " -list [options]████████ Dump the
>symbol list.\n"
>gcc/lto/lto-dump.c:264:18: " -demangle████████ Dump the
>demangled output.\n"
>gcc/lto/lto-dump.c:265:22: " -defined-only████████ Dump only the
>defined symbols.\n"
>gcc/lto/lto-dump.c:266:21: " -print-value████████ Dump initial
>values of the variables.\n"
>gcc/lto/lto-dump.c:267:19: " -name-sort████████ Sort the
>symbols alphabetically.\n"
>gcc/lto/lto-dump.c:268:19: " -size-sort████████ Sort the
>symbols according to size.\n"
>gcc/lto/lto-dump.c:269:22: " -reverse-sort████████ Dump the
>symbols in reverse order.\n"
>gcc/lto/lto-dump.c:270:15: " -symbol=████████████████ Dump the
>details of specific symbol.\n"
>gcc/lto/lto-dump.c:271:15: " -objects████████████████ Dump the
>details of LTO objects.\n"
>gcc/lto/lto-dump.c:272:17: " -callgraph████████████████Dump the
>callgraph in graphviz format.\n"
>gcc/lto/lto-dump.c:273:18: " -type-stats████████ Dump
>statistics of tree types.\n"
>gcc/lto/lto-dump.c:274:18: " -tree-stats████████ Dump
>statistics of trees.\n"
>gcc/lto/lto-dump.c:275:20: " -gimple-stats████████ Dump
>statistics of gimple statements.\n"
>gcc/lto/lto-dump.c:276:18: " -dump-body=████████ Dump the
>specific gimple body.\n"
>gcc/lto/lto-dump.c:277:19: " -dump-level=████████ Deciding the
>optimization level of body.\n"
>gcc/lto/lto-dump.c:278:12: " -help████████████████ Display the
>dump tool help.\n";
>
Is this correct? I think that replacing these spaces with tabs will do
bad things in some terminals.
>=== ERROR type #2: there should be exactly one space between function
>name and parenthesis (1 error(s)) ===
>gcc/lto/lang.opt:131:11:LTODump Var(flag_dump_callgraph)
I just followed the pattern in that file. Unless everything there is
wrong, the style in the patch is correct.
>
>=== ERROR type #3: there should be no space before a left square bracket
>(2 error(s)) ===
>gcc/lto/lto-dump.c:261:21: "Usage: lto-dump [OPTION]... SUB_COMMAND
>[OPTION]...\n\n"
>gcc/lto/lto-dump.c:263:13: " -list [options] Dump the
>symbol list.\n"
>
>=== ERROR type #4: trailing operator (1 error(s)) ===
>gcc/lto/lto-dump.c:260:18: const char *msg =
I think doing it in this way is cleaner than the previous version, where
there was a printf for each line. Therefore this change is less noisy
because I just write the string with keeping in mind with what will be
displayed in the terminal, and then print it. Of course this is my point
of view and it is completely subjective.
Any other style error suggested by the script were fixed.
On 07/01, Martin Liška wrote:
> On 7/1/19 2:56 PM, Giuliano Belinassi wrote:
> > Hi,
> >
> > On 07/01, Martin Jambor wrote:
> >> On Sat, Jun 29 2019, Giuliano Belinassi wrote:
> >>> This patch makes lto-dump dump the symbol table callgraph in graphviz
> >>> format.
> >> -ENOPATCH
> > Sorry,
> > I forgot the most important. Here it is.
>
> Hello.
>
> I like the intention! Please try to use:
> $ git diff HEAD > /tmp/patch && ~/Programming/gcc/contrib/check_GNU_style.py
> /tmp/patch
>
> which will show you some coding style issues.
>
> >
> >> Martin
> >>
> >>> I've not added any test to this because I couldn't find a way to call
> >>> lto-dump from the testsuite. Also, any feedback with regard to how can
> >>> I improve this is welcome.
> >>>
> >>> gcc/ChangeLog
> >>> 2019-06-29 Giuliano Belinassi <giuliano.belina...@usp.br>
> >>>
> >>> * cgraph.c (dump_graphviz): New function
> >>> * cgraph.h (dump_graphviz): New function
> >>> * symtab.c (dump_graphviz): New function
> >>> * varpool.c (dump_graphviz): New function
> >>>
> >>> gcc/lto/ChangeLog
> >>> 2019-06-29 Giuliano Belinassi <giuliano.belina...@usp.br>
> >>>
> >>> * lang.opt (flag_dump_callgraph): New flag
> >>> * lto-dump.c (dump_symtab_graphviz): New function
> >>> * lto-dump.c (dump_tool_help): New option
> >>> * lto-dump.c (lto_main): New option
> > Giuliano
> >
> > =======================================
> >
> > diff --git gcc/cgraph.c gcc/cgraph.c
> > index 28019aba434..55f4ee0bdaa 100644
> >
> > --- gcc/cgraph.c
> >
> > +++ gcc/cgraph.c
> >
> > @@ -2204,6 +2204,22 @@
> >
> > cgraph_node::dump (FILE *f)
> >
> > }
> > }
> > +/* Dump call graph node to file F in graphviz format. */
> > +
> > +void
> > +cgraph_node::dump_graphviz (FILE *f)
> > +{
> > + cgraph_edge *edge;
> > +
> > + for (edge = callees; edge; edge = edge->next_callee)
> > + {
> > + cgraph_node *callee = edge->callee;
> > +
> > + fprintf(f, "\t\"%s\" -> \"%s\"\n", name (), callee->name ());
> > + }
> > +}
> > +
> > +
> > /* Dump call graph node NODE to stderr. */
> > DEBUG_FUNCTION void
> >
> > =======================================
> >
> > diff --git gcc/cgraph.h gcc/cgraph.h
> > index 18839a4a5ec..181c00a3bd8 100644
> >
> > --- gcc/cgraph.h
> >
> > +++ gcc/cgraph.h
> >
> > @@ -135,6 +135,9 @@
> >
> > public:
> >
> > /* Dump symtab node to F. */
> > void dump (FILE *f);
> > + /* Dump symtab callgraph in graphviz format*/
> > + void dump_graphviz (FILE *f);
> > +
> > /* Dump symtab node to stderr. */
> > void DEBUG_FUNCTION debug (void);
> >
> > @@ -1106,6 +1109,9 @@
> >
> > public:
> >
> > /* Dump call graph node to file F. */
> > void dump (FILE *f);
> > + /* Dump call graph node to file F. */
> > + void dump_graphviz (FILE *f);
> > +
> > /* Dump call graph node to stderr. */
> > void DEBUG_FUNCTION debug (void);
> >
> > @@ -1861,6 +1867,9 @@
> >
> > public:
> >
> > /* Dump given varpool node to F. */
> > void dump (FILE *f);
> > + /* Dump given varpool node in graphviz format to F. */
> > + void dump_graphviz (FILE *f);
> > +
> > /* Dump given varpool node to stderr. */
> > void DEBUG_FUNCTION debug (void);
> >
> > @@ -2279,6 +2288,9 @@
> >
> > public:
> >
> > /* Dump symbol table to F. */
> > void dump (FILE *f);
> > + /* Dump symbol table to F in graphviz format. */
> > + void dump_graphviz (FILE *f);
> > +
>
> I would not define it for varpool_node as you don't need it.
>
> > /* Dump symbol table to stderr. */
> > void DEBUG_FUNCTION debug (void);
> >
> > =======================================
> >
> > diff --git gcc/lto/lang.opt gcc/lto/lang.opt
> > index 5bacef349e3..c62dd5aac08 100644
> >
> > --- gcc/lto/lang.opt
> >
> > +++ gcc/lto/lang.opt
> >
> > @@ -127,6 +127,9 @@
> >
> > help
> >
> > LTODump Var(flag_lto_dump_tool_help)
> > Dump the dump tool command line options.
> > +callgraph
> > +LTODump Var(flag_dump_callgraph)
> > +Dump the symtab callgraph.
> > fresolution=
> > LTO Joined
> >
> > =======================================
> >
> > diff --git gcc/lto/lto-dump.c gcc/lto/lto-dump.c
> > index 691d109ff34..bc20120f2d5 100644
> >
> > --- gcc/lto/lto-dump.c
> >
> > +++ gcc/lto/lto-dump.c
> >
> > @@ -197,6 +197,12 @@
> >
> > void dump_list_variables (void)
> >
> > e->dump ();
> > }
> > +/* Dump symbol table in graphviz format. */
> > +void dump_symtab_graphviz (void)
> > +{
> > + symtab->dump_graphviz (stdout);
> > +}
> > +
> > /* Dump symbol list. */
> > void dump_list (void)
> >
> > @@ -251,26 +257,27 @@
> >
> > void dump_body ()
> >
> > /* List of command line options for dumping. */
> > void dump_tool_help ()
> > {
> > - printf ("Usage: lto-dump [OPTION]... SUB_COMMAND [OPTION]...\n\n");
> > - printf ("LTO dump tool command line options.\n\n");
> > - printf (" -list [options] Dump the symbol list.\n");
> > - printf (" -demangle Dump the demangled output.\n");
> > - printf (" -defined-only Dump only the defined symbols.\n");
> > - printf (" -print-value Dump initial values of the "
> > - "variables.\n");
> > - printf (" -name-sort Sort the symbols alphabetically.\n");
> > - printf (" -size-sort Sort the symbols according to size.\n");
> > - printf (" -reverse-sort Dump the symbols in reverse order.\n");
> > - printf (" -symbol= Dump the details of specific symbol.\n");
> > - printf (" -objects Dump the details of LTO objects.\n");
> > - printf (" -type-stats Dump statistics of tree types.\n");
> > - printf (" -tree-stats Dump statistics of trees.\n");
> > - printf (" -gimple-stats Dump statistics of gimple "
> > - "statements.\n");
> > - printf (" -dump-body= Dump the specific gimple body.\n");
> > - printf (" -dump-level= Deciding the optimization level "
> > - "of body.\n");
> > - printf (" -help Display the dump tool help.\n");
> > + const char *msg =
> > + "Usage: lto-dump [OPTION]... SUB_COMMAND [OPTION]...\n\n"
> > + "LTO dump tool command line options.\n\n"
> > + " -list [options] Dump the symbol list.\n"
> > + " -demangle Dump the demangled output.\n"
> > + " -defined-only Dump only the defined symbols.\n"
> > + " -print-value Dump initial values of the variables.\n"
> > + " -name-sort Sort the symbols alphabetically.\n"
> > + " -size-sort Sort the symbols according to size.\n"
> > + " -reverse-sort Dump the symbols in reverse order.\n"
> > + " -symbol= Dump the details of specific symbol.\n"
> > + " -objects Dump the details of LTO objects.\n"
> > + " -callgraph Dump the callgraph in graphviz format.\n"
> > + " -type-stats Dump statistics of tree types.\n"
> > + " -tree-stats Dump statistics of trees.\n"
> > + " -gimple-stats Dump statistics of gimple statements.\n"
> > + " -dump-body= Dump the specific gimple body.\n"
> > + " -dump-level= Deciding the optimization level of body.\n"
> > + " -help Display the dump tool help.\n";
> > +
> > + fputs(msg, stdout);
> > return;
> > }
> >
> > @@ -344,4 +351,9 @@
> >
> > lto_main (void)
> >
> > dump_body ();
> > return;
> > }
> > + else if (flag_dump_callgraph)
> > + {
> > + dump_symtab_graphviz ();
> > + return;
> > + }
> > }
> >
> > =======================================
> >
> > diff --git gcc/symtab.c gcc/symtab.c
> > index 905ca05e578..13882b40ad3 100644
> >
> > --- gcc/symtab.c
> >
> > +++ gcc/symtab.c
> >
> > @@ -955,6 +955,15 @@
> >
> > symtab_node::dump (FILE *f)
> >
> > vnode->dump (f);
> > }
> > +void
> > +symtab_node::dump_graphviz (FILE *f)
> > +{
> > + if (cgraph_node *cnode = dyn_cast <cgraph_node *> (this))
> > + cnode->dump_graphviz (f);
> > + else if (varpool_node *vnode = dyn_cast <varpool_node *> (this))
> > + vnode->dump_graphviz (f);
>
> Then this part can be removed.
>
> > +}
> > +
> > void
> > symbol_table::dump (FILE *f)
> > {
> >
> > @@ -964,6 +973,16 @@
> >
> > symbol_table::dump (FILE *f)
> >
> > node->dump (f);
> > }
> > +void
> > +symbol_table::dump_graphviz (FILE *f)
> > +{
> > + symtab_node *node;
> > + fprintf (f, "digraph symtab {\n");
> > + FOR_EACH_SYMBOL (node)
> > + node->dump_graphviz (f);
> > + fprintf (f, "}\n");
> > +}
> > +
> > DEBUG_FUNCTION void
> > symbol_table::debug (void)
> > {
> >
> > =======================================
> >
> > diff --git gcc/varpool.c gcc/varpool.c
> > index 8e5a9372656..645236293f5 100644
> >
> > --- gcc/varpool.c
> >
> > +++ gcc/varpool.c
> >
> > @@ -237,6 +237,12 @@
> >
> > varpool_node::dump (FILE *f)
> >
> > fprintf (f, "\n");
> > }
> > +/* Dump given varpool node to F in graphviz format. */
> > +void
> > +varpool_node::dump_graphviz (FILE *f)
> > +{
> > + return;
> > +}
>
> Likewise here.
>
> Martin
>
> > /* Dump given varpool node to stderr. */
> > void varpool_node::debug (void)
> >
>
