Re: [PATCH][1/n] Cleanup internal interfaces, GCC modularization

2012-03-30 Thread Richard Guenther
On Thu, 29 Mar 2012, Jan Hubicka wrote:

  
  I am playing with doing some internal interface static analysis
  using the first patch below (and looking at LTO bootstrap results).
  
  An example, obvious patch resulting from that is the 2nd patch,
  resuling from the static analysis output
  
  /space/rguenther/src/svn/trunk/gcc/tree-ssa-pre.c:add_to_value can be made 
  static
  /space/rguenther/src/svn/trunk/gcc/tree-ssa-pre.c:print_value_expressions 
  can be made static
  
  the static analysis is very verbose (and does not consider ipa-refs yet).
  You also need to union results for building all frontends and all
  targets (well, in theory, or you can simply manually verify things
  which is a good idea anyway - even unused functions may be useful
  exported when they implement a generic data structure for example).
  
  Excercise for the reader: turn the analysis into a plugin.
  
  Richard.
  
  
  Index: gcc/lto/lto.c
  ===
  --- gcc/lto/lto.c   (revision 185918)
  +++ gcc/lto/lto.c   (working copy)
  @@ -2721,6 +2721,65 @@ read_cgraph_and_symbols (unsigned nfiles
 lto_symtab_merge_cgraph_nodes ();
 ggc_collect ();
   
  +  if (flag_wpa)
  +{
  +  struct cgraph_node *node;
  +  FILE *f = fopen (concat (dump_base_name, .callers, NULL), w);
  +  for (node = cgraph_nodes; node; node = node-next)
  +   {
  + tree caller_tu = NULL_TREE;
  + struct cgraph_edge *caller;
  + bool found = true;
  +
  + if (!TREE_PUBLIC (node-decl)
  + || !TREE_STATIC (node-decl)
  + || resolution_used_from_other_file_p (node-resolution))
  +   continue;
  +
  + if (!node-callers)
  +   {
  + expanded_location loc = expand_location (DECL_SOURCE_LOCATION 
  (node-decl));
  + fprintf (f, %s:%s no calls\n,
  +  loc.file, IDENTIFIER_POINTER (DECL_NAME (node-decl)));
  +   }
 
 With Mozilla folks I used the dumps from first WPA unreachable function 
 removal pass
 with some degree of success.  This gets a lot of non-trivial cases of dead 
 code,
 but also there are a lot of funny false positives wrt comdats etc.
  + for (caller = node-callers; caller; caller = caller-next_caller)
  +   {
  + if (!caller_tu)
  +   caller_tu = DECL_CONTEXT (caller-caller-decl);
  + else if (caller_tu
  +   DECL_CONTEXT (caller-caller-decl) != caller_tu)
  +   found = false;
  +   }
 Extending to IPA-REF should be straighforward.
  + if (found  caller_tu)
  +   {
  + expanded_location loc1 = expand_location (DECL_SOURCE_LOCATION 
  (node-decl));
  + expanded_location loc2 = expand_location (DECL_SOURCE_LOCATION 
  (node-callers-caller-decl));
  +
  + if (DECL_CONTEXT (node-decl) == caller_tu)
  +   fprintf (f, %s:%s can be made static\n,
  +loc1.file, IDENTIFIER_POINTER (DECL_NAME 
  (node-decl)));
 Indeed, this is also useful. Any plans to turn this into general 
 -Wsomething,
 or you will also stay just with an internal hack like I did? :)

;)  The result has way too many false positives (LTO bootstrap produces
quite some dead functions due to early inlining).  So yes, this will
stay internal ;)

Richard.


Re: [PATCH][1/n] Cleanup internal interfaces, GCC modularization

2012-03-30 Thread Richard Guenther
On Fri, 30 Mar 2012, Richard Guenther wrote:

 On Thu, 29 Mar 2012, Jan Hubicka wrote:
 
  With Mozilla folks I used the dumps from first WPA unreachable function 
  removal pass
  with some degree of success.  This gets a lot of non-trivial cases of dead 
  code,
  but also there are a lot of funny false positives wrt comdats etc.
   +   for (caller = node-callers; caller; caller = caller-next_caller)
   + {
   +   if (!caller_tu)
   + caller_tu = DECL_CONTEXT (caller-caller-decl);
   +   else if (caller_tu
   + DECL_CONTEXT (caller-caller-decl) != caller_tu)
   + found = false;
   + }
  Extending to IPA-REF should be straighforward.
   +   if (found  caller_tu)
   + {
   +   expanded_location loc1 = expand_location (DECL_SOURCE_LOCATION 
   (node-decl));
   +   expanded_location loc2 = expand_location (DECL_SOURCE_LOCATION 
   (node-callers-caller-decl));
   +
   +   if (DECL_CONTEXT (node-decl) == caller_tu)
   + fprintf (f, %s:%s can be made static\n,
   +  loc1.file, IDENTIFIER_POINTER (DECL_NAME 
   (node-decl)));
  Indeed, this is also useful. Any plans to turn this into general 
  -Wsomething,
  or you will also stay just with an internal hack like I did? :)
 
 ;)  The result has way too many false positives (LTO bootstrap produces
 quite some dead functions due to early inlining).  So yes, this will
 stay internal ;)

Btw, the following is the last incarnation of the patch - I've stopped
here for now.  If you LTO bootstrap with it you can find *.callers
files in the build tree (remember to use -O0 for added precision).

Richard.

Index: gcc/lto/lto.c
===
--- gcc/lto/lto.c   (revision 186007)
+++ gcc/lto/lto.c   (working copy)
@@ -2721,6 +2721,70 @@ read_cgraph_and_symbols (unsigned nfiles
   lto_symtab_merge_cgraph_nodes ();
   ggc_collect ();
 
+  if (flag_wpa)
+{
+  struct cgraph_node *node;
+  FILE *f = fopen (concat (dump_base_name, .callers, NULL), w);
+  for (node = cgraph_nodes; node; node = node-next)
+   {
+ tree caller_tu = NULL_TREE;
+ struct cgraph_edge *caller;
+ bool found = true;
+
+ if (!TREE_PUBLIC (node-decl)
+ || !TREE_STATIC (node-decl)
+ || DECL_PRESERVE_P (node-decl)
+ || resolution_used_from_other_file_p (node-resolution))
+   continue;
+
+ /* For now, until we walk references.  */
+ if (node-address_taken)
+   continue;
+
+ if (!node-callers)
+   {
+ expanded_location loc = expand_location (DECL_SOURCE_LOCATION 
(node-decl));
+ fprintf (f, %s:%s no calls\n,
+  loc.file, IDENTIFIER_POINTER (DECL_NAME (node-decl)));
+   }
+ for (caller = node-callers; caller; caller = caller-next_caller)
+   {
+ if (!caller_tu)
+   caller_tu = DECL_CONTEXT (caller-caller-decl);
+ else if (caller_tu
+   DECL_CONTEXT (caller-caller-decl) != caller_tu)
+   found = false;
+   }
+ if (found  caller_tu)
+   {
+ expanded_location loc1 = expand_location (DECL_SOURCE_LOCATION 
(node-decl));
+ expanded_location loc2 = expand_location (DECL_SOURCE_LOCATION 
(node-callers-caller-decl));
+
+ if (DECL_CONTEXT (node-decl) == caller_tu)
+   fprintf (f, %s:%s can be made static\n,
+loc1.file, IDENTIFIER_POINTER (DECL_NAME 
(node-decl)));
+ else
+   {
+ struct cgraph_edge *callee;
+ bool calls_nonpublic_static_fn = false;
+ /* Check if we can move node to the caller TU without
+moving anything else.  */
+ for (callee = node-callees; callee; callee = 
callee-next_callee)
+   {
+ if (!TREE_PUBLIC (callee-callee-decl)
+  TREE_STATIC (callee-callee-decl))
+   calls_nonpublic_static_fn = true;
+   }
+ if (!calls_nonpublic_static_fn)
+   fprintf (f, %s:%s called only from %s\n,
+loc1.file, IDENTIFIER_POINTER (DECL_NAME 
(node-decl)),
+loc2.file);
+   }
+   }
+   }
+  fclose (f);
+}
+
   if (flag_ltrans)
 for (node = cgraph_nodes; node; node = node-next)
   {


[PATCH][1/n] Cleanup internal interfaces, GCC modularization

2012-03-29 Thread Richard Guenther

I am playing with doing some internal interface static analysis
using the first patch below (and looking at LTO bootstrap results).

An example, obvious patch resulting from that is the 2nd patch,
resuling from the static analysis output

/space/rguenther/src/svn/trunk/gcc/tree-ssa-pre.c:add_to_value can be made 
static
/space/rguenther/src/svn/trunk/gcc/tree-ssa-pre.c:print_value_expressions 
can be made static

the static analysis is very verbose (and does not consider ipa-refs yet).
You also need to union results for building all frontends and all
targets (well, in theory, or you can simply manually verify things
which is a good idea anyway - even unused functions may be useful
exported when they implement a generic data structure for example).

Excercise for the reader: turn the analysis into a plugin.

Richard.


Index: gcc/lto/lto.c
===
--- gcc/lto/lto.c   (revision 185918)
+++ gcc/lto/lto.c   (working copy)
@@ -2721,6 +2721,65 @@ read_cgraph_and_symbols (unsigned nfiles
   lto_symtab_merge_cgraph_nodes ();
   ggc_collect ();
 
+  if (flag_wpa)
+{
+  struct cgraph_node *node;
+  FILE *f = fopen (concat (dump_base_name, .callers, NULL), w);
+  for (node = cgraph_nodes; node; node = node-next)
+   {
+ tree caller_tu = NULL_TREE;
+ struct cgraph_edge *caller;
+ bool found = true;
+
+ if (!TREE_PUBLIC (node-decl)
+ || !TREE_STATIC (node-decl)
+ || resolution_used_from_other_file_p (node-resolution))
+   continue;
+
+ if (!node-callers)
+   {
+ expanded_location loc = expand_location (DECL_SOURCE_LOCATION 
(node-decl));
+ fprintf (f, %s:%s no calls\n,
+  loc.file, IDENTIFIER_POINTER (DECL_NAME (node-decl)));
+   }
+ for (caller = node-callers; caller; caller = caller-next_caller)
+   {
+ if (!caller_tu)
+   caller_tu = DECL_CONTEXT (caller-caller-decl);
+ else if (caller_tu
+   DECL_CONTEXT (caller-caller-decl) != caller_tu)
+   found = false;
+   }
+ if (found  caller_tu)
+   {
+ expanded_location loc1 = expand_location (DECL_SOURCE_LOCATION 
(node-decl));
+ expanded_location loc2 = expand_location (DECL_SOURCE_LOCATION 
(node-callers-caller-decl));
+
+ if (DECL_CONTEXT (node-decl) == caller_tu)
+   fprintf (f, %s:%s can be made static\n,
+loc1.file, IDENTIFIER_POINTER (DECL_NAME 
(node-decl)));
+ else
+   {
+ struct cgraph_edge *callee;
+ bool calls_nonpublic_static_fn = false;
+ /* Check if we can move node to the caller TU without
+moving anything else.  */
+ for (callee = node-callees; callee; callee = 
callee-next_callee)
+   {
+ if (!TREE_PUBLIC (callee-callee-decl)
+  TREE_STATIC (callee-callee-decl))
+   calls_nonpublic_static_fn = true;
+   }
+ if (!calls_nonpublic_static_fn)
+   fprintf (f, %s:%s called only from %s\n,
+loc1.file, IDENTIFIER_POINTER (DECL_NAME 
(node-decl)),
+loc2.file);
+   }
+   }
+   }
+  fclose (f);
+}
+
   if (flag_ltrans)
 for (node = cgraph_nodes; node; node = node-next)
   {



2012-03-29  Richard Guenther  rguent...@suse.de

* tree-flow.h (struct pre_expr_d): Remove forward declaration.
(add_to_value): Remove.
(print_value_expressions): Likewise.
* tree-ssa-pre.c (add_to_value): Make static.
(print_value_expressions): Likewise.

Index: gcc/tree-flow.h
===
*** gcc/tree-flow.h (revision 185918)
--- gcc/tree-flow.h (working copy)
*** extern bool verify_eh_dispatch_edge (gim
*** 794,803 
  extern void maybe_remove_unreachable_handlers (void);
  
  /* In tree-ssa-pre.c  */
- struct pre_expr_d;
- void add_to_value (unsigned int, struct pre_expr_d *);
  void debug_value_expressions (unsigned int);
- void print_value_expressions (FILE *, unsigned int);
  
  /* In tree-ssa-sink.c  */
  bool is_hidden_global_store (gimple);
--- 794,800 
Index: gcc/tree-ssa-pre.c
===
*** gcc/tree-ssa-pre.c  (revision 185918)
--- gcc/tree-ssa-pre.c  (working copy)
*** phi_trans_add (pre_expr e, pre_expr v, b
*** 587,593 
  
  /* Add expression E to the expression set of value id V.  */
  
! void
  add_to_value (unsigned int v, pre_expr e)
  {
bitmap_set_t set;
--- 587,593 
  
  /* Add expression E to the expression set of value id V.  */
  
! static void
  

Re: [PATCH][1/n] Cleanup internal interfaces, GCC modularization

2012-03-29 Thread Jan Hubicka
 
 I am playing with doing some internal interface static analysis
 using the first patch below (and looking at LTO bootstrap results).
 
 An example, obvious patch resulting from that is the 2nd patch,
 resuling from the static analysis output
 
 /space/rguenther/src/svn/trunk/gcc/tree-ssa-pre.c:add_to_value can be made 
 static
 /space/rguenther/src/svn/trunk/gcc/tree-ssa-pre.c:print_value_expressions 
 can be made static
 
 the static analysis is very verbose (and does not consider ipa-refs yet).
 You also need to union results for building all frontends and all
 targets (well, in theory, or you can simply manually verify things
 which is a good idea anyway - even unused functions may be useful
 exported when they implement a generic data structure for example).
 
 Excercise for the reader: turn the analysis into a plugin.
 
 Richard.
 
 
 Index: gcc/lto/lto.c
 ===
 --- gcc/lto/lto.c (revision 185918)
 +++ gcc/lto/lto.c (working copy)
 @@ -2721,6 +2721,65 @@ read_cgraph_and_symbols (unsigned nfiles
lto_symtab_merge_cgraph_nodes ();
ggc_collect ();
  
 +  if (flag_wpa)
 +{
 +  struct cgraph_node *node;
 +  FILE *f = fopen (concat (dump_base_name, .callers, NULL), w);
 +  for (node = cgraph_nodes; node; node = node-next)
 + {
 +   tree caller_tu = NULL_TREE;
 +   struct cgraph_edge *caller;
 +   bool found = true;
 +
 +   if (!TREE_PUBLIC (node-decl)
 +   || !TREE_STATIC (node-decl)
 +   || resolution_used_from_other_file_p (node-resolution))
 + continue;
 +
 +   if (!node-callers)
 + {
 +   expanded_location loc = expand_location (DECL_SOURCE_LOCATION 
 (node-decl));
 +   fprintf (f, %s:%s no calls\n,
 +loc.file, IDENTIFIER_POINTER (DECL_NAME (node-decl)));
 + }

With Mozilla folks I used the dumps from first WPA unreachable function removal 
pass
with some degree of success.  This gets a lot of non-trivial cases of dead code,
but also there are a lot of funny false positives wrt comdats etc.
 +   for (caller = node-callers; caller; caller = caller-next_caller)
 + {
 +   if (!caller_tu)
 + caller_tu = DECL_CONTEXT (caller-caller-decl);
 +   else if (caller_tu
 + DECL_CONTEXT (caller-caller-decl) != caller_tu)
 + found = false;
 + }
Extending to IPA-REF should be straighforward.
 +   if (found  caller_tu)
 + {
 +   expanded_location loc1 = expand_location (DECL_SOURCE_LOCATION 
 (node-decl));
 +   expanded_location loc2 = expand_location (DECL_SOURCE_LOCATION 
 (node-callers-caller-decl));
 +
 +   if (DECL_CONTEXT (node-decl) == caller_tu)
 + fprintf (f, %s:%s can be made static\n,
 +  loc1.file, IDENTIFIER_POINTER (DECL_NAME 
 (node-decl)));
Indeed, this is also useful. Any plans to turn this into general -Wsomething,
or you will also stay just with an internal hack like I did? :)

Honza