On 7/1/19 7:32 PM, Giuliano Belinassi wrote:
> 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.
This one is a false positive.
>
>> === 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.
Likewise here.
>
>>
>> === 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.
I agree with yout.
>
> Any other style error suggested by the script were fixed.
Thanks,
Martin
>
> 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)
>>>
>>
