public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] add statistics counting to postreload, copy-rename, and math-opts
@ 2011-04-12 14:16 Nathan Froyd
  2011-04-12 14:27 ` Richard Guenther
  0 siblings, 1 reply; 11+ messages in thread
From: Nathan Froyd @ 2011-04-12 14:16 UTC (permalink / raw)
  To: gcc-patches

It's a shame more passes don't make use of the statistics_*
infrastructure.  This patch is a step towards rectifying that and adds
statistics_counter_event calls to passes mentioned in $SUBJECT.
postreload-gcse already tracked the stats for the dump file and so only
needs the statistics_counter_event calls; the other passes needed to be
taught about the statistics also.

Tested on x86_64-unknown-linux-gnu.  OK to commit?

-Nathan

	* postreload-gcse.c (gcse_after_reload_main): Add calls to
	statistics_counter_event.
	* tree-ssa-copyrename.c (stats): Define.
	(rename_ssa_copies): Count coalesced SSA_NAMEs.  Add call to
	statistics_counter_event.
	* tree-ssa-math-opts.c (reciprocal_stats, sincos_stats): Define.
	(bswap_stats, widen_mul_stats): Define.
	(insert_reciprocals): Increment rdivs_inserted.
	(execute_cse_reciprocals): Zeroize reciprocal_stats.  Increment
	rfuncs_inserted.  Add calls to statistics_counter_event.
	(execute_cse_sincos_1): Increment inserted.
	(execute_cse_sincos): Zeroize sincos_stats.  Add call to
	statistics_counter_event.
	(execute_optimize_bswap): Zeroize bswap_stats.  Increment fields
	of bswap_stats.  Add calls to statistics_counter_event.
	(convert_mult_to_widen): Increment widen_mults_inserted.
	(convert_plusminus_to_widen): Increment maccs_inserted.
	(convert_mult_to_fma): Increment fmas_inserted.
	(execute_optimize_widening_mul): Zeroize widen_mul_stats.  Add
	calls to statistics_counter_event.

diff --git a/gcc/postreload-gcse.c b/gcc/postreload-gcse.c
index 7eeecf4..8e26419 100644
--- a/gcc/postreload-gcse.c
+++ b/gcc/postreload-gcse.c
@@ -1294,6 +1294,13 @@ gcse_after_reload_main (rtx f ATTRIBUTE_UNUSED)
 	  fprintf (dump_file, "insns deleted:   %d\n", stats.insns_deleted);
 	  fprintf (dump_file, "\n\n");
 	}
+
+      statistics_counter_event (cfun, "copies inserted",
+				stats.copies_inserted);
+      statistics_counter_event (cfun, "moves inserted",
+				stats.moves_inserted);
+      statistics_counter_event (cfun, "insns deleted",
+				stats.insns_deleted);
     }
 
   /* We are finished with alias.  */
diff --git a/gcc/tree-ssa-copyrename.c b/gcc/tree-ssa-copyrename.c
index dfc0b4e..ae4fb5f 100644
--- a/gcc/tree-ssa-copyrename.c
+++ b/gcc/tree-ssa-copyrename.c
@@ -40,6 +40,12 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-pass.h"
 #include "langhooks.h"
 
+static struct
+{
+  /* Number of copies coalesced.  */
+  int coalesced;
+} stats;
+
 /* The following routines implement the SSA copy renaming phase.
 
    This optimization looks for copies between 2 SSA_NAMES, either through a
@@ -360,9 +366,12 @@ rename_ssa_copies (void)
 	      fprintf (debug, "\n");
 	    }
 	}
+      stats.coalesced++;
       replace_ssa_name_symbol (var, SSA_NAME_VAR (part_var));
     }
 
+  statistics_counter_event (cfun, "copies coalesced",
+			    stats.coalesced);
   delete_var_map (map);
   return updated ? TODO_remove_unused_locals : 0;
 }
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index 6e2213c..b9f631e 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -138,6 +138,41 @@ struct occurrence {
   bool bb_has_division;
 };
 
+static struct
+{
+  /* Number of 1.0/X ops inserted.  */
+  int rdivs_inserted;
+
+  /* Number of 1.0/FUNC ops inserted.  */
+  int rfuncs_inserted;
+} reciprocal_stats;
+
+static struct
+{
+  /* Number of cexpi calls inserted.  */
+  int inserted;
+} sincos_stats;
+
+static struct
+{
+  /* Number of hand-written 32-bit bswaps found.  */
+  int found_32bit;
+
+  /* Number of hand-written 64-bit bswaps found.  */
+  int found_64bit;
+} bswap_stats;
+
+static struct
+{
+  /* Number of widening multiplication ops inserted.  */
+  int widen_mults_inserted;
+
+  /* Number of integer multiply-and-accumulate ops inserted.  */
+  int maccs_inserted;
+
+  /* Number of fp fused multiply-add ops inserted.  */
+  int fmas_inserted;
+} widen_mul_stats;
 
 /* The instance of "struct occurrence" representing the highest
    interesting block in the dominator tree.  */
@@ -339,6 +374,8 @@ insert_reciprocals (gimple_stmt_iterator *def_gsi, struct occurrence *occ,
           gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
         }
 
+      reciprocal_stats.rdivs_inserted++;
+
       occ->recip_def_stmt = new_stmt;
     }
 
@@ -466,6 +503,7 @@ execute_cse_reciprocals (void)
 				sizeof (struct occurrence),
 				n_basic_blocks / 3 + 1);
 
+  memset (&reciprocal_stats, 0, sizeof (reciprocal_stats));
   calculate_dominance_info (CDI_DOMINATORS);
   calculate_dominance_info (CDI_POST_DOMINATORS);
 
@@ -568,6 +606,7 @@ execute_cse_reciprocals (void)
 		  gimple_replace_lhs (stmt1, arg1);
 		  gimple_call_set_fndecl (stmt1, fndecl);
 		  update_stmt (stmt1);
+		  reciprocal_stats.rfuncs_inserted++;
 
 		  FOR_EACH_IMM_USE_STMT (stmt, ui, arg1)
 		    {
@@ -580,6 +619,11 @@ execute_cse_reciprocals (void)
 	}
     }
 
+  statistics_counter_event (cfun, "reciprocal divs inserted",
+			    reciprocal_stats.rdivs_inserted);
+  statistics_counter_event (cfun, "reciprocal functions inserted",
+			    reciprocal_stats.rfuncs_inserted);
+
   free_dominance_info (CDI_DOMINATORS);
   free_dominance_info (CDI_POST_DOMINATORS);
   free_alloc_pool (occ_pool);
@@ -711,6 +755,7 @@ execute_cse_sincos_1 (tree name)
       gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
     }
   update_stmt (stmt);
+  sincos_stats.inserted++;
 
   /* And adjust the recorded old call sites.  */
   for (i = 0; VEC_iterate(gimple, stmts, i, use_stmt); ++i)
@@ -760,6 +805,7 @@ execute_cse_sincos (void)
   bool cfg_changed = false;
 
   calculate_dominance_info (CDI_DOMINATORS);
+  memset (&sincos_stats, 0, sizeof (sincos_stats));
 
   FOR_EACH_BB (bb)
     {
@@ -793,6 +839,9 @@ execute_cse_sincos (void)
 	}
     }
 
+  statistics_counter_event (cfun, "sincos statements inserted",
+			    sincos_stats.inserted);
+
   free_dominance_info (CDI_DOMINATORS);
   return cfg_changed ? TODO_cleanup_cfg : 0;
 }
@@ -1141,6 +1190,8 @@ execute_optimize_bswap (void)
       bswap64_type = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
     }
 
+  memset (&bswap_stats, 0, sizeof (bswap_stats));
+
   FOR_EACH_BB (bb)
     {
       gimple_stmt_iterator gsi;
@@ -1189,6 +1240,10 @@ execute_optimize_bswap (void)
 	    continue;
 
 	  changed = true;
+	  if (type_size == 32)
+	    bswap_stats.found_32bit++;
+	  else
+	    bswap_stats.found_64bit++;
 
 	  bswap_tmp = bswap_src;
 
@@ -1237,6 +1292,11 @@ execute_optimize_bswap (void)
 	}
     }
 
+  statistics_counter_event (cfun, "32-bit bswap implementations found",
+			    bswap_stats.found_32bit);
+  statistics_counter_event (cfun, "64-bit bswap implementations found",
+			    bswap_stats.found_64bit);
+
   return (changed ? TODO_dump_func | TODO_update_ssa | TODO_verify_ssa
 	  | TODO_verify_stmts : 0);
 }
@@ -1389,6 +1449,7 @@ convert_mult_to_widen (gimple stmt)
   gimple_assign_set_rhs2 (stmt, fold_convert (type2, rhs2));
   gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR);
   update_stmt (stmt);
+  widen_mul_stats.widen_mults_inserted++;
   return true;
 }
 
@@ -1491,6 +1552,7 @@ convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
 				    fold_convert (type2, mult_rhs2),
 				    add_rhs);
   update_stmt (gsi_stmt (*gsi));
+  widen_mul_stats.maccs_inserted++;
   return true;
 }
 
@@ -1666,6 +1728,7 @@ convert_mult_to_fma (gimple mul_stmt, tree op1, tree op2)
 						mulop1, op2,
 						addop);
       gsi_replace (&gsi, fma_stmt, true);
+      widen_mul_stats.fmas_inserted++;
     }
 
   return true;
@@ -1681,6 +1744,8 @@ execute_optimize_widening_mul (void)
   basic_block bb;
   bool cfg_changed = false;
 
+  memset (&widen_mul_stats, 0, sizeof (widen_mul_stats));
+
   FOR_EACH_BB (bb)
     {
       gimple_stmt_iterator gsi;
@@ -1752,6 +1817,13 @@ execute_optimize_widening_mul (void)
 	}
     }
 
+  statistics_counter_event (cfun, "widening multiplications inserted",
+			    widen_mul_stats.widen_mults_inserted);
+  statistics_counter_event (cfun, "widening maccs inserted",
+			    widen_mul_stats.maccs_inserted);
+  statistics_counter_event (cfun, "fused multiply-adds inserted",
+			    widen_mul_stats.fmas_inserted);
+
   return cfg_changed ? TODO_cleanup_cfg : 0;
 }
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-12 14:16 [PATCH] add statistics counting to postreload, copy-rename, and math-opts Nathan Froyd
@ 2011-04-12 14:27 ` Richard Guenther
  2011-04-12 14:32   ` Nathan Froyd
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Guenther @ 2011-04-12 14:27 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 9915 bytes --]

On Tue, Apr 12, 2011 at 4:16 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> It's a shame more passes don't make use of the statistics_*
> infrastructure.  This patch is a step towards rectifying that and adds
> statistics_counter_event calls to passes mentioned in $SUBJECT.
> postreload-gcse already tracked the stats for the dump file and so only
> needs the statistics_counter_event calls; the other passes needed to be
> taught about the statistics also.
>
> Tested on x86_64-unknown-linux-gnu.  OK to commit?

Ok if there are no complaints within 24h.  I actually have a local patch
adding many of these which I use whenever fiddling with the pass pipeline ...
(attached).

Richard.

> -Nathan
>
>        * postreload-gcse.c (gcse_after_reload_main): Add calls to
>        statistics_counter_event.
>        * tree-ssa-copyrename.c (stats): Define.
>        (rename_ssa_copies): Count coalesced SSA_NAMEs.  Add call to
>        statistics_counter_event.
>        * tree-ssa-math-opts.c (reciprocal_stats, sincos_stats): Define.
>        (bswap_stats, widen_mul_stats): Define.
>        (insert_reciprocals): Increment rdivs_inserted.
>        (execute_cse_reciprocals): Zeroize reciprocal_stats.  Increment
>        rfuncs_inserted.  Add calls to statistics_counter_event.
>        (execute_cse_sincos_1): Increment inserted.
>        (execute_cse_sincos): Zeroize sincos_stats.  Add call to
>        statistics_counter_event.
>        (execute_optimize_bswap): Zeroize bswap_stats.  Increment fields
>        of bswap_stats.  Add calls to statistics_counter_event.
>        (convert_mult_to_widen): Increment widen_mults_inserted.
>        (convert_plusminus_to_widen): Increment maccs_inserted.
>        (convert_mult_to_fma): Increment fmas_inserted.
>        (execute_optimize_widening_mul): Zeroize widen_mul_stats.  Add
>        calls to statistics_counter_event.
>
> diff --git a/gcc/postreload-gcse.c b/gcc/postreload-gcse.c
> index 7eeecf4..8e26419 100644
> --- a/gcc/postreload-gcse.c
> +++ b/gcc/postreload-gcse.c
> @@ -1294,6 +1294,13 @@ gcse_after_reload_main (rtx f ATTRIBUTE_UNUSED)
>          fprintf (dump_file, "insns deleted:   %d\n", stats.insns_deleted);
>          fprintf (dump_file, "\n\n");
>        }
> +
> +      statistics_counter_event (cfun, "copies inserted",
> +                               stats.copies_inserted);
> +      statistics_counter_event (cfun, "moves inserted",
> +                               stats.moves_inserted);
> +      statistics_counter_event (cfun, "insns deleted",
> +                               stats.insns_deleted);
>     }
>
>   /* We are finished with alias.  */
> diff --git a/gcc/tree-ssa-copyrename.c b/gcc/tree-ssa-copyrename.c
> index dfc0b4e..ae4fb5f 100644
> --- a/gcc/tree-ssa-copyrename.c
> +++ b/gcc/tree-ssa-copyrename.c
> @@ -40,6 +40,12 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tree-pass.h"
>  #include "langhooks.h"
>
> +static struct
> +{
> +  /* Number of copies coalesced.  */
> +  int coalesced;
> +} stats;
> +
>  /* The following routines implement the SSA copy renaming phase.
>
>    This optimization looks for copies between 2 SSA_NAMES, either through a
> @@ -360,9 +366,12 @@ rename_ssa_copies (void)
>              fprintf (debug, "\n");
>            }
>        }
> +      stats.coalesced++;
>       replace_ssa_name_symbol (var, SSA_NAME_VAR (part_var));
>     }
>
> +  statistics_counter_event (cfun, "copies coalesced",
> +                           stats.coalesced);
>   delete_var_map (map);
>   return updated ? TODO_remove_unused_locals : 0;
>  }
> diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
> index 6e2213c..b9f631e 100644
> --- a/gcc/tree-ssa-math-opts.c
> +++ b/gcc/tree-ssa-math-opts.c
> @@ -138,6 +138,41 @@ struct occurrence {
>   bool bb_has_division;
>  };
>
> +static struct
> +{
> +  /* Number of 1.0/X ops inserted.  */
> +  int rdivs_inserted;
> +
> +  /* Number of 1.0/FUNC ops inserted.  */
> +  int rfuncs_inserted;
> +} reciprocal_stats;
> +
> +static struct
> +{
> +  /* Number of cexpi calls inserted.  */
> +  int inserted;
> +} sincos_stats;
> +
> +static struct
> +{
> +  /* Number of hand-written 32-bit bswaps found.  */
> +  int found_32bit;
> +
> +  /* Number of hand-written 64-bit bswaps found.  */
> +  int found_64bit;
> +} bswap_stats;
> +
> +static struct
> +{
> +  /* Number of widening multiplication ops inserted.  */
> +  int widen_mults_inserted;
> +
> +  /* Number of integer multiply-and-accumulate ops inserted.  */
> +  int maccs_inserted;
> +
> +  /* Number of fp fused multiply-add ops inserted.  */
> +  int fmas_inserted;
> +} widen_mul_stats;
>
>  /* The instance of "struct occurrence" representing the highest
>    interesting block in the dominator tree.  */
> @@ -339,6 +374,8 @@ insert_reciprocals (gimple_stmt_iterator *def_gsi, struct occurrence *occ,
>           gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
>         }
>
> +      reciprocal_stats.rdivs_inserted++;
> +
>       occ->recip_def_stmt = new_stmt;
>     }
>
> @@ -466,6 +503,7 @@ execute_cse_reciprocals (void)
>                                sizeof (struct occurrence),
>                                n_basic_blocks / 3 + 1);
>
> +  memset (&reciprocal_stats, 0, sizeof (reciprocal_stats));
>   calculate_dominance_info (CDI_DOMINATORS);
>   calculate_dominance_info (CDI_POST_DOMINATORS);
>
> @@ -568,6 +606,7 @@ execute_cse_reciprocals (void)
>                  gimple_replace_lhs (stmt1, arg1);
>                  gimple_call_set_fndecl (stmt1, fndecl);
>                  update_stmt (stmt1);
> +                 reciprocal_stats.rfuncs_inserted++;
>
>                  FOR_EACH_IMM_USE_STMT (stmt, ui, arg1)
>                    {
> @@ -580,6 +619,11 @@ execute_cse_reciprocals (void)
>        }
>     }
>
> +  statistics_counter_event (cfun, "reciprocal divs inserted",
> +                           reciprocal_stats.rdivs_inserted);
> +  statistics_counter_event (cfun, "reciprocal functions inserted",
> +                           reciprocal_stats.rfuncs_inserted);
> +
>   free_dominance_info (CDI_DOMINATORS);
>   free_dominance_info (CDI_POST_DOMINATORS);
>   free_alloc_pool (occ_pool);
> @@ -711,6 +755,7 @@ execute_cse_sincos_1 (tree name)
>       gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
>     }
>   update_stmt (stmt);
> +  sincos_stats.inserted++;
>
>   /* And adjust the recorded old call sites.  */
>   for (i = 0; VEC_iterate(gimple, stmts, i, use_stmt); ++i)
> @@ -760,6 +805,7 @@ execute_cse_sincos (void)
>   bool cfg_changed = false;
>
>   calculate_dominance_info (CDI_DOMINATORS);
> +  memset (&sincos_stats, 0, sizeof (sincos_stats));
>
>   FOR_EACH_BB (bb)
>     {
> @@ -793,6 +839,9 @@ execute_cse_sincos (void)
>        }
>     }
>
> +  statistics_counter_event (cfun, "sincos statements inserted",
> +                           sincos_stats.inserted);
> +
>   free_dominance_info (CDI_DOMINATORS);
>   return cfg_changed ? TODO_cleanup_cfg : 0;
>  }
> @@ -1141,6 +1190,8 @@ execute_optimize_bswap (void)
>       bswap64_type = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
>     }
>
> +  memset (&bswap_stats, 0, sizeof (bswap_stats));
> +
>   FOR_EACH_BB (bb)
>     {
>       gimple_stmt_iterator gsi;
> @@ -1189,6 +1240,10 @@ execute_optimize_bswap (void)
>            continue;
>
>          changed = true;
> +         if (type_size == 32)
> +           bswap_stats.found_32bit++;
> +         else
> +           bswap_stats.found_64bit++;
>
>          bswap_tmp = bswap_src;
>
> @@ -1237,6 +1292,11 @@ execute_optimize_bswap (void)
>        }
>     }
>
> +  statistics_counter_event (cfun, "32-bit bswap implementations found",
> +                           bswap_stats.found_32bit);
> +  statistics_counter_event (cfun, "64-bit bswap implementations found",
> +                           bswap_stats.found_64bit);
> +
>   return (changed ? TODO_dump_func | TODO_update_ssa | TODO_verify_ssa
>          | TODO_verify_stmts : 0);
>  }
> @@ -1389,6 +1449,7 @@ convert_mult_to_widen (gimple stmt)
>   gimple_assign_set_rhs2 (stmt, fold_convert (type2, rhs2));
>   gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR);
>   update_stmt (stmt);
> +  widen_mul_stats.widen_mults_inserted++;
>   return true;
>  }
>
> @@ -1491,6 +1552,7 @@ convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
>                                    fold_convert (type2, mult_rhs2),
>                                    add_rhs);
>   update_stmt (gsi_stmt (*gsi));
> +  widen_mul_stats.maccs_inserted++;
>   return true;
>  }
>
> @@ -1666,6 +1728,7 @@ convert_mult_to_fma (gimple mul_stmt, tree op1, tree op2)
>                                                mulop1, op2,
>                                                addop);
>       gsi_replace (&gsi, fma_stmt, true);
> +      widen_mul_stats.fmas_inserted++;
>     }
>
>   return true;
> @@ -1681,6 +1744,8 @@ execute_optimize_widening_mul (void)
>   basic_block bb;
>   bool cfg_changed = false;
>
> +  memset (&widen_mul_stats, 0, sizeof (widen_mul_stats));
> +
>   FOR_EACH_BB (bb)
>     {
>       gimple_stmt_iterator gsi;
> @@ -1752,6 +1817,13 @@ execute_optimize_widening_mul (void)
>        }
>     }
>
> +  statistics_counter_event (cfun, "widening multiplications inserted",
> +                           widen_mul_stats.widen_mults_inserted);
> +  statistics_counter_event (cfun, "widening maccs inserted",
> +                           widen_mul_stats.maccs_inserted);
> +  statistics_counter_event (cfun, "fused multiply-adds inserted",
> +                           widen_mul_stats.fmas_inserted);
> +
>   return cfg_changed ? TODO_cleanup_cfg : 0;
>  }
>
>

[-- Attachment #2: passes-reorder --]
[-- Type: application/octet-stream, Size: 19566 bytes --]

2008-08-14  Richard Guenther  <rguenther@suse.de>

	* tree-cfgcleanup.c (remove_forwarder_block): Count the event.
	(remove_forwarder_block_with_phi): Likewise.
	* tree-ssa-dse.c (dse_optimize_stmt): Count the number of
	deleted stores.
	* tree-ssa-forwprop.c (remove_prop_source_from_use): Count the
	number of deleted statements.
	(forward_propagate_into_gimple_cond): Count the number of
	simplified conditions.
	(forward_propagate_into_cond): Likewise.
	(forward_propagate_addr_expr): Count the number of propagated
	addresses.  Count the number of deleted statements.
	(forward_propagate_comparison): Count the number of propagated
	conversions.
	(simplify_not_neg_expr): Count the number of propagated negations.
	(simplify_gimple_switch): Count the number of propagated conversions.
	(tree_ssa_forward_propagate_single_use_vars): Count the number of
	deleted statements.
	* tree-ssa.c (execute_update_addresses_taken): Count the number
	of variables promoted to registers.
	* tree-tailcall.c (optimize_tail_call): Count the number of
	converted tail recursions.
	* cfghooks.c (delete_basic_block): Count the number of deleted
	basic blocks.
	(merge_blocks): Count the number of merged blocks.
	* tree-inline.c (expand_call_inline): Count the number of calls
	inlined.
	* tree-ssa-phiprop.c (propagate_with_phi): Count the number of
	converted loads.
	(pass_phiprop): Require PROP_alias.
	* tree-ssa-ifcombine.c (ifcombine_ifandif): Count the number of
	combined bit tests.
	(ifcombine_iforif): Likewise.  Count the number of combined
	comparisons.
	* tree-ssa-loop-ivcanon.c (tree_unroll_loops_completely): Count
	the number of unrolled loops.
	* tree-ssa-phiopt.c (conditional_replacement): Count the
	number of simplifications.
	(value_replacement): Likewise.
	(minmax_replacement): Likewise.
	(abs_replacement): Likewise.
	(cond_store_replacement): Likewise.



Index: trunk/gcc/tree-cfgcleanup.c
===================================================================
*** trunk.orig/gcc/tree-cfgcleanup.c	2010-08-30 15:45:07.000000000 +0200
--- trunk/gcc/tree-cfgcleanup.c	2010-09-21 13:43:02.000000000 +0200
*************** remove_forwarder_block (basic_block bb)
*** 534,539 ****
--- 534,541 ----
    /* And kill the forwarder block.  */
    delete_basic_block (bb);
  
+   statistics_counter_event (cfun, "Forwarder blocks removed", 1);
+ 
    return true;
  }
  
*************** remove_forwarder_block_with_phi (basic_b
*** 938,943 ****
--- 940,947 ----
    /* Remove BB since all of BB's incoming edges have been redirected
       to DEST.  */
    delete_basic_block (bb);
+ 
+   statistics_counter_event (cfun, "Forwarder blocks removed", 1);
  }
  
  /* This pass merges PHI nodes if one feeds into another.  For example,
Index: trunk/gcc/tree-ssa-dse.c
===================================================================
*** trunk.orig/gcc/tree-ssa-dse.c	2010-07-09 10:42:02.000000000 +0200
--- trunk/gcc/tree-ssa-dse.c	2010-09-21 13:43:02.000000000 +0200
*************** dse_optimize_stmt (struct dse_global_dat
*** 330,335 ****
--- 330,336 ----
                print_gimple_stmt (dump_file, gsi_stmt (gsi), dump_flags, 0);
                fprintf (dump_file, "'\n");
              }
+ 	  statistics_counter_event (cfun, "Stores deleted", 1);
  
  	  /* Then we need to fix the operand of the consuming stmt.  */
  	  unlink_stmt_vdef (stmt);
Index: trunk/gcc/tree-ssa-forwprop.c
===================================================================
*** trunk.orig/gcc/tree-ssa-forwprop.c	2010-08-30 15:45:07.000000000 +0200
--- trunk/gcc/tree-ssa-forwprop.c	2010-09-21 13:43:02.000000000 +0200
*************** remove_prop_source_from_use (tree name,
*** 321,326 ****
--- 321,327 ----
      gsi = gsi_for_stmt (stmt);
      release_defs (stmt);
      gsi_remove (&gsi, true);
+     statistics_counter_event (cfun, "Statements deleted", 1);
  
      name = (gimple_assign_copy_p (stmt)) ? gimple_assign_rhs1 (stmt) : NULL;
    } while (name && TREE_CODE (name) == SSA_NAME);
*************** forward_propagate_into_gimple_cond (gimp
*** 461,466 ****
--- 462,468 ----
  
          gimple_cond_set_condition_from_tree (stmt, unshare_expr (tmp));
  	update_stmt (stmt);
+ 	statistics_counter_event (cfun, "Conditions simplified", 1);
  
  	/* Remove defining statements.  */
  	remove_prop_source_from_use (name, NULL);
*************** forward_propagate_into_cond (gimple_stmt
*** 572,577 ****
--- 574,580 ----
  	gimple_assign_set_rhs_from_tree (gsi_p, unshare_expr (tmp));
  	stmt = gsi_stmt (*gsi_p);
  	update_stmt (stmt);
+ 	statistics_counter_event (cfun, "Conditions simplified", 1);
  
  	/* Remove defining statements.  */
  	remove_prop_source_from_use (name, NULL);
*************** tidy_after_forward_propagate_addr (gimpl
*** 604,609 ****
--- 607,614 ----
  
    if (TREE_CODE (gimple_assign_rhs1 (stmt)) == ADDR_EXPR)
       recompute_tree_invariant_for_addr_expr (gimple_assign_rhs1 (stmt));
+ 
+   statistics_counter_event (cfun, "Addresses propagated", 1);
  }
  
  /* DEF_RHS contains the address of the 0th element in an array.
*************** forward_propagate_addr_expr (tree name,
*** 1106,1111 ****
--- 1111,1117 ----
  	  gimple_stmt_iterator gsi = gsi_for_stmt (use_stmt);
  	  release_defs (use_stmt);
  	  gsi_remove (&gsi, true);
+ 	  statistics_counter_event (cfun, "Statements deleted", 1);
  	}
      }
  
*************** forward_propagate_comparison (gimple stm
*** 1200,1205 ****
--- 1206,1212 ----
  	gimple_assign_set_rhs_from_tree (&gsi, unshare_expr (tmp));
  	use_stmt = gsi_stmt (gsi);
  	update_stmt (use_stmt);
+ 	statistics_counter_event (cfun, "Conversions propagated", 1);
        }
  
        /* Remove defining statements.  */
*************** simplify_not_neg_expr (gimple_stmt_itera
*** 1256,1261 ****
--- 1263,1269 ----
  	  gimple_assign_set_rhs_from_tree (gsi_p, rhs_def_operand);
  	  stmt = gsi_stmt (*gsi_p);
  	  update_stmt (stmt);
+ 	  statistics_counter_event (cfun, "Negations propagated", 1);
  	}
      }
  }
*************** simplify_gimple_switch (gimple stmt)
*** 1311,1316 ****
--- 1319,1325 ----
  		{
  		  gimple_switch_set_index (stmt, def);
  		  update_stmt (stmt);
+ 		  statistics_counter_event (cfun, "Conversions propagated", 1);
  		}
  	    }
  	}
*************** tree_ssa_forward_propagate_single_use_va
*** 1687,1692 ****
--- 1696,1702 ----
  		      release_defs (stmt);
  		      todoflags |= TODO_remove_unused_locals;
  		      gsi_remove (&gsi, true);
+ 		      statistics_counter_event (cfun, "Statements deleted", 1);
  		    }
  		  else
  		    gsi_next (&gsi);
*************** tree_ssa_forward_propagate_single_use_va
*** 1750,1755 ****
--- 1760,1766 ----
  		      release_defs (stmt);
  		      todoflags |= TODO_remove_unused_locals;
  		      gsi_remove (&gsi, true);
+ 		      statistics_counter_event (cfun, "Statements deleted", 1);
  		    }
  		  else
  		    gsi_next (&gsi);
Index: trunk/gcc/tree-tailcall.c
===================================================================
*** trunk.orig/gcc/tree-tailcall.c	2010-09-06 10:56:48.000000000 +0200
--- trunk/gcc/tree-tailcall.c	2010-09-21 13:43:02.000000000 +0200
*************** optimize_tail_call (struct tailcall *t,
*** 901,906 ****
--- 901,907 ----
    if (t->tail_recursion)
      {
        eliminate_tail_call (t);
+       statistics_counter_event (cfun, "Tail recursions", 1);
        return true;
      }
  
Index: trunk/gcc/cfghooks.c
===================================================================
*** trunk.orig/gcc/cfghooks.c	2010-07-09 10:42:02.000000000 +0200
--- trunk/gcc/cfghooks.c	2010-09-21 13:43:02.000000000 +0200
*************** delete_basic_block (basic_block bb)
*** 496,501 ****
--- 496,502 ----
      internal_error ("%s does not support delete_basic_block", cfg_hooks->name);
  
    cfg_hooks->delete_basic_block (bb);
+   statistics_counter_event (cfun, "Blocks deleted", 1);
  
    if (current_loops != NULL)
      {
*************** merge_blocks (basic_block a, basic_block
*** 681,686 ****
--- 682,688 ----
      internal_error ("%s does not support merge_blocks", cfg_hooks->name);
  
    cfg_hooks->merge_blocks (a, b);
+   statistics_counter_event (cfun, "Blocks merged", 1);
  
    if (current_loops != NULL)
      remove_bb_from_loops (b);
Index: trunk/gcc/tree-inline.c
===================================================================
*** trunk.orig/gcc/tree-inline.c	2010-09-20 11:09:18.000000000 +0200
--- trunk/gcc/tree-inline.c	2010-09-21 13:43:02.000000000 +0200
*************** expand_call_inline (basic_block bb, gimp
*** 3800,3805 ****
--- 3800,3806 ----
  	}
        goto egress;
      }
+   statistics_counter_event (cfun, "Functions inlined", 1);
    fn = cg_edge->callee->decl;
  
  #ifdef ENABLE_CHECKING
Index: trunk/gcc/tree-ssa-phiprop.c
===================================================================
*** trunk.orig/gcc/tree-ssa-phiprop.c	2010-08-24 10:32:38.000000000 +0200
--- trunk/gcc/tree-ssa-phiprop.c	2010-09-21 13:43:02.000000000 +0200
*************** propagate_with_phi (basic_block bb, gimp
*** 355,360 ****
--- 355,361 ----
  	  gsi_remove (&gsi, false);
  
  	  phi_inserted = true;
+ 	  statistics_counter_event (cfun, "Loads from PHIs promoted", 1);
  	}
        else
  	{
Index: trunk/gcc/tree-ssa-ifcombine.c
===================================================================
*** trunk.orig/gcc/tree-ssa-ifcombine.c	2010-08-30 15:45:07.000000000 +0200
--- trunk/gcc/tree-ssa-ifcombine.c	2010-09-21 13:43:02.000000000 +0200
*************** ifcombine_ifandif (basic_block inner_con
*** 360,365 ****
--- 360,366 ----
  	  print_generic_expr (dump_file, bit2, 0);
  	  fprintf (dump_file, ")\n");
  	}
+       statistics_counter_event (cfun, "Bit tests combined", 1);
  
        return true;
      }
*************** ifcombine_iforif (basic_block inner_cond
*** 506,511 ****
--- 507,513 ----
  	  print_generic_expr (dump_file, bits2, 0);
  	  fprintf (dump_file, "\n");
  	}
+       statistics_counter_event (cfun, "Bit tests combined", 1);
  
        return true;
      }
*************** ifcombine_iforif (basic_block inner_cond
*** 541,546 ****
--- 543,549 ----
  	  print_generic_expr (dump_file, t, 0);
  	  fprintf (dump_file, "\n");
  	}
+       statistics_counter_event (cfun, "Comparisons combined", 1);
  
        return true;
      }
Index: trunk/gcc/tree-ssa-loop-ivcanon.c
===================================================================
*** trunk.orig/gcc/tree-ssa-loop-ivcanon.c	2010-09-20 17:52:49.000000000 +0200
--- trunk/gcc/tree-ssa-loop-ivcanon.c	2010-09-21 13:43:02.000000000 +0200
*************** tree_unroll_loops_completely (bool may_i
*** 532,539 ****
  	    ul = UL_ALL;
  	  else
  	    ul = UL_NO_GROWTH;
! 	  changed |= canonicalize_loop_induction_variables
! 		       (loop, false, ul, !flag_tree_loop_ivcanon);
  	}
  
        if (changed)
--- 532,543 ----
  	    ul = UL_ALL;
  	  else
  	    ul = UL_NO_GROWTH;
! 	  if (canonicalize_loop_induction_variables
! 	        (loop, false, ul, !flag_tree_loop_ivcanon))
! 	    {
! 	      statistics_counter_event (cfun, "Loops completely unrolled", 1);
! 	      changed = true;
! 	    }
  	}
  
        if (changed)
Index: trunk/gcc/tree-ssa-phiopt.c
===================================================================
*** trunk.orig/gcc/tree-ssa-phiopt.c	2010-08-30 15:45:07.000000000 +0200
--- trunk/gcc/tree-ssa-phiopt.c	2010-09-21 13:43:02.000000000 +0200
*************** conditional_replacement (basic_block con
*** 538,543 ****
--- 538,544 ----
    replace_phi_edge_with_variable (cond_bb, e1, phi, new_var);
  
    /* Note that we optimized this PHI.  */
+   statistics_counter_event (cfun, "CFG replaced with condition value", 1);
    return true;
  }
  
*************** value_replacement (basic_block cond_bb,
*** 616,621 ****
--- 617,623 ----
        replace_phi_edge_with_variable (cond_bb, e1, phi, arg);
  
        /* Note that we optimized this PHI.  */
+       statistics_counter_event (cfun, "CFG replaced with PHI", 1);
        return true;
      }
    return false;
*************** minmax_replacement (basic_block cond_bb,
*** 866,871 ****
--- 868,875 ----
    gsi_insert_before (&gsi, new_stmt, GSI_NEW_STMT);
  
    replace_phi_edge_with_variable (cond_bb, e1, phi, result);
+   statistics_counter_event (cfun, "CFG replaced with min/max", 1);
+ 
    return true;
  }
  
*************** abs_replacement (basic_block cond_bb, ba
*** 987,992 ****
--- 991,997 ----
      }
  
    replace_phi_edge_with_variable (cond_bb, e1, phi, result);
+   statistics_counter_event (cfun, "CFG transformed to ABS", 1);
  
    /* Note that we optimized this PHI.  */
    return true;
*************** cond_store_replacement (basic_block midd
*** 1261,1266 ****
--- 1266,1272 ----
    else
      gsi_insert_before (&gsi, new_stmt, GSI_NEW_STMT);
  
+   statistics_counter_event (cfun, "Conditional stores eliminated", 1);
    return true;
  }
  
Index: trunk/gcc/tree-predcom.c
===================================================================
*** trunk.orig/gcc/tree-predcom.c	2010-09-06 15:55:25.000000000 +0200
--- trunk/gcc/tree-predcom.c	2010-09-21 13:43:02.000000000 +0200
*************** tree_predictive_commoning_loop (struct l
*** 2544,2549 ****
--- 2544,2550 ----
  		 "Executing predictive commoning without unrolling.\n");
        execute_pred_commoning (loop, chains, tmp_vars);
      }
+   statistics_counter_event (cfun, "Loops predictive commoned", 1);
  
  end: ;
    release_chains (chains);
Index: trunk/gcc/tree-scalar-evolution.c
===================================================================
*** trunk.orig/gcc/tree-scalar-evolution.c	2010-08-24 10:32:38.000000000 +0200
--- trunk/gcc/tree-scalar-evolution.c	2010-09-21 13:43:02.000000000 +0200
*************** scev_const_prop (void)
*** 3316,3321 ****
--- 3316,3322 ----
        edge exit;
        tree def, rslt, niter;
        gimple_stmt_iterator bsi;
+       unsigned nr;
  
        /* If we do not know exact number of iterations of the loop, we cannot
  	 replace the final value.  */
*************** scev_const_prop (void)
*** 3335,3340 ****
--- 3336,3342 ----
        ex_loop = superloop_at_depth (loop,
  				    loop_depth (exit->dest->loop_father) + 1);
  
+       nr = 0;
        for (psi = gsi_start_phis (exit->dest); !gsi_end_p (psi); )
  	{
  	  phi = gsi_stmt (psi);
*************** scev_const_prop (void)
*** 3383,3389 ****
--- 3385,3394 ----
        					  true, GSI_SAME_STMT);
  	  ass = gimple_build_assign (rslt, def);
  	  gsi_insert_before (&bsi, ass, GSI_SAME_STMT);
+ 	  ++nr;
  	}
+       if (nr > 0)
+ 	statistics_counter_event (cfun, "Final loop IV values replaced", nr);
      }
    return 0;
  }
Index: trunk/gcc/tree-ssa-loop-ch.c
===================================================================
*** trunk.orig/gcc/tree-ssa-loop-ch.c	2010-07-09 10:42:02.000000000 +0200
--- trunk/gcc/tree-ssa-loop-ch.c	2010-09-21 13:43:02.000000000 +0200
*************** copy_loop_headers (void)
*** 208,213 ****
--- 208,214 ----
  	  fprintf (dump_file, "Duplication failed.\n");
  	  continue;
  	}
+       statistics_counter_event (cfun, "Loop headers copied", 1);
  
        /* If the loop has the form "for (i = j; i < j + 10; i++)" then
  	 this copying can introduce a case where we rely on undefined
Index: trunk/gcc/tree-ssa-loop-unswitch.c
===================================================================
*** trunk.orig/gcc/tree-ssa-loop-unswitch.c	2010-08-30 15:45:07.000000000 +0200
--- trunk/gcc/tree-ssa-loop-unswitch.c	2010-09-21 13:43:02.000000000 +0200
*************** tree_unswitch_single_loop (struct loop *
*** 345,350 ****
--- 345,351 ----
        free (bbs);
        return changed;
      }
+   statistics_counter_event (cfun, "Loops unswitched", 1);
  
    /* Update the SSA form after unswitching.  */
    update_ssa (TODO_update_ssa);
Index: trunk/gcc/tree-ssa-copyrename.c
===================================================================
*** trunk.orig/gcc/tree-ssa-copyrename.c	2010-08-25 17:53:05.000000000 +0200
--- trunk/gcc/tree-ssa-copyrename.c	2010-09-21 13:43:02.000000000 +0200
*************** copy_rename_partition_coalesce (var_map
*** 252,257 ****
--- 252,258 ----
  			  TDF_SLIM);
        fprintf (debug, "\n");
      }
+   statistics_counter_event (cfun, "Variables coalesced", 1);
    return true;
  }
  
Index: trunk/gcc/tree-ssa-loop-im.c
===================================================================
*** trunk.orig/gcc/tree-ssa-loop-im.c	2010-09-06 15:55:25.000000000 +0200
--- trunk/gcc/tree-ssa-loop-im.c	2010-09-21 13:43:02.000000000 +0200
*************** move_computations_stmt (struct dom_walk_
*** 1200,1206 ****
    struct loop *level;
    gimple_stmt_iterator bsi;
    gimple stmt;
!   unsigned cost = 0;
    struct lim_aux_data *lim_data;
  
    if (!loop_outer (bb->loop_father))
--- 1200,1206 ----
    struct loop *level;
    gimple_stmt_iterator bsi;
    gimple stmt;
!   unsigned cost = 0, cnt = 0;
    struct lim_aux_data *lim_data;
  
    if (!loop_outer (bb->loop_father))
*************** move_computations_stmt (struct dom_walk_
*** 1300,1310 ****
--- 1300,1313 ----
  	  fprintf (dump_file, "(cost %u) out of loop %d.\n\n",
  		   cost, level->num);
  	}
+       cnt++;
  
        mark_virtual_ops_for_renaming (stmt);
        gsi_insert_on_edge (loop_preheader_edge (level), stmt);
        gsi_remove (&bsi, false);
      }
+ 
+   statistics_counter_event (cfun, "Loop invariant motions", cnt);
  }
  
  /* Hoist the statements out of the loops prescribed by data stored in
*************** hoist_memory_references (struct loop *lo
*** 2130,2143 ****
  			 VEC (edge, heap) *exits)
  {
    mem_ref_p ref;
!   unsigned  i;
    bitmap_iterator bi;
  
    EXECUTE_IF_SET_IN_BITMAP (mem_refs, 0, i, bi)
      {
        ref = VEC_index (mem_ref_p, memory_accesses.refs_list, i);
        execute_sm (loop, exits, ref);
      }
  }
  
  /* Returns true if REF is always accessed in LOOP.  If STORED_P is true
--- 2133,2148 ----
  			 VEC (edge, heap) *exits)
  {
    mem_ref_p ref;
!   unsigned i, cnt = 0;
    bitmap_iterator bi;
  
    EXECUTE_IF_SET_IN_BITMAP (mem_refs, 0, i, bi)
      {
        ref = VEC_index (mem_ref_p, memory_accesses.refs_list, i);
        execute_sm (loop, exits, ref);
+       cnt++;
      }
+   statistics_counter_event (cfun, "Loop store motions", cnt);
  }
  
  /* Returns true if REF is always accessed in LOOP.  If STORED_P is true
Index: trunk/gcc/tree-ssa-math-opts.c
===================================================================
*** trunk.orig/gcc/tree-ssa-math-opts.c	2010-08-25 11:06:27.000000000 +0200
--- trunk/gcc/tree-ssa-math-opts.c	2010-09-21 13:43:02.000000000 +0200
*************** execute_cse_sincos_1 (tree name)
*** 686,691 ****
--- 686,693 ----
        VEC_free(gimple, heap, stmts);
        return false;
      }
+   statistics_histogram_event (cfun, "number of sin/cos calls csed to one sincos",
+ 			      VEC_length (gimple, stmts));
  
    /* Simply insert cexpi at the beginning of top_bb but not earlier than
       the name def statement.  */
Index: trunk/gcc/tree-ssa-uncprop.c
===================================================================
*** trunk.orig/gcc/tree-ssa-uncprop.c	2010-08-30 15:45:07.000000000 +0200
--- trunk/gcc/tree-ssa-uncprop.c	2010-09-21 13:43:02.000000000 +0200
*************** uncprop_into_successor_phis (basic_block
*** 499,504 ****
--- 499,505 ----
  		  if (SSA_NAME_VAR (equiv) == SSA_NAME_VAR (PHI_RESULT (phi)))
  		    {
  		      SET_PHI_ARG_DEF (phi, e->dest_idx, equiv);
+ 		      statistics_counter_event (cfun, "PHI arguments un-propagated", 1);
  		      break;
  		    }
  		}

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-12 14:27 ` Richard Guenther
@ 2011-04-12 14:32   ` Nathan Froyd
  2011-04-12 14:38     ` Richard Guenther
  0 siblings, 1 reply; 11+ messages in thread
From: Nathan Froyd @ 2011-04-12 14:32 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

On Tue, Apr 12, 2011 at 04:27:01PM +0200, Richard Guenther wrote:
> On Tue, Apr 12, 2011 at 4:16 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> > It's a shame more passes don't make use of the statistics_*
> > infrastructure.  This patch is a step towards rectifying that and adds
> > statistics_counter_event calls to passes mentioned in $SUBJECT.
> > postreload-gcse already tracked the stats for the dump file and so only
> > needs the statistics_counter_event calls; the other passes needed to be
> > taught about the statistics also.
> 
> Ok if there are no complaints within 24h.  I actually have a local patch
> adding many of these which I use whenever fiddling with the pass pipeline ...
> (attached).

Thanks.  I may go twiddle that patch to do something similar to mine and
submit that.  Do you use your patch for checking that the same set of
optimizations get performed, then?  I'm interested in using the
statistics for identifying passes that don't buy us much across a wide
variety of codebases.  (Suggestions for suitable ones welcome!)

-Nathan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-12 14:32   ` Nathan Froyd
@ 2011-04-12 14:38     ` Richard Guenther
  2011-04-12 14:51       ` Nathan Froyd
  2011-04-12 15:01       ` Steven Bosscher
  0 siblings, 2 replies; 11+ messages in thread
From: Richard Guenther @ 2011-04-12 14:38 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches

On Tue, Apr 12, 2011 at 4:32 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> On Tue, Apr 12, 2011 at 04:27:01PM +0200, Richard Guenther wrote:
>> On Tue, Apr 12, 2011 at 4:16 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
>> > It's a shame more passes don't make use of the statistics_*
>> > infrastructure.  This patch is a step towards rectifying that and adds
>> > statistics_counter_event calls to passes mentioned in $SUBJECT.
>> > postreload-gcse already tracked the stats for the dump file and so only
>> > needs the statistics_counter_event calls; the other passes needed to be
>> > taught about the statistics also.
>>
>> Ok if there are no complaints within 24h.  I actually have a local patch
>> adding many of these which I use whenever fiddling with the pass pipeline ...
>> (attached).
>
> Thanks.  I may go twiddle that patch to do something similar to mine and
> submit that.  Do you use your patch for checking that the same set of
> optimizations get performed, then?  I'm interested in using the
> statistics for identifying passes that don't buy us much across a wide
> variety of codebases.  (Suggestions for suitable ones welcome!)

Yes, I used it exactly for that.  And also to verify that passes don't
do anything if replicated (well, for those that shouldn't at least).

Don't expect any low-hanging fruit though ;)  I catched all of it already.

Candidates are obviously SPEC and GCC itself.  I also use tramp3d
of course.  That said, even if a pass does nearly nothing we often
have testcases that need it ...

Richard.

> -Nathan
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-12 14:38     ` Richard Guenther
@ 2011-04-12 14:51       ` Nathan Froyd
  2011-04-12 14:54         ` Richard Guenther
  2011-04-12 15:01       ` Steven Bosscher
  1 sibling, 1 reply; 11+ messages in thread
From: Nathan Froyd @ 2011-04-12 14:51 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

On Tue, Apr 12, 2011 at 04:37:42PM +0200, Richard Guenther wrote:
> On Tue, Apr 12, 2011 at 4:32 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> > Thanks.  I may go twiddle that patch to do something similar to mine and
> > submit that.  Do you use your patch for checking that the same set of
> > optimizations get performed, then?  I'm interested in using the
> > statistics for identifying passes that don't buy us much across a wide
> > variety of codebases.  (Suggestions for suitable ones welcome!)
> 
> Yes, I used it exactly for that.  And also to verify that passes don't
> do anything if replicated (well, for those that shouldn't at least).
> 
> Don't expect any low-hanging fruit though ;)  I catched all of it already.
> 
> Candidates are obviously SPEC and GCC itself.  I also use tramp3d
> of course.  That said, even if a pass does nearly nothing we often
> have testcases that need it ...

True, but maybe those testcases should be adjusted--per-pass flags,
rather than blindly assuming -O2 includes them.  And it's not clear to
me that the statistics_counter_event infrastructure really helps
catching do-nothing passes, since it doesn't record stats that increment
by zero...

-Nathan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-12 14:51       ` Nathan Froyd
@ 2011-04-12 14:54         ` Richard Guenther
  2011-04-12 15:09           ` Nathan Froyd
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Guenther @ 2011-04-12 14:54 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches

On Tue, Apr 12, 2011 at 4:51 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> On Tue, Apr 12, 2011 at 04:37:42PM +0200, Richard Guenther wrote:
>> On Tue, Apr 12, 2011 at 4:32 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
>> > Thanks.  I may go twiddle that patch to do something similar to mine and
>> > submit that.  Do you use your patch for checking that the same set of
>> > optimizations get performed, then?  I'm interested in using the
>> > statistics for identifying passes that don't buy us much across a wide
>> > variety of codebases.  (Suggestions for suitable ones welcome!)
>>
>> Yes, I used it exactly for that.  And also to verify that passes don't
>> do anything if replicated (well, for those that shouldn't at least).
>>
>> Don't expect any low-hanging fruit though ;)  I catched all of it already.
>>
>> Candidates are obviously SPEC and GCC itself.  I also use tramp3d
>> of course.  That said, even if a pass does nearly nothing we often
>> have testcases that need it ...
>
> True, but maybe those testcases should be adjusted--per-pass flags,
> rather than blindly assuming -O2 includes them.  And it's not clear to

It's easier to add things to GCC than to argue removing things ...

> me that the statistics_counter_event infrastructure really helps
> catching do-nothing passes, since it doesn't record stats that increment
> by zero...

Well, if the overall count is zero then nothing was done.

Richard.

> -Nathan
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-12 14:38     ` Richard Guenther
  2011-04-12 14:51       ` Nathan Froyd
@ 2011-04-12 15:01       ` Steven Bosscher
  1 sibling, 0 replies; 11+ messages in thread
From: Steven Bosscher @ 2011-04-12 15:01 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Nathan Froyd, gcc-patches

On Tue, Apr 12, 2011 at 4:37 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> Yes, I used it exactly for that.  And also to verify that passes don't
> do anything if replicated (well, for those that shouldn't at least).

What about passes that undo the work of previous patches -- and then
followed by a patch that re-does the changes? Think CPROP ->
loop-invariant -> CPROP, and I'm sure there are other examples. That
kind of thing makes the statistics gathering a bit suspect...

Ciao!
Steven

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-12 14:54         ` Richard Guenther
@ 2011-04-12 15:09           ` Nathan Froyd
  2011-04-13  9:07             ` Richard Guenther
  0 siblings, 1 reply; 11+ messages in thread
From: Nathan Froyd @ 2011-04-12 15:09 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

On Tue, Apr 12, 2011 at 04:54:43PM +0200, Richard Guenther wrote:
> On Tue, Apr 12, 2011 at 4:51 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> > True, but maybe those testcases should be adjusted--per-pass flags,
> > rather than blindly assuming -O2 includes them.  And it's not clear to
> 
> It's easier to add things to GCC than to argue removing things ...

And sometimes not even easy to argue for adding things. :)

> > me that the statistics_counter_event infrastructure really helps
> > catching do-nothing passes, since it doesn't record stats that increment
> > by zero...
> 
> Well, if the overall count is zero then nothing was done.

Granted, but that fact should still be recorded.  The situation we have
today, for something like:

func1: statistic for "statx" was 0
  - nothing is recorded in the statistics table
func2: statistic for "statx" was 0
  - nothing is recorded in the statistics table
func3: statistic for "statx" was 0
  - nothing is recorded in the statistics table
...

and so forth, is that at the end of the day, the dump file won't even
include any information about "statx".  If you had some func7387 where
"statx" was non-zero, you could infer that nothing else happened in the
previous 7386 functions.  For the case where a pass is truly useless on
a TU, it's hard to figure out from the statistics dump alone.  And I'd
argue that it's useful to see explicitly that the pass only helped in 1
out of 7387 functions, rather than trying to infer it from missing data.

-Nathan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-12 15:09           ` Nathan Froyd
@ 2011-04-13  9:07             ` Richard Guenther
  2011-04-13 18:43               ` Nathan Froyd
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Guenther @ 2011-04-13  9:07 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches

On Tue, Apr 12, 2011 at 5:09 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> On Tue, Apr 12, 2011 at 04:54:43PM +0200, Richard Guenther wrote:
>> On Tue, Apr 12, 2011 at 4:51 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
>> > True, but maybe those testcases should be adjusted--per-pass flags,
>> > rather than blindly assuming -O2 includes them.  And it's not clear to
>>
>> It's easier to add things to GCC than to argue removing things ...
>
> And sometimes not even easy to argue for adding things. :)
>
>> > me that the statistics_counter_event infrastructure really helps
>> > catching do-nothing passes, since it doesn't record stats that increment
>> > by zero...
>>
>> Well, if the overall count is zero then nothing was done.
>
> Granted, but that fact should still be recorded.  The situation we have
> today, for something like:
>
> func1: statistic for "statx" was 0
>  - nothing is recorded in the statistics table
> func2: statistic for "statx" was 0
>  - nothing is recorded in the statistics table
> func3: statistic for "statx" was 0
>  - nothing is recorded in the statistics table
> ...
>
> and so forth, is that at the end of the day, the dump file won't even
> include any information about "statx".  If you had some func7387 where
> "statx" was non-zero, you could infer that nothing else happened in the
> previous 7386 functions.  For the case where a pass is truly useless on
> a TU, it's hard to figure out from the statistics dump alone.  And I'd
> argue that it's useful to see explicitly that the pass only helped in 1
> out of 7387 functions, rather than trying to infer it from missing data.

I always use statistics-stats (thus, overall stats, not per function).  The
per function ones omit zero counts during dumping on purpose
(to make the dump smaller).

Richard.

> -Nathan
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-13  9:07             ` Richard Guenther
@ 2011-04-13 18:43               ` Nathan Froyd
  2011-04-14  8:51                 ` Richard Guenther
  0 siblings, 1 reply; 11+ messages in thread
From: Nathan Froyd @ 2011-04-13 18:43 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

On Wed, Apr 13, 2011 at 11:07:15AM +0200, Richard Guenther wrote:
> On Tue, Apr 12, 2011 at 5:09 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> > Granted, but that fact should still be recorded.  The situation we have
> > today, for something like:
> >
> > func1: statistic for "statx" was 0
> >  - nothing is recorded in the statistics table
> > func2: statistic for "statx" was 0
> >  - nothing is recorded in the statistics table
> > func3: statistic for "statx" was 0
> >  - nothing is recorded in the statistics table
> > ...
> >
> > and so forth, is that at the end of the day, the dump file won't even
> > include any information about "statx".  If you had some func7387 where
> > "statx" was non-zero, you could infer that nothing else happened in the
> > previous 7386 functions.  For the case where a pass is truly useless on
> > a TU, it's hard to figure out from the statistics dump alone.  And I'd
> > argue that it's useful to see explicitly that the pass only helped in 1
> > out of 7387 functions, rather than trying to infer it from missing data.
> 
> I always use statistics-stats (thus, overall stats, not per function).  The
> per function ones omit zero counts during dumping on purpose
> (to make the dump smaller).

I didn't know about statistics-stats (or didn't realize that's what the
code was trying to do), that's useful.  And it looks like all the
statistics dumping things omit zero counts on purpose, not just the
per-function ones.

But that has no bearing on the point above: zero counts are not even
*recorded* today.  E.g. if you apply the patch upthread, grab a random C
file, compile it with -O2/3 -fdump-statistics/-stats, and examine the
dump file, you might not even know that new statistics counters have
been added.  Taking out the checks to avoid printing zero counts doesn't
help either, because the data simply doesn't get recorded.  This
infrastructure makes it somewhat difficult to figure out, in an
automated way from the dump file alone, whether passes are actually
doing anything.

Enough grousing.  I'm assuming turning on accumulation and dumping of
zero counts always would be frowned upon; would it be acceptable to turn
accumulation and dumping of zero counts if -details is given?

-Nathan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] add statistics counting to postreload, copy-rename, and math-opts
  2011-04-13 18:43               ` Nathan Froyd
@ 2011-04-14  8:51                 ` Richard Guenther
  0 siblings, 0 replies; 11+ messages in thread
From: Richard Guenther @ 2011-04-14  8:51 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches

On Wed, Apr 13, 2011 at 8:43 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> On Wed, Apr 13, 2011 at 11:07:15AM +0200, Richard Guenther wrote:
>> On Tue, Apr 12, 2011 at 5:09 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
>> > Granted, but that fact should still be recorded.  The situation we have
>> > today, for something like:
>> >
>> > func1: statistic for "statx" was 0
>> >  - nothing is recorded in the statistics table
>> > func2: statistic for "statx" was 0
>> >  - nothing is recorded in the statistics table
>> > func3: statistic for "statx" was 0
>> >  - nothing is recorded in the statistics table
>> > ...
>> >
>> > and so forth, is that at the end of the day, the dump file won't even
>> > include any information about "statx".  If you had some func7387 where
>> > "statx" was non-zero, you could infer that nothing else happened in the
>> > previous 7386 functions.  For the case where a pass is truly useless on
>> > a TU, it's hard to figure out from the statistics dump alone.  And I'd
>> > argue that it's useful to see explicitly that the pass only helped in 1
>> > out of 7387 functions, rather than trying to infer it from missing data.
>>
>> I always use statistics-stats (thus, overall stats, not per function).  The
>> per function ones omit zero counts during dumping on purpose
>> (to make the dump smaller).
>
> I didn't know about statistics-stats (or didn't realize that's what the
> code was trying to do), that's useful.  And it looks like all the
> statistics dumping things omit zero counts on purpose, not just the
> per-function ones.
>
> But that has no bearing on the point above: zero counts are not even
> *recorded* today.  E.g. if you apply the patch upthread, grab a random C
> file, compile it with -O2/3 -fdump-statistics/-stats, and examine the
> dump file, you might not even know that new statistics counters have
> been added.  Taking out the checks to avoid printing zero counts doesn't
> help either, because the data simply doesn't get recorded.  This
> infrastructure makes it somewhat difficult to figure out, in an
> automated way from the dump file alone, whether passes are actually
> doing anything.

Oh, ok - now I understand what you are saying.  Yes, that's by design
and hard to fix - counters are "registered" on their first bump.

> Enough grousing.  I'm assuming turning on accumulation and dumping of
> zero counts always would be frowned upon; would it be acceptable to turn
> accumulation and dumping of zero counts if -details is given?

Well, you'd have to fix the registration problem ...

But I don't see zero counts as very important, if not to verify that the
statistics code is placed at a good spot.

Richard.

> -Nathan
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2011-04-14  8:51 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-12 14:16 [PATCH] add statistics counting to postreload, copy-rename, and math-opts Nathan Froyd
2011-04-12 14:27 ` Richard Guenther
2011-04-12 14:32   ` Nathan Froyd
2011-04-12 14:38     ` Richard Guenther
2011-04-12 14:51       ` Nathan Froyd
2011-04-12 14:54         ` Richard Guenther
2011-04-12 15:09           ` Nathan Froyd
2011-04-13  9:07             ` Richard Guenther
2011-04-13 18:43               ` Nathan Froyd
2011-04-14  8:51                 ` Richard Guenther
2011-04-12 15:01       ` Steven Bosscher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).