Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(55)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 9630043: [google gcc-4_7] coverage callback instrumentation

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 9 months ago by xur
Modified:
11 years, 9 months ago
Reviewers:
davidxl
CC:
gcc-patches_gcc.gnu.org
Base URL:
svn+ssh://gcc.gnu.org/svn/gcc/branches/google/gcc-4_7/
Visibility:
Public.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+124 lines, -24 lines) Patch
M gcc/params.def View 1 chunk +10 lines, -0 lines 0 comments Download
M gcc/profile.c View 2 chunks +3 lines, -1 line 0 comments Download
M gcc/tree-profile.c View 11 chunks +102 lines, -22 lines 0 comments Download
M gcc/tree-ssa-uninit.c View 1 chunk +1 line, -1 line 0 comments Download
M libgcc/libgcov.c View 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 2
xur
This patch is to be used with customized coverage reduction. The functionalities are under two ...
11 years, 9 months ago (2013-05-21 22:28:30 UTC) #1
davidxl
11 years, 9 months ago (2013-05-22 16:07:01 UTC) #2 
Looks ok to me in general.
1) the parameter name is not ideal -- it is not callonce.
2) it might be better to extend the callonce parameter into
-ftest-coverage option such as -ftest-coverage=exec_once?
3) need documentation in invoke.texi
4) watch out for long lines.

cc Teresa.

David

On Tue, May 21, 2013 at 3:28 PM, Rong Xu <xur@google.com> wrote:
> This patch is to be used with customized coverage reduction.
>
> The functionalities are under two parameter options:
> --param=COVERAGE-CALLBACK={0|1}
> when enabled with 1, it injects a callback function for
> each arc counter. Default off.
> --param=COVERAGE-CALLONCE={0|1}
> when enabled with 1, it stops incrementing the arc counter
> when they flip to 1. Default off. Even with an extra condition
> check, this is drastically faster than the normal coverage test
> for highly threaded applications.
>
> It also fixes a bug in profile sampling where some counters
> (like those after the call stmt) are not sampled.
>
> It disables the may-uninit check if the warning will not be
> emitted.
>
> Tested with bootstrap, regressions test and google internal
> benchmarks.
>
> Thanks,
>
> -Rong
>
> 2013-05-21  Rong Xu  <xur@google.com>
>
>         * gcc/tree-ssa-uninit.c (gate_warn_uninitialized): Disable if
>         may-uninit warning will not emitted.
>         * gcc/params.def : (PARAM_COVERAGE_CALLBACK) New.
>         (PARAM_COVERAGE_CALLONCE): NEW.
>         * gcc/profile.c (branch_prob): Not increment edge counter after
>         first update.
>         (tree_init_instrumentation_sampling): Ditto.
>         (COVERAGE_CALLBACK_FUNC_NAME): New.
>         (COVERAGE_INSERT_CALL): New.
>         (insert_if_then): Update branch probability.
>         (add_sampling_wrapper): Ditto.
>         (add_callback_wrapper): New.
>         (add_sampling_to_edge_counters): Make predicate insertion
>         complete.
>         (gimple_gen_edge_profiler): Add coverage callback function.
>         (gimple_gen_ic_profiler): Fix trailing space.
>         (gimple_gen_ic_func_topn_profiler): Ditto.
>         (gimple_gen_dc_profiler): Ditto.
>         * libgcc/libgcov.c (__coverage_callback): Add an empty callback
>         function.
>
> Index: libgcc/libgcov.c
> ===================================================================
> --- libgcc/libgcov.c    (revision 198952)
> +++ libgcc/libgcov.c    (working copy)
> @@ -135,6 +135,14 @@ extern int gcov_dump_complete ATTRIBUTE_HIDDEN;
>     these symbols will always need to be resolved.  */
>  void (*__gcov_dummy_ref1)() = &__gcov_reset;
>  void (*__gcov_dummy_ref2)() = &__gcov_dump;
> +
> +__attribute__((weak)) void
> +__coverage_callback (gcov_type funcdef_no __attribute__ ((unused)),
> +                     int edge_no __attribute__ ((unused)))
> +{
> +   /* nothing */
> +}
> +
>  #endif /* __GCOV_KERNEL__ */
>
>  /* Utility function for outputing errors.  */
> Index: gcc/profile.c
> ===================================================================
> --- gcc/profile.c       (revision 198952)
> +++ gcc/profile.c       (working copy)
> @@ -69,6 +69,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "timevar.h"
>  #include "cfgloop.h"
>  #include "tree-pass.h"
> +#include "params.h"
>
>  #include "profile.h"
>
> @@ -1480,7 +1481,8 @@ branch_prob (void)
>        /* Commit changes done by instrumentation.  */
>        gsi_commit_edge_inserts ();
>
> -      if (flag_profile_generate_sampling)
> +      if (flag_profile_generate_sampling
> +          || PARAM_VALUE (PARAM_COVERAGE_CALLONCE))
>          add_sampling_to_edge_counters ();
>      }
>
> Index: gcc/params.def
> ===================================================================
> --- gcc/params.def      (revision 198952)
> +++ gcc/params.def      (working copy)
> @@ -1060,6 +1060,16 @@ DEFPARAM (PARAM_PROFILE_GENERATE_SAMPLING_PERIOD,
>           "sampling rate with -fprofile-generate-sampling",
>           100, 0, 2000000000)
>
> +DEFPARAM (PARAM_COVERAGE_CALLBACK,
> +         "coverage-callback",
> +         "callback a user-define function when for arc counter increments.",
> +         0, 0, 1)
> +
> +DEFPARAM (PARAM_COVERAGE_CALLONCE,
> +         "coverage-callonce",
> +         "Stop increment coverage counts once they become 1.",
> +         0, 0, 1)
> +
>  /* Used for debugging purpose. Tell the compiler to find
>     the gcda file in the current directory.  */
>  DEFPARAM (PARAM_GCOV_DEBUG,
> Index: gcc/tree-profile.c
> ===================================================================
> --- gcc/tree-profile.c  (revision 198952)
> +++ gcc/tree-profile.c  (working copy)
> @@ -54,6 +54,15 @@ along with GCC; see the file COPYING3.  If not see
>  #include "target.h"
>  #include "output.h"
>
> +/* Default name for coverage callback function.  */
> +#define COVERAGE_CALLBACK_FUNC_NAME "__coverage_callback"
> +
> +/* If we insert a callback to edge instrumentation code. Avoid this
> +   for the callback function itself.  */
> +#define COVERAGE_INSERT_CALL   ((PARAM_VALUE (PARAM_COVERAGE_CALLBACK) == 1)
\
> +                                 && strcmp (get_name (current_function_decl),
\
> +                                        COVERAGE_CALLBACK_FUNC_NAME))
> +
>  /* Number of statements inserted for each edge counter increment.  */
>  #define EDGE_COUNTER_STMT_COUNT 3
>
> @@ -202,14 +211,16 @@ static tree GTY(()) gcov_lipo_merge_modu_edges = N
>  static tree GTY(()) gcov_lipo_strict_inclusion = NULL_TREE;
>
>  /* Insert STMT_IF around given sequence of consecutive statements in the
> -   same basic block starting with STMT_START, ending with STMT_END.  */
> +   same basic block starting with STMT_START, ending with STMT_END.
> +   PROB is the probability of the taken branch.  */
>
>  static void
> -insert_if_then (gimple stmt_start, gimple stmt_end, gimple stmt_if)
> +insert_if_then (gimple stmt_start, gimple stmt_end, gimple stmt_if, int
prob)
>  {
>    gimple_stmt_iterator gsi;
>    basic_block bb_original, bb_before_if, bb_after_if;
> -  edge e_if_taken, e_then_join;
> +  edge e_if_taken, e_then_join, e_else;
> +  int orig_frequency;
>
>    gsi = gsi_for_stmt (stmt_start);
>    gsi_insert_before (&gsi, stmt_if, GSI_SAME_STMT);
> @@ -220,7 +231,11 @@ static void
>    e_then_join = split_block (e_if_taken->dest, stmt_end);
>    bb_before_if = e_if_taken->src;
>    bb_after_if = e_then_join->dest;
> -  make_edge (bb_before_if, bb_after_if, EDGE_FALSE_VALUE);
> +  e_else = make_edge (bb_before_if, bb_after_if, EDGE_FALSE_VALUE);
> +  orig_frequency = bb_original->frequency;
> +  e_if_taken->probability = prob;
> +  e_else->probability = REG_BR_PROB_BASE - prob;
> +  e_if_taken->dest->frequency = orig_frequency * (prob / REG_BR_PROB_BASE);
>  }
>
>  /* Transform:
> @@ -274,9 +289,34 @@ add_sampling_wrapper (gimple stmt_start, gimple st
>    gsi_insert_before (&gsi, stmt_reset_counter, GSI_SAME_STMT);
>
>    /* Insert IF block.  */
> -  insert_if_then (stmt_reset_counter, stmt_end, stmt_if);
> +  /* Sampling rate can be changed runtime: hard to guess the branch prob,
> +     so make it 1.  */
> +  insert_if_then (stmt_reset_counter, stmt_end, stmt_if, REG_BR_PROB_BASE);
>  }
>
> +static void
> +add_callonce_wrapper (gimple stmt_start, gimple stmt_end)
> +{
> +  tree zero, tmp_var, tmp1;
> +  gimple stmt_if, stmt_assign;
> +  gimple_stmt_iterator gsi;
> +
> +  /* Create all the new statements needed.  */
> +  tmp_var = create_tmp_reg (get_gcov_type (), "PROF_temp");
> +  tmp1 = make_ssa_name (tmp_var, NULL);
> +  stmt_assign = gimple_build_assign (tmp1, gimple_assign_lhs (stmt_end));
> +
> +  zero = build_int_cst (get_gcov_type (), 0);
> +  stmt_if = gimple_build_cond (EQ_EXPR, tmp1, zero, NULL_TREE, NULL_TREE);
> +  find_referenced_vars_in (stmt_if);
> +
> +  gsi = gsi_for_stmt (stmt_start);
> +  gsi_insert_before (&gsi, stmt_assign, GSI_SAME_STMT);
> +
> +  /* Insert IF block.  */
> +  insert_if_then (stmt_start, stmt_end, stmt_if, 1);
> +}
> +
>  /* Return whether STMT is the beginning of an instrumentation block to be
>     applied sampling.  */
>
> @@ -295,22 +335,34 @@ add_sampling_to_edge_counters (void)
>    basic_block bb;
>
>    FOR_EACH_BB_REVERSE (bb)
> -    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> +    for (gsi = gsi_last (bb_seq (bb)); !gsi_end_p (gsi); gsi_prev (&gsi))
>        {
> -        gimple stmt = gsi_stmt (gsi);
> -        if (is_instrumentation_to_be_sampled (stmt))
> +        gimple stmt_end = gsi_stmt (gsi);
> +        if (is_instrumentation_to_be_sampled (stmt_end))
>            {
> -            gimple stmt_end;
> +            gimple stmt_beg;
>              int i;
> +            int edge_counter_stmt_count = EDGE_COUNTER_STMT_COUNT;
> +
>              /* The code for edge counter increment has
EDGE_COUNTER_STMT_COUNT
>                 gimple statements. Advance that many statements to find the
> -               last statement.  */
> -            for (i = 0; i < EDGE_COUNTER_STMT_COUNT - 1; i++)
> -              gsi_next (&gsi);
> -            stmt_end = gsi_stmt (gsi);
> -            gcc_assert (stmt_end);
> -            add_sampling_wrapper (stmt, stmt_end);
> -            break;
> +               beginning statement.  */
> +            if (COVERAGE_INSERT_CALL)
> +              edge_counter_stmt_count++;
> +
> +            for (i = 0; i < edge_counter_stmt_count - 1; i++)
> +              gsi_prev (&gsi);
> +            stmt_beg = gsi_stmt (gsi);
> +            gcc_assert (stmt_beg);
> +
> +
> +            if (flag_profile_generate_sampling)
> +              add_sampling_wrapper (stmt_beg, stmt_end);
> +            if (PARAM_VALUE (PARAM_COVERAGE_CALLONCE))
> +              add_callonce_wrapper (stmt_beg, stmt_end);
> +
> +            /* reset the iterator and continue.  */
> +            gsi = gsi_last (bb_seq (bb));
>            }
>        }
>  }
> @@ -508,6 +560,9 @@ tree_init_instrumentation_sampling (void)
>          DECL_TLS_MODEL (gcov_sample_counter_decl) =
>              decl_default_tls_model (gcov_sample_counter_decl);
>      }
> +  if (PARAM_VALUE (PARAM_COVERAGE_CALLONCE)
> +      && instrumentation_to_be_sampled == 0)
> +    instrumentation_to_be_sampled = pointer_set_create ();
>  }
>
>  void
> @@ -675,6 +730,30 @@ gimple_gen_edge_profiler (int edgeno, edge e)
>      ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
>
>    one = build_int_cst (gcov_type_node, 1);
> +
> +  /* insert a callback stmt stmt */
> +  if (COVERAGE_INSERT_CALL)
> +    {
> +      gimple call;
> +      tree tree_edgeno = build_int_cst (gcov_type_node, edgeno);
> +      tree tree_uid = build_int_cst (gcov_type_node,
current_function_funcdef_no);
> +      tree callback_fn_type
> +              = build_function_type_list (void_type_node,
> +                                          gcov_type_node,
> +                                          integer_type_node,
> +                                          NULL_TREE);
> +      tree tree_callback_fn = build_fn_decl (COVERAGE_CALLBACK_FUNC_NAME,
> +                                             callback_fn_type);
> +      TREE_NOTHROW (tree_callback_fn) = 1;
> +      DECL_ATTRIBUTES (tree_callback_fn)
> +        = tree_cons (get_identifier ("leaf"), NULL,
> +                     DECL_ATTRIBUTES (tree_callback_fn));
> +
> +      call = gimple_build_call (tree_callback_fn, 2, tree_uid, tree_edgeno);
> +      find_referenced_vars_in (call);
> +      gsi_insert_on_edge(e, call);
> +    }
> +
>    if (PROFILE_GEN_EDGE_ATOMIC)
>      {
>        /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
> @@ -695,13 +774,14 @@ gimple_gen_edge_profiler (int edgeno, edge e)
>        gimple_assign_set_lhs (stmt2, make_ssa_name (gcov_type_tmp_var,
stmt2));
>        stmt3 = gimple_build_assign (unshare_expr (ref), gimple_assign_lhs
(stmt2));
>
> -      if (flag_profile_generate_sampling)
> -        pointer_set_insert (instrumentation_to_be_sampled, stmt1);
> -
>        gsi_insert_on_edge (e, stmt1);
>        gsi_insert_on_edge (e, stmt2);
>      }
>    gsi_insert_on_edge (e, stmt3);
> +
> +  if (flag_profile_generate_sampling
> +      || PARAM_VALUE (PARAM_COVERAGE_CALLONCE))
> +    pointer_set_insert (instrumentation_to_be_sampled, stmt3);
>  }
>
>  /* Emits code to get VALUE to instrument at GSI, and returns the
> @@ -802,7 +882,7 @@ gimple_gen_ic_profiler (histogram_value value, uns
>    gimple stmt;
>    gimple_stmt_iterator gsi;
>    tree ref_ptr;
> -
> +
>    stmt = value->hvalue.stmt;
>    gsi = gsi_for_stmt (stmt);
>    ref_ptr = tree_coverage_counter_addr (tag, base);
> @@ -905,7 +985,7 @@ gimple_gen_ic_func_topn_profiler (void)
>    gcov_info = build_fold_addr_expr (gcov_info_decl);
>    cur_func_id = build_int_cst (get_gcov_unsigned_t (),
>                                FUNC_DECL_FUNC_ID (cfun));
> -  stmt1 = gimple_build_call (tree_indirect_call_topn_profiler_fn,
> +  stmt1 = gimple_build_call (tree_indirect_call_topn_profiler_fn,
>                              3, cur_func, gcov_info, cur_func_id);
>    gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT);
>  }
> @@ -922,7 +1002,7 @@ gimple_gen_dc_profiler (unsigned base, gimple call
>    gimple stmt1, stmt2, stmt3;
>    gimple_stmt_iterator gsi = gsi_for_stmt (call_stmt);
>    tree tmp1, tmp2, tmp3, callee = gimple_call_fn (call_stmt);
> -
> +
>    /* Insert code:
>       __gcov_direct_call_counters = get_relevant_counter_ptr ();
>       __gcov_callee = (void *) callee;
> Index: gcc/tree-ssa-uninit.c
> ===================================================================
> --- gcc/tree-ssa-uninit.c       (revision 198952)
> +++ gcc/tree-ssa-uninit.c       (working copy)
> @@ -2033,7 +2033,7 @@ execute_late_warn_uninitialized (void)
>  static bool
>  gate_warn_uninitialized (void)
>  {
> -  return warn_uninitialized != 0;
> +  return (warn_uninitialized != 0 && warn_maybe_uninitialized != 0);
>  }
>
>  struct gimple_opt_pass pass_late_warn_uninitialized =
>
> --
> This patch is available for review at http://codereview.appspot.com/9630043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b