[google 4.7] atomic update of profile counters (issue6965050)

Rong Xu xur@google.com
Thu Dec 20 00:56:00 GMT 2012


On Wed, Dec 19, 2012 at 4:29 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Wed, Dec 19, 2012 at 12:08 PM, Rong Xu <xur@google.com> wrote:
> > Hi,
> >
> > This patch adds the supprot of atomic update the profile counters.
> > Tested with google internal benchmarks and fdo kernel build.
>
> I think you should use the __atomic_ functions instead of __sync_
> functions as they allow better performance for simple counters as you
> can use __ATOMIC_RELAXED.

You are right. I think __ATOMIC_RELAXED should be OK here.
Thanks for the suggestion.

>
> And this would be useful for the trunk also.  I was going to implement
> this exact thing this week but some other important stuff came up.

I'll post trunk patch later.

>
> Thanks,
> Andrew Pinski
>
>
> >
> > Thanks,
> >
> > -Rong
> >
> > 2012-12-19  Rong Xu  <xur@google.com>
> >
> >         * gcc/common.opt: Add -fprofile-gen-atomic option.
> >         * gcc/gcov-io.h: Add profile atomic update support.
> >         * gcc/tree-profile.c (gimple_init_edge_profiler): Ditto.
> >         (gimple_gen_edge_profiler): Ditto.
> >         * libgcc/libgcov.c (__gcov_one_value_profiler_body): Ditto.
> >         (__gcov_one_value_profiler_body_atomic): Ditto.
> >         (__gcov_one_value_profiler_atomic): Ditto.
> >         (__gcov_indirect_call_profiler_atomic): Ditto.
> >
> > Index: gcc/common.opt
> > ===================================================================
> > --- gcc/common.opt      (revision 194562)
> > +++ gcc/common.opt      (working copy)
> > @@ -1754,6 +1754,15 @@ fprofile-dump
> >  Common Report Var(flag_profile_dump) Init(0) Optimization
> >  Dump CFG profile for comparison.
> >
> > +; fprofile-gen-atomic=0: disable aotimically update.
> > +; fprofile-gen-atomic=1: aotimically update edge profile counters.
> > +; fprofile-gen-atomic=2: aotimically update value profile counters.
> > +; fprofile-gen-atomic=3: aotimically update edge and value profile counters.
> > +; other values will be ignored (fall back to the default of 0).
> > +fprofile-gen-atomic=
> > +Common Joined UInteger Report Var(flag_profile_gen_atomic) Init(0) Optimization
> > +fprofile-gen-atomic=[0..3] Atomically increments for profile counters.
> > +
> >  fprofile-generate
> >  Common
> >  Enable common options for generating profile info for profile feedback directed optimizations
> > Index: gcc/gcov-io.h
> > ===================================================================
> > --- gcc/gcov-io.h       (revision 194562)
> > +++ gcc/gcov-io.h       (working copy)
> > @@ -300,6 +300,14 @@ typedef unsigned gcov_type_unsigned __attribute__
> >
> >  #endif  /* BITS_PER_UNIT == 8  */
> >
> > +#if LONG_LONG_TYPE_SIZE > 32
> > +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_8
> > +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_8
> > +#else
> > +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_4
> > +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_4
> > +#endif
> > +
> >  #undef EXTRACT_MODULE_ID_FROM_GLOBAL_ID
> >  #undef EXTRACT_FUNC_ID_FROM_GLOBAL_ID
> >  #undef GEN_FUNC_GLOBAL_ID
> > @@ -322,6 +330,18 @@ typedef unsigned gcov_type_unsigned __attribute__
> >  typedef unsigned gcov_unsigned_t;
> >  typedef unsigned gcov_position_t;
> >
> > +#if LONG_LONG_TYPE_SIZE > 32
> > +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_8
> > +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_8
> > +#else
> > +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_4
> > +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_4
> > +#endif
> > +#define PROFILE_GEN_EDGE_ATOMIC (flag_profile_gen_atomic == 1 || \
> > +                                 flag_profile_gen_atomic == 3)
> > +#define PROFILE_GEN_VALUE_ATOMIC (flag_profile_gen_atomic == 2 || \
> > +                                  flag_profile_gen_atomic == 3)
> > +
> >  /* gcov_type is typedef'd elsewhere for the compiler */
> >  #if IN_GCOV
> >  #define GCOV_LINKAGE static
> > Index: gcc/tree-profile.c
> > ===================================================================
> > --- gcc/tree-profile.c  (revision 194562)
> > +++ gcc/tree-profile.c  (working copy)
> > @@ -471,7 +471,12 @@ gimple_init_edge_profiler (void)
> >               = build_function_type_list (void_type_node,
> >                                           gcov_type_ptr, gcov_type_node,
> >                                           NULL_TREE);
> > -      tree_one_value_profiler_fn
> > +      if (PROFILE_GEN_VALUE_ATOMIC)
> > +        tree_one_value_profiler_fn
> > +             = build_fn_decl ("__gcov_one_value_profiler_atomic",
> > +                                    one_value_profiler_fn_type);
> > +      else
> > +        tree_one_value_profiler_fn
> >               = build_fn_decl ("__gcov_one_value_profiler",
> >                                      one_value_profiler_fn_type);
> >        TREE_NOTHROW (tree_one_value_profiler_fn) = 1;
> > @@ -487,7 +492,12 @@ gimple_init_edge_profiler (void)
> >                                        gcov_type_ptr, gcov_type_node,
> >                                        ptr_void,
> >                                        ptr_void, NULL_TREE);
> > -      tree_indirect_call_profiler_fn
> > +      if (PROFILE_GEN_VALUE_ATOMIC)
> > +        tree_indirect_call_profiler_fn
> > +             = build_fn_decl ("__gcov_indirect_call_profiler_atomic",
> > +                                    ic_profiler_fn_type);
> > +      else
> > +        tree_indirect_call_profiler_fn
> >               = build_fn_decl ("__gcov_indirect_call_profiler",
> >                                      ic_profiler_fn_type);
> >        TREE_NOTHROW (tree_indirect_call_profiler_fn) = 1;
> > @@ -563,21 +573,37 @@ gimple_gen_edge_profiler (int edgeno, edge e)
> >       gets re-set in tree_profiling.  */
> >    if (gcov_type_tmp_var == NULL_TREE)
> >      gcov_type_tmp_var = create_tmp_reg (gcov_type_node, "PROF_edge_counter");
> > -  ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
> > +
> > +  if (PROFILE_GEN_EDGE_ATOMIC)
> > +    ref = tree_coverage_counter_addr (GCOV_COUNTER_ARCS, edgeno);
> > +  else
> > +    ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
> > +
> >    one = build_int_cst (gcov_type_node, 1);
> > -  stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
> > -  gimple_assign_set_lhs (stmt1, make_ssa_name (gcov_type_tmp_var, stmt1));
> > -  find_referenced_vars_in (stmt1);
> > -  stmt2 = gimple_build_assign_with_ops (PLUS_EXPR, gcov_type_tmp_var,
> > -                                       gimple_assign_lhs (stmt1), one);
> > -  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 (PROFILE_GEN_EDGE_ATOMIC)
> > +    {
> > +      /* __sync_fetch_and_add_8 (&counter, 1); */
> > +      stmt3 = gimple_build_call (builtin_decl_explicit
> > +                                    (GCOV_TYPE_SYNC_FETCH_AND_ADD),
> > +                                 2, ref, one);
> > +      find_referenced_vars_in (stmt3);
> > +    }
> > +  else
> > +    {
> > +      stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
> > +      gimple_assign_set_lhs (stmt1, make_ssa_name (gcov_type_tmp_var, stmt1));
> > +      find_referenced_vars_in (stmt1);
> > +      stmt2 = gimple_build_assign_with_ops (PLUS_EXPR, gcov_type_tmp_var,
> > +                                       gimple_assign_lhs (stmt1), one);
> > +      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);
> > +      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, stmt1);
> > +      gsi_insert_on_edge (e, stmt2);
> > +    }
> >    gsi_insert_on_edge (e, stmt3);
> >  }
> >
> > Index: libgcc/libgcov.c
> > ===================================================================
> > --- libgcc/libgcov.c    (revision 194562)
> > +++ libgcc/libgcov.c    (working copy)
> > @@ -1632,6 +1632,22 @@ __gcov_one_value_profiler_body (gcov_type *counter
> >    counters[2]++;
> >  }
> >
> > +/* Atomic update version of __gcov_one_value_profile_body().  */
> > +static inline void
> > +__gcov_one_value_profiler_body_atomic (gcov_type *counters, gcov_type value)
> > +{
> > +  if (value == counters[0])
> > +    GCOV_TYPE_SYNC_FETCH_AND_ADD_FN (&counters[1], 1);
> > +  else if (counters[1] == 0)
> > +    {
> > +      counters[1] = 1;
> > +      counters[0] = value;
> > +    }
> > +  else
> > +    GCOV_TYPE_SYNC_FETCH_AND_ADD_FN (&counters[1], -1);
> > +  GCOV_TYPE_SYNC_FETCH_AND_ADD_FN (&counters[2], 1);
> > +}
> > +
> >  #ifdef L_gcov_indirect_call_topn_profiler
> >  /* Tries to keep track the most frequent N values in the counters where
> >     N is specified by parameter TOPN_VAL. To track top N values, 2*N counter
> > @@ -1740,6 +1756,12 @@ __gcov_one_value_profiler (gcov_type *counters, gc
> >  {
> >    __gcov_one_value_profiler_body (counters, value);
> >  }
> > +
> > +void
> > +__gcov_one_value_profiler_atomic (gcov_type *counters, gcov_type value)
> > +{
> > +  __gcov_one_value_profiler_body_atomic (counters, value);
> > +}
> >  #endif
> >
> >  #ifdef L_gcov_indirect_call_profiler
> > @@ -1774,6 +1796,17 @@ __gcov_indirect_call_profiler (gcov_type* counter,
> >           && *(void **) cur_func == *(void **) callee_func))
> >      __gcov_one_value_profiler_body (counter, value);
> >  }
> > +
> > +/* Atomic update version of __gcov_indirect_call_profiler().  */
> > +void
> > +__gcov_indirect_call_profiler_atomic (gcov_type* counter, gcov_type value,
> > +                                      void* cur_func, void* callee_func)
> > +{
> > +  if (cur_func == callee_func
> > +      || (VTABLE_USES_DESCRIPTORS && callee_func
> > +          && *(void **) cur_func == *(void **) callee_func))
> > +    __gcov_one_value_profiler_body_atomic (counter, value);
> > +}
> >  #endif
> >
> >
> > @@ -2089,9 +2122,11 @@ EXPORT_SYMBOL (__gcov_merge_reusedist);
> >
> >  EXPORT_SYMBOL (__gcov_average_profiler);
> >  EXPORT_SYMBOL (__gcov_indirect_call_profiler);
> > +EXPORT_SYMBOL (__gcov_indirect_call_profiler_atomic);
> >  EXPORT_SYMBOL (__gcov_interval_profiler);
> >  EXPORT_SYMBOL (__gcov_ior_profiler);
> >  EXPORT_SYMBOL (__gcov_one_value_profiler);
> > +EXPORT_SYMBOL (__gcov_one_value_profiler_atomic);
> >  EXPORT_SYMBOL (__gcov_pow2_profiler);
> >
> >  #endif /* __GCOV_KERNEL__ */
> >
> > --
> > This patch is available for review at http://codereview.appspot.com/6965050



More information about the Gcc-patches mailing list