Re: [PATCH 1/4] Cherry-pick fprofile-generate-atomic from google/gcc-4_9 branch

[Nathan Sidwell]

On 08/01/16 09:29, Martin Liška wrote:

I also added a small hunk that describes problematic of app having not-joined 
(or detached) threads,
can you please take a look at documentation change, maybe it would need some 
transformation?

sorry for the tady response,thanks for the ping.

In general good.   Some nits:


+++ b/gcc/tree-profile.c
@@ -164,7 +164,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 (flag_profile_update == PROFILE_UPDATE_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);

this hunk uses a different idiom to ...

@@ -180,11 +185,14 @@ gimple_init_edge_profiler (void)
                                          gcov_type_node,
                                          ptr_void,
                                          NULL_TREE);
+      const char *profiler_fn_name = "__gcov_indirect_call_profiler_v2";
+      if (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE))
+       profiler_fn_name = "__gcov_indirect_call_topn_profiler";
+      if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
+       profiler_fn_name = "__gcov_indirect_call_profiler_v2_atomic";

I prefer the latter's approach.

@@ -241,22 +249,37 @@ gimple_init_edge_profiler (void)
 void
 gimple_gen_edge_profiler (int edgeno, edge e)
...
+  else
+    {
/* COMMENT thread unsafe sequence */
+      tree ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);


diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
+static inline void
+__gcov_one_value_profiler_body_atomic (gcov_type *counters, gcov_type value)
+{
...

The body looks to have data races.  Some kind of cmp_store needed on 
counters[1]?  Maybe it can't be completely race free?

nathan