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/Makefile.in b/gcc/Makefile.in
index b871640..f6de933 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.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,
diff --git a/gcc/doc/gcc.texi b/gcc/doc/gcc.texi
index da1c1ab..d03e4b3 100644
--- a/gcc/doc/gcc.texi
+++ b/gcc/doc/gcc.texi
@@ -68,7 +68,7 @@ Texts being (a) (see below), and with the Back-Cover Texts being (b)
 * gcov: (gcc) Gcov.            @command{gcov}---a test coverage program.
 * gcov-tool: (gcc) Gcov-tool.  @command{gcov-tool}---an offline gcda profile processing program.
 * gcov-dump: (gcc) Gcov-dump.  @command{gcov-dump}---an offline gcda and gcno profile dump tool.
-* lto-dump-tool: (gcc) lto-dump-tool. @command{lto-dump-tool}---Tool for
+* lto-dump: (gcc) lto-dump. @command{lto-dump}---Tool for
 dumping LTO object files.
 @end direntry
 This file documents the use of the GNU compilers.
@@ -144,7 +144,7 @@ Introduction, gccint, GNU Compiler Collection (GCC) Internals}.
 * Gcov::            @command{gcov}---a test coverage program.
 * Gcov-tool::       @command{gcov-tool}---an offline gcda profile processing program.
 * Gcov-dump::       @command{gcov-dump}---an offline gcda and gcno profile dump tool.
-* lto-dump-tool::   @command{lto-dump-tool}---Tool for dumping LTO
+* lto-dump::   @command{lto-dump}---Tool for dumping LTO
 object files.
 * Trouble::         If you have trouble using GCC.
 * Bugs::            How, why and where to report bugs.
diff --git a/gcc/doc/lto-dump.texi b/gcc/doc/lto-dump.texi
index b66019d..062b795 100644
--- a/gcc/doc/lto-dump.texi
+++ b/gcc/doc/lto-dump.texi
@@ -25,39 +25,39 @@ included in the gfdl(7) man page.
      funds for GNU development.
 @c man end
 @c Set file name and title for the man page.
-@setfilename lto-dump-tool
+@setfilename lto-dump
 @settitle Tool for dumping LTO object files. 
 @end ignore
 
-@node lto-dump-tool
-@chapter @command{lto-dump-tool}---Tool for dumping LTO object files.
+@node lto-dump
+@chapter @command{lto-dump}---Tool for dumping LTO object files.
 
 @menu
-* lto-dump-tool Intro::             Introduction to lto-dump-tool.
-* Invoking lto-dump-tool::          How to use lto-dump-tool.
+* lto-dump Intro::             Introduction to lto-dump.
+* Invoking lto-dump::          How to use lto-dump.
 @end menu
 
-@node lto-dump-tool Intro
-@section Introduction to @command{lto-dump-tool}
+@node lto-dump Intro
+@section Introduction to @command{lto-dump}
 @c man begin DESCRIPTION
 
-@command{lto-dump-tool} is a tool you can use in conjunction with GCC to
+@command{lto-dump} is a tool you can use in conjunction with GCC to
 dump link time optimization object files.
 
 @c man end
 
-@node Invoking lto-dump-tool
-@section Invoking @command{lto-dump-tool}
+@node Invoking lto-dump
+@section Invoking @command{lto-dump}
 
 @smallexample
-Usage: lto-dump-tool @r{[}@var{OPTION}@r{]} ... @var{objfiles}
+Usage: lto-dump @r{[}@var{OPTION}@r{]} ... @var{objfiles}
 @end smallexample
 
-@command{lto-dump-tool} accepts the following options:
+@command{lto-dump} accepts the following options:
 
 @ignore
 @c man begin SYNOPSIS
-lto-dump-tool [@option{-list}]
+lto-dump [@option{-list}]
      [@option{-demangle}]
      [@option{-defined-only}]
      [@option{-print-value}]
@@ -66,13 +66,13 @@ lto-dump-tool [@option{-list}]
      [@option{-reverse-sort}]
      [@option{-no-sort}]
      [@option{-symbol=}]
-     [@option{-objects=}]
+     [@option{-objects}]
      [@option{-type-stats}]
      [@option{-tree-stats}]
      [@option{-gimple-stats}]
      [@option{-dump-level=}]
      [@option{-dump-body=}]
-     [@option{-help}] @var{lto-dump-tool}
+     [@option{-help}] @var{lto-dump}
 @c man end
 @end ignore
 
@@ -105,7 +105,7 @@ Dump the symbols in order of occurence.
 @item -symbol=
 Dump the details of specific symbol.
 
-@item -objects=
+@item -objects
 Dump the details of LTO objects.
 
 @item -type-stats
diff --git a/gcc/lto/Make-lang.in b/gcc/lto/Make-lang.in
index e474f85..b8049ba 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:
diff --git a/gcc/lto/lang.opt b/gcc/lto/lang.opt
index 931f6b5..9103810 100644
--- a/gcc/lto/lang.opt
+++ b/gcc/lto/lang.opt
@@ -94,10 +94,6 @@ reverse-sort
 LTO Var(flag_lto_reverse_sort)
 Display the symbols in reverse order.
 
-no-sort
-LTO Var(flag_lto_no_sort)
-Display the symbols in order of occurence.
-
 symbol=
 LTO Driver RejectNegative Joined Var(flag_lto_dump_symbol)
 
diff --git a/gcc/lto/lto-common.c b/gcc/lto/lto-common.c
index f664faa..10356e4 100644
--- a/gcc/lto/lto-common.c
+++ b/gcc/lto/lto-common.c
@@ -1827,14 +1827,14 @@ lto_read_decls (struct lto_file_decl_data *decl_data, const void *data,
 
     if (flag_lto_dump_type_stats)
     {
-      fprintf (stdout, "\n\n     Type     Frequency   Percentage\n\n");
+      fprintf (stdout, "       Type     Frequency   Percentage\n\n");
       for (hash_map<code_id_hash, unsigned>::iterator itr = hm.begin ();
 	   itr != hm.end ();
 	   ++itr)
       {
 	std::pair<unsigned, unsigned> p = *itr;
 	enum tree_code code = (enum tree_code) p.first;
-	fprintf (stdout, "%14s %6d %10.2f%\n", get_tree_code_name (code),
+	fprintf (stdout, "%14s %6d %12.2f%\n", get_tree_code_name (code),
 		 p.second, float (p.second)/total*100);
       }
     }
diff --git a/gcc/lto/lto-dump.c b/gcc/lto/lto-dump.c
index 5930b14..7b2717c 100644
--- a/gcc/lto/lto-dump.c
+++ b/gcc/lto/lto-dump.c
@@ -152,16 +152,17 @@ void dump_list_functions (void)
       v.safe_push (e);
   }
 
-  if (!flag_lto_no_sort)
-  {
-    if (flag_lto_size_sort)
-      v.qsort (size_compare);
-    else if (flag_lto_name_sort)
-      v.qsort (name_compare);
-  }
+  if (flag_lto_size_sort)
+    v.qsort (size_compare);
+  else if (flag_lto_name_sort)
+    v.qsort (name_compare);
   if (flag_lto_reverse_sort)
     v.reverse ();
 
+  printf ("Type   Visibility  Size  Name");
+  if (flag_lto_print_value)
+    printf("  Value");
+  printf("\n");
   int i=0;
   symbol_entry* e;
   FOR_EACH_VEC_ELT (v, i, e)
@@ -182,15 +183,10 @@ void dump_list_variables (void)
       v.safe_push (e);
   }
 
-  if (!flag_lto_no_sort)
-  {
-    if (flag_lto_size_sort)
-      v.qsort (size_compare);
-    else if (flag_lto_name_sort)
-      v.qsort (name_compare);
-  }
-
-
+  if (flag_lto_size_sort)
+    v.qsort (size_compare);
+  else if (flag_lto_name_sort)
+    v.qsort (name_compare);
   if (flag_lto_reverse_sort)
     v.reverse ();
 
@@ -213,49 +209,57 @@ void dump_list (void)
 /* Dump specific variables and functions used in IL.  */
 void dump_symbol ()
 {
+  int flag = 0;
   symtab_node *node;
   printf ("Symbol: %s\n", flag_lto_dump_symbol);
   FOR_EACH_SYMBOL (node)
     if (!strcmp (flag_lto_dump_symbol, node->name ()))
+    {
       node->debug ();
-  printf ("\n");
+      printf ("\n");
+      flag = 1;
+    }
+    if (!flag)
+      error_at (input_location, "Symbol not found.");
   return;
 }
 
 /* Dump specific gimple body of specified function.  */
 void dump_body ()
 {
+  int flag = 0;
   dump_flags_t flags;
   flags = (flag_dump_level)
 	 ? parse_dump_option (flag_dump_level, 0, 0)
 	 : TDF_NONE;
   cgraph_node *cnode;
   FOR_EACH_FUNCTION (cnode)
+  if (cnode->definition && !strcmp (cnode->name (), flag_dump_body))
   {
-    if (cnode->definition && !strcmp (cnode->name (), flag_dump_body))
-    {
-      printf ("Gimple Body of Function: %s\n", cnode->name ());
-      cnode->get_untransformed_body ();
-      debug_function (cnode->decl, flags);
-    }
+    printf ("Gimple Body of Function: %s\n", cnode->name ());
+    cnode->get_untransformed_body ();
+    debug_function (cnode->decl, flags);
+    flag = 1;
   }
-    return;
+  if (!flag)
+    error_at (input_location, "Function not found.");
+  return;
 }
 
 /* List of command line options for dumping.  */
 void dump_tool_help ()
 {
   printf ("\nLTO dump tool command line options.\n\n");
-  printf ("-list : Dump the symbol list.\n");
-  printf ("    -demangle : Dump the demangled output.\n");
-  printf ("    -defined-only : Dump only the defined symbols.\n");
-  printf ("    -print-value : Dump initial values of the variables.\n");
-  printf ("    -name-sort : Sort the symbols alphabetically.\n");
-  printf ("    -size-sort : Sort the symbols according to size.\n");
-  printf ("    -reverse-sort : Dump the symbols in reverse order.\n");
-  printf ("    -no-sort : Dump the symbols in order of occurence.\n");
+  printf("Usage: lto-dump [OPTION]... SUB_COMMAND [OPTION]...\n\n");
+  printf ("-list [options] : Dump the symbol list.\n");
+  printf ("  -demangle : Dump the demangled output.\n");
+  printf ("  -defined-only : Dump only the defined symbols.\n");
+  printf ("  -print-value : Dump initial values of the variables.\n");
+  printf ("  -name-sort : Sort the symbols alphabetically.\n");
+  printf ("  -size-sort : Sort the symbols according to size.\n");
+  printf ("  -reverse-sort : Dump the symbols in reverse order.\n");
   printf ("-symbol= : Dump the details of specific symbol.\n");
-  printf ("-objects= : Dump the details of LTO objects.\n");
+  printf ("-objects : Dump the details of LTO objects.\n");
   printf ("-type-stats : Dump statistics of tree types.\n");
   printf ("-tree-stats : Dump statistics of trees.\n");
   printf ("-gimple-stats : Dump statistics of gimple statements.\n");
@@ -308,7 +312,7 @@ lto_main (void)
   {
     cgraph_node *node;
     FOR_EACH_DEFINED_FUNCTION (node)
-    node->get_untransformed_body ();
+      node->get_untransformed_body ();
     if (!GATHER_STATISTICS)
       warning_at (input_location, 0, "Not configured with --enable-gather-detailed-mem-stats.");
     else
@@ -319,8 +323,13 @@ lto_main (void)
   /* Dump tree statistics.  */
   if (flag_lto_tree_stats)
   {
-    printf ("Tree Statistics\n");
-    dump_tree_statistics ();
+    if (!GATHER_STATISTICS)
+      warning_at (input_location, 0, "Not configured with --enable-gather-detailed-mem-stats.");
+    else
+    {
+      printf ("Tree Statistics\n");
+      dump_tree_statistics ();
+    }
     return;
   }
 

Reply via email to