[GSOC] LTO dump tool project - feedback

[Martin Liška <mliska at suse dot cz>]

Hi.

I decided to come up with a new sub-thread that will be linked
to a root of email discussion. First, thank you for working on
the project and there's a feedback that I can provide now:

1) options of lto-dump:
   a) no-demangle - does not make sense because it's default
   b) you should exit after one command, you probably don't want to support:
       -list bzlib.o -tree-stats
   c) following should be maybe handled:
      $ lto-dump -list bzlib.o bzlib.o

bzlib.c:468:5: note: previously defined here
bzlib.c:407:5: error: ‘BZ2_bzCompress’ has already been defined
 int BZ_API(BZ2_bzCompress) ( bz_stream *strm, int action )
     ^
bzlib.c:407:5: note: previously defined here
bzlib.c:148:5: error: ‘BZ2_bzCompressInit’ has already been defined
 int BZ_API(BZ2_bzCompressInit)

   d) you should add --help option, take a look at gcov-dump --help, or gcov --help,
      or gcov-tool --help.
   e) alpha-sort - maybe name-sort, or alphabetic-sort?

2) output formatting
   a) you mix usage of tabulars and fixed columns (%10d), please use the later
      at all places
   b) I see a lot of extra output when using the tool:
...
Reading object files: bzlib.o {GC start 2164k} 
Reading the callgraph
Merging declarations
Reading summaries
Reading function bodies:
Performing interprocedural optimizations
 <whole-program> <profile_estimate> <icf> in:BZ2_bzCompressEnd in:BZ2_bzDecompressEnd <devirt> <cp> <cdtor> <fnsummary> <inline> <pure-const> <free-fnsummary> <static-var> <single-use> <comdats>Assembling functions:
 <materialize-all-clones> <simdclone> add_pair_to_block in:add_pair_to_block default_bzfree in:default_bzfree default_bzalloc in:default_bzalloc handle_compress.isra.2 in:handle_compress.isra.2 in:copy_output_until_stop in:copy_input_until_stop BZ2_bz__AssertH__fail in:BZ2_bz__AssertH__fail BZ2_bzCompressInit in:BZ2_bzCompressInit BZ2_bzCompress in:BZ2_bzCompress BZ2_bzCompressEnd BZ2_bzDecompressInit in:BZ2_bzDecompressInit BZ2_indexIntoF in:BZ2_indexIntoF BZ2_bzDecompress in:BZ2_bzDecompress in:unRLE_obuf_to_output_SMALL in:unRLE_obuf_to_output_FAST {GC 5331k -> 2747k} BZ2_bzDecompressEnd BZ2_bzWriteOpen in:BZ2_bzWriteOpen in:BZ2_bzWriteOpen.part.3 BZ2_bzWrite in:BZ2_bzWrite in:BZ2_bzWrite.part.4 BZ2_bzWriteClose64 in:BZ2_bzWriteClose64 in:BZ2_bzWriteClose64.part.5 BZ2_bzWriteClose in:BZ2_bzWriteClose BZ2_bzReadOpen in:BZ2_bzReadOpen in:BZ2_bzReadOpen.part.6 {GC 5332k -> 2307k} bzopen_or_bzdopen in:bzopen_or_bzdopen BZ2_bzReadClose in:BZ2_bzReadClose in:BZ2_bzReadClose.part.7 BZ2_bzRead in:BZ2_bzRead in:BZ2_bzRead.part.8 in:myfeof BZ2_bzReadGetUnused in:BZ2_bzReadGetUnused in:BZ2_bzReadGetUnused.part.9 BZ2_bzBuffToBuffCompress in:BZ2_bzBuffToBuffCompress BZ2_bzBuffToBuffDecompress in:BZ2_bzBuffToBuffDecompress BZ2_bzlibVersion in:BZ2_bzlibVersion BZ2_bzopen in:BZ2_bzopen BZ2_bzdopen in:BZ2_bzdopen BZ2_bzread in:BZ2_bzread in:BZ2_bzread.part.10 BZ2_bzwrite in:BZ2_bzwrite BZ2_bzflush in:BZ2_bzflush BZ2_bzclose in:BZ2_bzclose BZ2_bzerror in:BZ2_bzerror in:bzerrorstrings
...

please remove it
   c) you mix all upper letters in header of lists, please unify that (Visibility, SECTION NAME)
   d) -gimple-stats does not work for me, no output and -tree-stats definitely not print all trees used
   e) you should not require -fdump-level for -fdump-body, select a default value for the level please
   f) why -fdump-level and -fdump-body start with '-f'

3) written source code
   a) I need following patch to build it:
diff --git a/gcc/lto/lto-dump.c b/gcc/lto/lto-dump.c
index 9a1ff61852c..48fc7d9a181 100644
--- a/gcc/lto/lto-dump.c
+++ b/gcc/lto/lto-dump.c
@@ -370,4 +370,4 @@ lto_main (void)
   timevar_push (TV_PARSE_GLOBAL);
 }
 
-#include "gt-lto-lto.h"
+//#include "gt-lto-lto.h"
diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
index bc61d25f61d..301ed4b2e25 100644
--- a/gcc/lto/lto.c
+++ b/gcc/lto/lto.c
@@ -143,4 +143,4 @@ lto_main (void)
   timevar_push (TV_PARSE_GLOBAL);
 }
 
-#include "gt-lto-lto.h"
+//#include "gt-lto-lto.h"

can you please check it?

   b) you added parse_dump_option, but it's used only in dump-lto.c, why not in dumpfile.c?
   c) lto-dump.c still contains a lot of code in lto_main that's not needed (and is responsible
      for extra output)
   e) you moved all to lto-common.c, but functionality exclusively used only in lto.c should remain there
   f) you should probably dump to stdout, rather that stderr
   g) there are still quite some functions in lto-dump.c that are missing comment, lto_main comment should
      be rewritten
4) misc
   a) lto-dump should be installed with 'make install', similarly as lto1 is
   b) lto-dump command option should be documentation, again take a look for gcov-dump, gcov, gcov-tool (*.texi file)
   c) would be nice to have some test-cases for that, but it's probably out of scope of the project
   d) I bet there will be quite some extra includes of header file in lto-dump.c, please adjust file comment and
      year of creation

Martin