Hi,
Thanks for inputs and feedback. As per feedback, I have made all the
corrections/changes to the best of my understanding. Please find
details of the same given below.:
1) options of lto-dump:
a) removed the no-demangle command line option as it is default.
b) exit added after every dump. (to make it execute separately)
c) Made necessary change to consider single occurrence of object file
even if there are multiple occurrences.
d) -help option added.
e) alpha-sort renamed as name-sort,
flag_lto_alpha_sort renamed as flag_lto_name_sort,
alpha_compare() renamed as name_compare().
2) output formatting:
a) All '\t' replaced with (%10d) format.
b) Made necessary changes to remove extra output.
c) All list headers unified in a way that first letter is capital.
d) Made changes so that all used trees are printed. For gimple stats I
tried configuring with --enable-gather-detailed-mem-stats which worked
for me.
e) TDF_NONE made default level, -dump-body= can be invoked without -dump-level=
f)Renamed the command line options as -dump-body=, -dump-level=
3) written source code:
a) Deleted the header "gt-lto-lto.h" from lto.c and lto-dump.c and
added to lto-common.c
b) Modified parse_dump_option() and used in dumpfile.c.
c) Unnecessary code from lto_main() in lto-dump.c deleted.
e) Shifted all the functionalities used exclusively in lto.c from
lto-common.c to lto.c.
f) Used stdout instead of stderr.
g) Added/rewritten comments for all functions and structs and also for
lto_main().
4) misc:
a) Tried to modify make file for 'make install' of lto-dump. (diff
file attached)
b) Documented the lto-dump-tool command line options in
lto-dump-tool.texi could not link it in makefile. (diff file attached)
d) Removed all extra headers from lto-dump.c, added year of creation
and file comments.
I have pushed all the changes to the branch lto-dump-tool-v4.
I am facing some issue while modifying the Makefile for the points 4.a
and 4.b. Please find the diff file for the same attached herewith.
Thanks and Regards,
Hrishikesh
On Wed, Aug 1, 2018 at 6:59 PM, Martin Liška <mli...@suse.cz> wrote:
> 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
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index b871640..4f38261 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3161,7 +3161,7 @@ TEXI_GCC_FILES = gcc.texi gcc-common.texi gcc-vers.texi frontends.texi \
gcov.texi trouble.texi bugreport.texi service.texi \
contribute.texi compat.texi funding.texi gnu.texi gpl_v3.texi \
fdl.texi contrib.texi cppenv.texi cppopts.texi avr-mmcu.texi \
- implement-c.texi implement-cxx.texi gcov-tool.texi gcov-dump.texi
+ implement-c.texi implement-cxx.texi gcov-tool.texi gcov-dump.texi lto-dump-tool.texi
# we explicitly use $(srcdir)/doc/tm.texi here to avoid confusion with
# the generated tm.texi; the latter might have a more recent timestamp,
@@ -3600,6 +3600,15 @@ install-common: native lang.install-common installdirs
gcov-tool$(exeext) $(DESTDIR)$(bindir)/$(GCOV_TOOL_INSTALL_NAME)$(exeext); \
fi; \
fi
+# Install lto-dump-tool if it was compiled.
+ -if test "$(enable_as_accelerator)" != "yes" ; then \
+ if [ -f lto-dump-tool$(exeext) ]; \
+ then \
+ rm -f $(DESTDIR)$(bindir)/$(LTO_DUMP_TOOL_INSTALL_NAME)$(exeext); \
+ $(INSTALL_PROGRAM) \
+ lto-dump-tool$(exeext) $(DESTDIR)$(bindir)/$(LTO_DUMP_TOOL_INSTALL_NAME)$(exeext); \
+ fi; \
+ fi
# Install gcov-dump if it was compiled.
-if test "$(enable_as_accelerator)" != "yes" ; then \
if [ -f gcov-dump$(exeext) ]; \
