Hi,
I have made changes for feedback points 3 and 10.
With this I have completed/ made changes for points
1, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12.
I have pushed the changes to the repo. Please find the diff file
attached herewith.
Regards,
Hrishikesh
On Thu, Aug 16, 2018 at 7:50 PM Hrishikesh Kulkarni
<hrishikeshpa...@gmail.com> wrote:
>
> Hi,
>
> Thanks for the feedback.
> I have made the corrections for the feedback points:
> 1, 4, 5, 6, 7, 8, 9, 11, 12.
> I am working on the remaining points.
>
> I have pushed the changes to the repo (lto-dump-tool-v4 branch) and
> attached the diff file herewith.
>
> Regards,
>
> Hrishikesh
>
> On Wed, Aug 15, 2018 at 5:30 PM, Martin Liška <mli...@suse.cz> wrote:
> > 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
diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 0c25e71..95dccda 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -1346,9 +1346,12 @@ parse_dump_option (const char *option_value, const char **pos_p,
*pos_p = ptr + 1;
break;
}
- else if (swtch)
- warning (0, "ignoring unknown option %q.*s in %<-fdump-%s%>",
- length, ptr, swtch);
+ else
+ {
+ warning (0, "ignoring unknown option %q.*s",
+ length, ptr);
+ flags = TDF_ERROR;
+ }
found:
ptr = end_ptr;
}
diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
index 9d7b049..03cec10 100644
--- a/gcc/dumpfile.h
+++ b/gcc/dumpfile.h
@@ -146,7 +146,9 @@ enum dump_flag
| MSG_NOTE),
/* Dumping for -fcompare-debug. */
- TDF_COMPARE_DEBUG = (1 << 25)
+ TDF_COMPARE_DEBUG = (1 << 25),
+
+ TDF_ERROR = (1<<26)
};
/* Dump flags type. */
diff --git a/gcc/lto/lto-dump.c b/gcc/lto/lto-dump.c
index 7b2717c..b1854c2 100644
--- a/gcc/lto/lto-dump.c
+++ b/gcc/lto/lto-dump.c
@@ -232,6 +232,11 @@ void dump_body ()
flags = (flag_dump_level)
? parse_dump_option (flag_dump_level, 0, 0)
: TDF_NONE;
+ if (flags == TDF_ERROR)
+ {
+ error_at (input_location, "Level not found, use none, slim, blocks, vops.");
+ return;
+ }
cgraph_node *cnode;
FOR_EACH_FUNCTION (cnode)
if (cnode->definition && !strcmp (cnode->name (), flag_dump_body))
@@ -288,7 +293,7 @@ lto_main (void)
/* Initialize the LTO front end. */
lto_fe_init ();
-
+ g_timer = false;
/* Read all the symbols and call graph from all the files in the
command line. */
read_cgraph_and_symbols (num_in_fnames, in_fnames);
