On 10/4/18 3:37 PM, Hrishikesh Kulkarni wrote:
> Hi,
> 
> Please find the patch for LTO dump tool attached herewith.
> 
> Regards,
> 
> Hrishikesh
> 

Hello.

Thank you for working on that as GSoC student and I hope we can get the patch
into GCC 9.1 release. However I have first bunch of comments what would be
nice to address:

- please rebase the patch as there are some changes in dumpfile.c that conflict
with your patch
- you smashed quite some whitespaces, particularly you replaced '\t' with '  ' 
at places like this:

@@ -2438,7 +214,7 @@ stream_out (char *temp_filename, lto_symtab_encoder_t 
encoder,
       int i;
       do_stream_out (temp_filename, encoder, part);
       for (i = 0; i < nruns; i++)
-       wait_for_child ();
+  wait_for_child ();
     }
   asm_nodes_output = true;
 #else

and very many other places

- you came up with gcc/lto/lto-common.c - it would be more readable if you do 
no-op patch
that will do the refactoring and then second part will implement the lto-dump 
functionality
on top of it

- I see some compilation errors:

g++ -fno-PIE -c   -g -O2 -DIN_GCC     -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  
-DHAVE_CONFIG_H -I. -Ilto -I/home/marxin/Programming/gcc/gcc 
-I/home/marxin/Programming/gcc/gcc/lto 
-I/home/marxin/Programming/gcc/gcc/../include 
-I/home/marxin/Programming/gcc/gcc/../libcpp/include  
-I/home/marxin/Programming/gcc/gcc/../libdecnumber 
-I/home/marxin/Programming/gcc/gcc/../libdecnumber/bid -I../libdecnumber 
-I/home/marxin/Programming/gcc/gcc/../libbacktrace   -o lto/lto-dump.o -MT 
lto/lto-dump.o -MMD -MP -MF lto/.deps/lto-dump.TPo 
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c: In member function ‘virtual 
void symbol_entry::dump()’:
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c:64:13: warning: '0' flag 
ignored with precision and ‘%u’ gnu_printf format [-Wformat=]
     printf ("%s  %s  %0.4zu  %s  ", type_name, visibility, sz, name);
             ^~~~~~~~~~~~~~~~~~~~~~
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c: In function ‘int 
size_compare(const void*, const void*)’:
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c:123:41: warning: cast from type 
‘const void*’ to type ‘symbol_entry**’ casts away qualifiers [-Wcast-qual]
   symbol_entry *e1 = *(symbol_entry **) a;
                                         ^
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c:124:41: warning: cast from type 
‘const void*’ to type ‘symbol_entry**’ casts away qualifiers [-Wcast-qual]
   symbol_entry *e2 = *(symbol_entry **) b;
                                         ^
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c: In function ‘int 
name_compare(const void*, const void*)’:
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c:133:41: warning: cast from type 
‘const void*’ to type ‘symbol_entry**’ casts away qualifiers [-Wcast-qual]
   symbol_entry *e1 = *(symbol_entry **) a;
                                         ^
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c:134:41: warning: cast from type 
‘const void*’ to type ‘symbol_entry**’ casts away qualifiers [-Wcast-qual]
   symbol_entry *e2 = *(symbol_entry **) b;
                                         ^
In file included from /home/marxin/Programming/gcc/gcc/lto/lto-dump.c:32:
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c: In function ‘void 
dump_symbol()’:
/home/marxin/Programming/gcc/gcc/cgraph.h:2665:4: warning: this ‘for’ clause 
does not guard... [-Wmisleading-indentation]
    for ((node) = symtab->first_symbol (); (node); (node) = (node)->next)
    ^~~
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c:215:3: note: in expansion of 
macro ‘FOR_EACH_SYMBOL’
   FOR_EACH_SYMBOL (node)
   ^~~~~~~~~~~~~~~
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c:222:5: note: ...this statement, 
but the latter is misleadingly indented as if it were guarded by the ‘for’
     if (!flag)
     ^~
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c: In function ‘void lto_main()’:
/home/marxin/Programming/gcc/gcc/lto/lto-dump.c:296:13: error: cannot convert 
‘bool’ to ‘timer*’ in assignment
   g_timer = false;
             ^~~~~

Thanks,
Martin

Reply via email to