On Wed, Dec 19, 2012 at 4:29 PM, Andrew Pinski <pins...@gmail.com> wrote:
>
> On Wed, Dec 19, 2012 at 12:08 PM, Rong Xu <x...@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 <x...@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
