Hi.

After last update of the branch, there's a feedback that will be needed
before we can adept to have it merged into trunk:

1) there's patch for lto-dump proper install:

diff --git a/gcc/lto/Make-lang.in b/gcc/lto/Make-lang.in
index e474f85ebc6..e9d2659025c 100644
--- a/gcc/lto/Make-lang.in
+++ b/gcc/lto/Make-lang.in
@@ -46,7 +46,10 @@ lto.all.cross: $(LTO_EXE) $(LTO_DUMP_EXE)
 lto.start.encap: $(LTO_EXE) $(LTO_DUMP_EXE)
 lto.rest.encap:
 lto.tags:
-lto.install-common:
+lto.install-common: installdirs
+       $(INSTALL_PROGRAM) $(LTO_DUMP_EXE) \
+         $(DESTDIR)/$(bindir)/$(LTO_DUMP_EXE)
+
 lto.install-man:
 lto.install-info:
 lto.dvi:

2) If I build bzip2 (just add -flto into Makefile) with your branch I see an 
ICE:

$ cd bzip2-1.0.6
$ make
[...]
gcc -Wall -Winline -O2 -g -D_FILE_OFFSET_BITS=64 -flto=9   -o bzip2 bzip2.o -L. 
-lbz2
lto1: internal compiler error: unexpected offset
0x767bfc lto_resolution_read
        ../../gcc/lto/lto-common.c:1951
0x767bfc lto_file_read
        ../../gcc/lto/lto-common.c:2169
0x767bfc read_cgraph_and_symbols(unsigned int, char const**)
        ../../gcc/lto/lto-common.c:2631
0x74f042 lto_main()
        ../../gcc/lto/lto.c:580

3) I still see an extra info that is not needed:

Time variable                                   usr           sys          wall 
              GGC
 phase setup                        :   0.00 (  0%)   0.00 (  0%)   0.00 (  0%) 
   2029 kB ( 52%)
 phase parsing                      :   0.01 (100%)   0.00 (  0%)   0.02 (100%) 
   1887 kB ( 48%)
 ipa cp                             :   0.00 (  0%)   0.00 (  0%)   0.01 ( 50%) 
    119 kB (  3%)
 ipa lto gimple in                  :   0.00 (  0%)   0.00 (  0%)   0.00 (  0%) 
   1136 kB ( 29%)
 tree operand scan                  :   0.01 (100%)   0.00 (  0%)   0.01 ( 50%) 
    158 kB (  4%)
 TOTAL                              :   0.01          0.00          0.02        
   3917 kB
Extra diagnostic checks enabled; compiler may run slowly.
Configure with --enable-checking=release to disable checks.

4) -list:
 a) please add header, it was useful
 b) -no-sort makes no sense to me, it's default isn't it?

5) -symbol=
 a) if symbol is not found, we should print an error
6) -objects= - why '=' ?
7) -type-stats - you have extra empty lines at the beginning
   - please align header with values
8) -tree-stats - probably also --enable-gather-.. is needed?
9) -dump-body - again, if does not exist, error should be printed
10) -dump-level - again error if wrong value, document possible values if 
possible
11) please fix formatting of lto-dump so that it explains which suboptions are 
possible:

$ gcov-tool --help
Usage: gcov-tool [OPTION]... SUB_COMMAND [OPTION]...

Offline tool to handle gcda counts

  -h, --help                            Print this help, then exit
  -v, --version                         Print version number, then exit
  merge [options] <dir1> <dir2>         Merge coverage file contents
    -o, --output <dir>                  Output directory
    -v, --verbose                       Verbose mode
    -w, --weight <w1,w2>                Set weights (float point values)
  rewrite [options] <dir>               Rewrite coverage file contents
    -n, --normalize <int64_t>           Normalize the profile
    -o, --output <dir>                  Output directory
    -s, --scale <float or simple-frac>  Scale the profile counters
    -v, --verbose                       Verbose mode
  overlap [options] <dir1> <dir2>       Compute the overlap of two profiles
    -f, --function                      Print function level info
    -F, --fullname                      Print full filename
    -h, --hotonly                       Only print info for hot 
objects/functions
    -o, --object                        Print object level info
    -t <float>, --hot_threshold <float> Set the threshold for hotness
    -v, --verbose                       Verbose mode

12) Do not name it in documentation 'lto-dump-tool', use simply 'lto-dump'.
13) make sure man page is generated for lto-dump
14) I noticed that you smashed some whitespaces for files where you didn't do 
any
changes: gcc/tree.c, lto.c(do_whole_program_analysis) and other places
15) test the tool on a bigger program built with LTO, ideally for both LGEN and 
also
LTRANS files
16) The last bigger challenge is the option handling, we currently participate 
in lto/lang.opt.
Ideally I would prefer using standard mechanism getopt_long, take a look at 
gcov-dump.c.
Can you please investigate that.

Anyway, thank you for working on that. I hope we can send a patch submission 
into GCC's trunk soon.
Martin

Reply via email to