[PATCH] profile feedback: -fprofile-use= and -fprofile-correction, correctness fixes and option semantic changes.

Jan Hubicka hubicka@ucw.cz
Sun Mar 30 09:50:00 GMT 2008


Hi,
so now about the other patch.
> diff -r 986583ed535e gcc/common.opt
> --- a/gcc/common.opt	Wed Mar 19 14:42:54 2008 -0700
> +++ b/gcc/common.opt	Thu Mar 20 10:01:56 2008 -0700
> @@ -811,6 +811,10 @@ Common Report Var(profile_arc_flag)
>  Common Report Var(profile_arc_flag)
>  Insert arc-based program profiling code
>  
> +fprofile-correction
> +Common Report Var(flag_profile_correction)
> +Enable correction of flow inconsistent profile data input

You are missing texinfo docs here.  I would add few words that this
option is meant to help ignore race conditions when profiling threaded
programs.  Once Michael's atomic counters is in, we could have the
options cross referenced.

Also could you please add symmetric option to GCOV implementation?
> +/* Make the block/edge counts of the entire CFG flow-consistent.
> +   This is one super hack.  The problem is difficult to solve well
> +   but we rely on compute_all_counts()
> +   to leave no negative number and repeatedly apply the backward propagation
> +   of counts, until everything's consistent.  */

We can either have this hack, or we can simply disable the optimization
and emit counters on each edge as done by some compilers...  I guess
experience will show how much off the result can be and how much we
care.
> @@ -335,6 +338,16 @@ tree_gen_ic_func_profiler (void)
>  			       cur_func,
>  			       ic_void_ptr_var);
>        bsi_insert_after (&bsi, stmt1, BSI_NEW_STMT);
> +
> +      gcc_assert (EDGE_COUNT (bb->succs) == 1);
> +      bb = split_edge (EDGE_I (bb->succs, 0));
> +      bsi = bsi_start (bb);
> +      /* Set __gcov_indirect_call_callee to 0,
> +         so that calls from other modules won't get misattributed
> +	 to the last caller of the current callee. */
> +      void0 = build_int_cst (build_pointer_type (void_type_node), 0);
> +      stmt2 = build_gimple_modify_stmt (ic_void_ptr_var, void0);
> +      bsi_insert_after (&bsi, stmt2, BSI_NEW_STMT);

This is unrealted, right?  Please put it in as separate patch. It is OK.
> diff -r 986583ed535e gcc/tree-ssa-propagate.c
> --- a/gcc/tree-ssa-propagate.c	Wed Mar 19 14:42:54 2008 -0700
> +++ b/gcc/tree-ssa-propagate.c	Thu Mar 20 10:01:56 2008 -0700
> @@ -40,6 +40,7 @@
>  #include "langhooks.h"
>  #include "varray.h"
>  #include "vec.h"
> +#include "value-prof.h"
>  
>  /* This file implements a generic value propagation engine based on
>     the same propagation used by the SSA-CCP algorithm [1].
> @@ -734,6 +735,7 @@ set_rhs (tree *stmt_p, tree expr)
>        /* Replace the whole statement with EXPR.  If EXPR has no side
>  	 effects, then replace *STMT_P with an empty statement.  */
>        ann = stmt_ann (stmt);
> +      gimple_remove_stmt_histograms (cfun, stmt);

What are you shooting for here?
I think arbitrary replacement is not invalidating the histogram, in most
propagation algorithms it will be the same value, just computed
differently.    The histograms can be attached to the values in LHS or
RHS, so we probably would need to do some more cureful work.

Otherwise OK, but please post patch with gcov change too.

Honza



More information about the Gcc-patches mailing list