atomic update of profile counters (issue7000044)

atomic update of profile counters (issue7000044)

[Rong Xu]

Hi,

This patch adds support of atomic update of profiles counters. The goal is to improve
the poor counter values for highly thread programs. 

The atomic update is under a new option -fprofile-gen-atomic=<N>
N=0: default, no atomic update
N=1: atomic update edge counters.
N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
N=3: both edge counter and the above value profile counters.
Other value: fall back to the default.

This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
the indirect-call profile is the most relevant one here.

Test with bootstrap.

Comments and suggestions are welcomed.

Thanks,

-Rong


2012-12-20  Rong Xu  <xur@google.com>

	* libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
        function. Atomic update profile counters.
	(__gcov_one_value_profiler_atomic): Ditto.
	(__gcov_indirect_call_profiler_atomic): Ditto.
	* gcc/gcov-io.h: Macros for atomic update.
	* gcc/common.opt: New option.
	* gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
        update profile counters.
	(gimple_gen_edge_profiler): Ditto.

Index: libgcc/libgcov.c
===================================================================
--- libgcc/libgcov.c	(revision 194652)
+++ libgcc/libgcov.c	(working copy)
@@ -1113,12 +1113,35 @@ __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_ATOMIC_FETCH_ADD_FN (&counters[1], 1, MEMMODEL_RELAXED);
+  else if (counters[1] == 0)
+    {    
+      counters[1] = 1; 
+      counters[0] = value;
+    }    
+  else 
+    GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[1], -1, MEMMODEL_RELAXED);
+  GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[2], 1, MEMMODEL_RELAXED);
+}
+
 #ifdef L_gcov_one_value_profiler
 void
 __gcov_one_value_profiler (gcov_type *counters, gcov_type value)
 {
   __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
@@ -1153,6 +1176,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
 
 
Index: gcc/gcov-io.h
===================================================================
--- gcc/gcov-io.h	(revision 194652)
+++ gcc/gcov-io.h	(working copy)
@@ -202,7 +202,15 @@ typedef unsigned gcov_type_unsigned __attribute__
 #endif
 #endif
 
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
+#else
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
+#endif
 
+
 #if defined (TARGET_POSIX_IO)
 #define GCOV_LOCKED 1
 #else
@@ -212,6 +220,18 @@ typedef unsigned gcov_type_unsigned __attribute__
 #else /* !IN_LIBGCOV */
 /* About the host */
 
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
+#else
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_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)
+
 typedef unsigned gcov_unsigned_t;
 typedef unsigned gcov_position_t;
 /* gcov_type is typedef'd elsewhere for the compiler */
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 194652)
+++ gcc/common.opt	(working copy)
@@ -1635,6 +1635,15 @@ fprofile-correction
 Common Report Var(flag_profile_correction)
 Enable correction of flow inconsistent profile data input
 
+; fprofile-gen-atomic=0: disable atomically update.
+; fprofile-gen-atomic=1: atomically update edge profile counters.
+; fprofile-gen-atomic=2: atomically update value profile counters.
+; fprofile-gen-atomic=3: atomically 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/tree-profile.c
===================================================================
--- gcc/tree-profile.c	(revision 194652)
+++ gcc/tree-profile.c	(working copy)
@@ -147,7 +147,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;
@@ -163,9 +168,14 @@ gimple_init_edge_profiler (void)
 					  gcov_type_ptr, gcov_type_node,
 					  ptr_void,
 					  ptr_void, NULL_TREE);
-      tree_indirect_call_profiler_fn
-	      = build_fn_decl ("__gcov_indirect_call_profiler",
-				     ic_profiler_fn_type);
+      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;
       DECL_ATTRIBUTES (tree_indirect_call_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
@@ -211,8 +221,21 @@ gimple_gen_edge_profiler (int edgeno, edge e)
   tree ref, one, gcov_type_tmp_var;
   gimple stmt1, stmt2, stmt3;
 
+  one = build_int_cst (gcov_type_node, 1);
+  if (PROFILE_GEN_EDGE_ATOMIC)
+    {
+      ref = tree_coverage_counter_addr (GCOV_COUNTER_ARCS, edgeno);
+      /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
+      stmt1 = gimple_build_call (builtin_decl_explicit (
+                                   GCOV_TYPE_ATOMIC_FETCH_ADD),
+                                 3, ref, one,
+                                 build_int_cst (integer_type_node,
+                                   MEMMODEL_RELAXED));
+     gsi_insert_on_edge (e, stmt1);
+     return;
+    }
+
   ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
-  one = build_int_cst (gcov_type_node, 1);
   gcov_type_tmp_var = make_temp_ssa_name (gcov_type_node,
 					  NULL, "PROF_edge_counter");
   stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);

--
This patch is available for review at http://codereview.appspot.com/7000044

Re: atomic update of profile counters (issue7000044)

[Jan Hubicka]

> Hi,
> 
> This patch adds support of atomic update of profiles counters. The goal is to improve
> the poor counter values for highly thread programs. 
> 
> The atomic update is under a new option -fprofile-gen-atomic=<N>
> N=0: default, no atomic update
> N=1: atomic update edge counters.
> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
> N=3: both edge counter and the above value profile counters.
> Other value: fall back to the default.
> 
> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
> the indirect-call profile is the most relevant one here.
> 
> Test with bootstrap.
> 
> Comments and suggestions are welcomed.
> 
> Thanks,
> 
> -Rong
> 
> 
> 2012-12-20  Rong Xu  <xur@google.com>
> 
> 	* libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>         function. Atomic update profile counters.
> 	(__gcov_one_value_profiler_atomic): Ditto.
> 	(__gcov_indirect_call_profiler_atomic): Ditto.
> 	* gcc/gcov-io.h: Macros for atomic update.
> 	* gcc/common.opt: New option.
> 	* gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>         update profile counters.
> 	(gimple_gen_edge_profiler): Ditto.

The patch looks resonable.  Eventually we probably should provide rest of the value counters
in thread safe manner.  What happens on targets not having atomic operations?

Honza

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

On Fri, Dec 21, 2012 at 1:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Hi,
>>
>> This patch adds support of atomic update of profiles counters. The goal is to improve
>> the poor counter values for highly thread programs.
>>
>> The atomic update is under a new option -fprofile-gen-atomic=<N>
>> N=0: default, no atomic update
>> N=1: atomic update edge counters.
>> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
>> N=3: both edge counter and the above value profile counters.
>> Other value: fall back to the default.
>>
>> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
>> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
>> the indirect-call profile is the most relevant one here.
>>
>> Test with bootstrap.
>>
>> Comments and suggestions are welcomed.
>>
>> Thanks,
>>
>> -Rong
>>
>>
>> 2012-12-20  Rong Xu  <xur@google.com>
>>
>>       * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>>         function. Atomic update profile counters.
>>       (__gcov_one_value_profiler_atomic): Ditto.
>>       (__gcov_indirect_call_profiler_atomic): Ditto.
>>       * gcc/gcov-io.h: Macros for atomic update.
>>       * gcc/common.opt: New option.
>>       * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>>         update profile counters.
>>       (gimple_gen_edge_profiler): Ditto.
>
> The patch looks resonable.  Eventually we probably should provide rest of the value counters
> in thread safe manner.  What happens on targets not having atomic operations?

From http://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins,
it says:
      "If a particular operation cannot be implemented on the target
processor, a warning is generated and a call an external function is
generated. "

So I think there will be a warning and eventually a link error of unsat.

Thanks,

-Rong


>
> Honza

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

Hi Honza,

In the other thread of discussion (similar patch in google-4_7
branch), you said you were thinking if to let this patch into trunk in
stage 3. Can you give some update?

Thanks,

-Rong

On Fri, Dec 21, 2012 at 10:37 AM, Rong Xu <xur@google.com> wrote:
> On Fri, Dec 21, 2012 at 1:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> Hi,
>>>
>>> This patch adds support of atomic update of profiles counters. The goal is to improve
>>> the poor counter values for highly thread programs.
>>>
>>> The atomic update is under a new option -fprofile-gen-atomic=<N>
>>> N=0: default, no atomic update
>>> N=1: atomic update edge counters.
>>> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
>>> N=3: both edge counter and the above value profile counters.
>>> Other value: fall back to the default.
>>>
>>> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
>>> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
>>> the indirect-call profile is the most relevant one here.
>>>
>>> Test with bootstrap.
>>>
>>> Comments and suggestions are welcomed.
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>>
>>> 2012-12-20  Rong Xu  <xur@google.com>
>>>
>>>       * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>>>         function. Atomic update profile counters.
>>>       (__gcov_one_value_profiler_atomic): Ditto.
>>>       (__gcov_indirect_call_profiler_atomic): Ditto.
>>>       * gcc/gcov-io.h: Macros for atomic update.
>>>       * gcc/common.opt: New option.
>>>       * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>>>         update profile counters.
>>>       (gimple_gen_edge_profiler): Ditto.
>>
>> The patch looks resonable.  Eventually we probably should provide rest of the value counters
>> in thread safe manner.  What happens on targets not having atomic operations?
>
> From http://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins,
> it says:
>       "If a particular operation cannot be implemented on the target
> processor, a warning is generated and a call an external function is
> generated. "
>
> So I think there will be a warning and eventually a link error of unsat.
>
> Thanks,
>
> -Rong
>
>
>>
>> Honza

Re: atomic update of profile counters (issue7000044)

[Xinliang David Li]

It would be great if this can make into gcc4.8. The patch has close to
0 impact on code stability.

David

On Fri, Dec 28, 2012 at 11:32 AM, Rong Xu <xur@google.com> wrote:
> Hi Honza,
>
> In the other thread of discussion (similar patch in google-4_7
> branch), you said you were thinking if to let this patch into trunk in
> stage 3. Can you give some update?
>
> Thanks,
>
> -Rong
>
> On Fri, Dec 21, 2012 at 10:37 AM, Rong Xu <xur@google.com> wrote:
>> On Fri, Dec 21, 2012 at 1:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>> Hi,
>>>>
>>>> This patch adds support of atomic update of profiles counters. The goal is to improve
>>>> the poor counter values for highly thread programs.
>>>>
>>>> The atomic update is under a new option -fprofile-gen-atomic=<N>
>>>> N=0: default, no atomic update
>>>> N=1: atomic update edge counters.
>>>> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
>>>> N=3: both edge counter and the above value profile counters.
>>>> Other value: fall back to the default.
>>>>
>>>> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
>>>> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
>>>> the indirect-call profile is the most relevant one here.
>>>>
>>>> Test with bootstrap.
>>>>
>>>> Comments and suggestions are welcomed.
>>>>
>>>> Thanks,
>>>>
>>>> -Rong
>>>>
>>>>
>>>> 2012-12-20  Rong Xu  <xur@google.com>
>>>>
>>>>       * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>>>>         function. Atomic update profile counters.
>>>>       (__gcov_one_value_profiler_atomic): Ditto.
>>>>       (__gcov_indirect_call_profiler_atomic): Ditto.
>>>>       * gcc/gcov-io.h: Macros for atomic update.
>>>>       * gcc/common.opt: New option.
>>>>       * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>>>>         update profile counters.
>>>>       (gimple_gen_edge_profiler): Ditto.
>>>
>>> The patch looks resonable.  Eventually we probably should provide rest of the value counters
>>> in thread safe manner.  What happens on targets not having atomic operations?
>>
>> From http://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins,
>> it says:
>>       "If a particular operation cannot be implemented on the target
>> processor, a warning is generated and a call an external function is
>> generated. "
>>
>> So I think there will be a warning and eventually a link error of unsat.
>>
>> Thanks,
>>
>> -Rong
>>
>>
>>>
>>> Honza

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

[-- Attachment #1: Type: text/plain, Size: 3377 bytes --]

Hi,

Here is a new patch. The only difference is to declare
__atomic_fetch_add as weak. This is
needed for targets without sync/atomic builtin support. The patch
contains a call to the builtin regardless of the new options
-fprofile-gen-atomic. This results in a unsat in these targets even
for regular profile-gen built.

With this new patch, if the user uses -fprofile-gen-atomic in these
target, the generated code will seg fault.

We think a better solution is to emit the builtin call only in these
targets with the support, and give warning for non-supported target.
But I did not find any target hook for this. Does anyone know how to
do this?

Thanks,

-Rong


On Fri, Dec 28, 2012 at 11:35 AM, Xinliang David Li <davidxl@google.com> wrote:
> It would be great if this can make into gcc4.8. The patch has close to
> 0 impact on code stability.
>
> David
>
> On Fri, Dec 28, 2012 at 11:32 AM, Rong Xu <xur@google.com> wrote:
>> Hi Honza,
>>
>> In the other thread of discussion (similar patch in google-4_7
>> branch), you said you were thinking if to let this patch into trunk in
>> stage 3. Can you give some update?
>>
>> Thanks,
>>
>> -Rong
>>
>> On Fri, Dec 21, 2012 at 10:37 AM, Rong Xu <xur@google.com> wrote:
>>> On Fri, Dec 21, 2012 at 1:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>> Hi,
>>>>>
>>>>> This patch adds support of atomic update of profiles counters. The goal is to improve
>>>>> the poor counter values for highly thread programs.
>>>>>
>>>>> The atomic update is under a new option -fprofile-gen-atomic=<N>
>>>>> N=0: default, no atomic update
>>>>> N=1: atomic update edge counters.
>>>>> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
>>>>> N=3: both edge counter and the above value profile counters.
>>>>> Other value: fall back to the default.
>>>>>
>>>>> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
>>>>> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
>>>>> the indirect-call profile is the most relevant one here.
>>>>>
>>>>> Test with bootstrap.
>>>>>
>>>>> Comments and suggestions are welcomed.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Rong
>>>>>
>>>>>
>>>>> 2012-12-20  Rong Xu  <xur@google.com>
>>>>>
>>>>>       * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>>>>>         function. Atomic update profile counters.
>>>>>       (__gcov_one_value_profiler_atomic): Ditto.
>>>>>       (__gcov_indirect_call_profiler_atomic): Ditto.
>>>>>       * gcc/gcov-io.h: Macros for atomic update.
>>>>>       * gcc/common.opt: New option.
>>>>>       * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>>>>>         update profile counters.
>>>>>       (gimple_gen_edge_profiler): Ditto.
>>>>
>>>> The patch looks resonable.  Eventually we probably should provide rest of the value counters
>>>> in thread safe manner.  What happens on targets not having atomic operations?
>>>
>>> From http://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins,
>>> it says:
>>>       "If a particular operation cannot be implemented on the target
>>> processor, a warning is generated and a call an external function is
>>> generated. "
>>>
>>> So I think there will be a warning and eventually a link error of unsat.
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>>
>>>>
>>>> Honza

[-- Attachment #2: patch2.diff --]
[-- Type: application/octet-stream, Size: 7303 bytes --]

2013-01-02  Rong Xu  <xur@google.com>
        * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New 
        function. Atomic update profile counters.
        (__gcov_one_value_profiler_atomic): Ditto.
        (__gcov_indirect_call_profiler_atomic): Ditto.
        * gcc/gcov-io.h: Macros for atomic update
        * gcc/common.opt: New option.
        * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
        update profile counters.
        (gimple_gen_edge_profiler): Ditto.

Index: libgcc/libgcov.c
===================================================================
--- libgcc/libgcov.c	(revision 194652)
+++ libgcc/libgcov.c	(working copy)
@@ -1113,12 +1113,35 @@ __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_ATOMIC_FETCH_ADD_FN (&counters[1], 1, MEMMODEL_RELAXED);
+  else if (counters[1] == 0)
+    {    
+      counters[1] = 1; 
+      counters[0] = value;
+    }    
+  else 
+    GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[1], -1, MEMMODEL_RELAXED);
+  GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[2], 1, MEMMODEL_RELAXED);
+}
+
 #ifdef L_gcov_one_value_profiler
 void
 __gcov_one_value_profiler (gcov_type *counters, gcov_type value)
 {
   __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
@@ -1153,6 +1176,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
 
 
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 194652)
+++ gcc/common.opt	(working copy)
@@ -1635,6 +1635,15 @@ fprofile-correction
 Common Report Var(flag_profile_correction)
 Enable correction of flow inconsistent profile data input
 
+; 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 194652)
+++ gcc/gcov-io.h	(working copy)
@@ -202,7 +202,19 @@ typedef unsigned gcov_type_unsigned __attribute__
 #endif
 #endif
 
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
+#else
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
+#endif
+/* Make the atomic builtin weak. Otherwise we get link unsat 
+   if the builtin is not available.  */
+extern gcov_type GCOV_TYPE_ATOMIC_FETCH_ADD_FN
+  (gcov_type*, gcov_type, int) __attribute__ ((weak));
 
+
 #if defined (TARGET_POSIX_IO)
 #define GCOV_LOCKED 1
 #else
@@ -212,6 +224,18 @@ typedef unsigned gcov_type_unsigned __attribute__
 #else /* !IN_LIBGCOV */
 /* About the host */
 
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
+#else
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_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)
+
 typedef unsigned gcov_unsigned_t;
 typedef unsigned gcov_position_t;
 /* gcov_type is typedef'd elsewhere for the compiler */
Index: gcc/tree-profile.c
===================================================================
--- gcc/tree-profile.c	(revision 194652)
+++ gcc/tree-profile.c	(working copy)
@@ -147,7 +147,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;
@@ -163,9 +168,14 @@ gimple_init_edge_profiler (void)
 					  gcov_type_ptr, gcov_type_node,
 					  ptr_void,
 					  ptr_void, NULL_TREE);
-      tree_indirect_call_profiler_fn
-	      = build_fn_decl ("__gcov_indirect_call_profiler",
-				     ic_profiler_fn_type);
+      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;
       DECL_ATTRIBUTES (tree_indirect_call_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
@@ -211,8 +221,21 @@ gimple_gen_edge_profiler (int edgeno, edge e)
   tree ref, one, gcov_type_tmp_var;
   gimple stmt1, stmt2, stmt3;
 
+  one = build_int_cst (gcov_type_node, 1);
+  if (PROFILE_GEN_EDGE_ATOMIC)
+    {
+      ref = tree_coverage_counter_addr (GCOV_COUNTER_ARCS, edgeno);
+      /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
+      stmt1 = gimple_build_call (builtin_decl_explicit (
+                                   GCOV_TYPE_ATOMIC_FETCH_ADD),
+                                 3, ref, one,
+                                 build_int_cst (integer_type_node,
+                                   MEMMODEL_RELAXED));
+     gsi_insert_on_edge (e, stmt1);
+     return;
+    }
+
   ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
-  one = build_int_cst (gcov_type_node, 1);
   gcov_type_tmp_var = make_temp_ssa_name (gcov_type_node,
 					  NULL, "PROF_edge_counter");
   stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);

Re: atomic update of profile counters (issue7000044)

[Andrew Pinski]

On Wed, Jan 2, 2013 at 5:15 PM, Rong Xu <xur@google.com> wrote:
> Hi,
>
> Here is a new patch. The only difference is to declare
> __atomic_fetch_add as weak. This is
> needed for targets without sync/atomic builtin support. The patch
> contains a call to the builtin regardless of the new options
> -fprofile-gen-atomic. This results in a unsat in these targets even
> for regular profile-gen built.
>
> With this new patch, if the user uses -fprofile-gen-atomic in these
> target, the generated code will seg fault.
>
> We think a better solution is to emit the builtin call only in these
> targets with the support, and give warning for non-supported target.
> But I did not find any target hook for this. Does anyone know how to
> do this?

Why not use libatomic for those targets?

Thanks,
Andrew Pinski



>
> Thanks,
>
> -Rong
>
>
> On Fri, Dec 28, 2012 at 11:35 AM, Xinliang David Li <davidxl@google.com> wrote:
>> It would be great if this can make into gcc4.8. The patch has close to
>> 0 impact on code stability.
>>
>> David
>>
>> On Fri, Dec 28, 2012 at 11:32 AM, Rong Xu <xur@google.com> wrote:
>>> Hi Honza,
>>>
>>> In the other thread of discussion (similar patch in google-4_7
>>> branch), you said you were thinking if to let this patch into trunk in
>>> stage 3. Can you give some update?
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>> On Fri, Dec 21, 2012 at 10:37 AM, Rong Xu <xur@google.com> wrote:
>>>> On Fri, Dec 21, 2012 at 1:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This patch adds support of atomic update of profiles counters. The goal is to improve
>>>>>> the poor counter values for highly thread programs.
>>>>>>
>>>>>> The atomic update is under a new option -fprofile-gen-atomic=<N>
>>>>>> N=0: default, no atomic update
>>>>>> N=1: atomic update edge counters.
>>>>>> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
>>>>>> N=3: both edge counter and the above value profile counters.
>>>>>> Other value: fall back to the default.
>>>>>>
>>>>>> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
>>>>>> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
>>>>>> the indirect-call profile is the most relevant one here.
>>>>>>
>>>>>> Test with bootstrap.
>>>>>>
>>>>>> Comments and suggestions are welcomed.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Rong
>>>>>>
>>>>>>
>>>>>> 2012-12-20  Rong Xu  <xur@google.com>
>>>>>>
>>>>>>       * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>>>>>>         function. Atomic update profile counters.
>>>>>>       (__gcov_one_value_profiler_atomic): Ditto.
>>>>>>       (__gcov_indirect_call_profiler_atomic): Ditto.
>>>>>>       * gcc/gcov-io.h: Macros for atomic update.
>>>>>>       * gcc/common.opt: New option.
>>>>>>       * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>>>>>>         update profile counters.
>>>>>>       (gimple_gen_edge_profiler): Ditto.
>>>>>
>>>>> The patch looks resonable.  Eventually we probably should provide rest of the value counters
>>>>> in thread safe manner.  What happens on targets not having atomic operations?
>>>>
>>>> From http://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins,
>>>> it says:
>>>>       "If a particular operation cannot be implemented on the target
>>>> processor, a warning is generated and a call an external function is
>>>> generated. "
>>>>
>>>> So I think there will be a warning and eventually a link error of unsat.
>>>>
>>>> Thanks,
>>>>
>>>> -Rong
>>>>
>>>>
>>>>>
>>>>> Honza

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

Does libatomic support all targets?
I think it's a good idea to change the driver to link in this library
if the option is specified.
But still, we need to make the builtin weak.

Thanks,

-Rong

On Wed, Jan 2, 2013 at 5:25 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Jan 2, 2013 at 5:15 PM, Rong Xu <xur@google.com> wrote:
>> Hi,
>>
>> Here is a new patch. The only difference is to declare
>> __atomic_fetch_add as weak. This is
>> needed for targets without sync/atomic builtin support. The patch
>> contains a call to the builtin regardless of the new options
>> -fprofile-gen-atomic. This results in a unsat in these targets even
>> for regular profile-gen built.
>>
>> With this new patch, if the user uses -fprofile-gen-atomic in these
>> target, the generated code will seg fault.
>>
>> We think a better solution is to emit the builtin call only in these
>> targets with the support, and give warning for non-supported target.
>> But I did not find any target hook for this. Does anyone know how to
>> do this?
>
> Why not use libatomic for those targets?
>
> Thanks,
> Andrew Pinski
>
>
>
>>
>> Thanks,
>>
>> -Rong
>>
>>
>> On Fri, Dec 28, 2012 at 11:35 AM, Xinliang David Li <davidxl@google.com> wrote:
>>> It would be great if this can make into gcc4.8. The patch has close to
>>> 0 impact on code stability.
>>>
>>> David
>>>
>>> On Fri, Dec 28, 2012 at 11:32 AM, Rong Xu <xur@google.com> wrote:
>>>> Hi Honza,
>>>>
>>>> In the other thread of discussion (similar patch in google-4_7
>>>> branch), you said you were thinking if to let this patch into trunk in
>>>> stage 3. Can you give some update?
>>>>
>>>> Thanks,
>>>>
>>>> -Rong
>>>>
>>>> On Fri, Dec 21, 2012 at 10:37 AM, Rong Xu <xur@google.com> wrote:
>>>>> On Fri, Dec 21, 2012 at 1:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This patch adds support of atomic update of profiles counters. The goal is to improve
>>>>>>> the poor counter values for highly thread programs.
>>>>>>>
>>>>>>> The atomic update is under a new option -fprofile-gen-atomic=<N>
>>>>>>> N=0: default, no atomic update
>>>>>>> N=1: atomic update edge counters.
>>>>>>> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
>>>>>>> N=3: both edge counter and the above value profile counters.
>>>>>>> Other value: fall back to the default.
>>>>>>>
>>>>>>> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
>>>>>>> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
>>>>>>> the indirect-call profile is the most relevant one here.
>>>>>>>
>>>>>>> Test with bootstrap.
>>>>>>>
>>>>>>> Comments and suggestions are welcomed.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> -Rong
>>>>>>>
>>>>>>>
>>>>>>> 2012-12-20  Rong Xu  <xur@google.com>
>>>>>>>
>>>>>>>       * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>>>>>>>         function. Atomic update profile counters.
>>>>>>>       (__gcov_one_value_profiler_atomic): Ditto.
>>>>>>>       (__gcov_indirect_call_profiler_atomic): Ditto.
>>>>>>>       * gcc/gcov-io.h: Macros for atomic update.
>>>>>>>       * gcc/common.opt: New option.
>>>>>>>       * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>>>>>>>         update profile counters.
>>>>>>>       (gimple_gen_edge_profiler): Ditto.
>>>>>>
>>>>>> The patch looks resonable.  Eventually we probably should provide rest of the value counters
>>>>>> in thread safe manner.  What happens on targets not having atomic operations?
>>>>>
>>>>> From http://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins,
>>>>> it says:
>>>>>       "If a particular operation cannot be implemented on the target
>>>>> processor, a warning is generated and a call an external function is
>>>>> generated. "
>>>>>
>>>>> So I think there will be a warning and eventually a link error of unsat.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Rong
>>>>>
>>>>>
>>>>>>
>>>>>> Honza

Re: atomic update of profile counters (issue7000044)

[Andrew Pinski]

On Wed, Jan 2, 2013 at 5:29 PM, Rong Xu <xur@google.com> wrote:
> Does libatomic support all targets?

It supports all targets that support pthreads.

Thanks,
Andrew


> I think it's a good idea to change the driver to link in this library
> if the option is specified.
> But still, we need to make the builtin weak.
>
> Thanks,
>
> -Rong
>
> On Wed, Jan 2, 2013 at 5:25 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Wed, Jan 2, 2013 at 5:15 PM, Rong Xu <xur@google.com> wrote:
>>> Hi,
>>>
>>> Here is a new patch. The only difference is to declare
>>> __atomic_fetch_add as weak. This is
>>> needed for targets without sync/atomic builtin support. The patch
>>> contains a call to the builtin regardless of the new options
>>> -fprofile-gen-atomic. This results in a unsat in these targets even
>>> for regular profile-gen built.
>>>
>>> With this new patch, if the user uses -fprofile-gen-atomic in these
>>> target, the generated code will seg fault.
>>>
>>> We think a better solution is to emit the builtin call only in these
>>> targets with the support, and give warning for non-supported target.
>>> But I did not find any target hook for this. Does anyone know how to
>>> do this?
>>
>> Why not use libatomic for those targets?
>>
>> Thanks,
>> Andrew Pinski
>>
>>
>>
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>>
>>> On Fri, Dec 28, 2012 at 11:35 AM, Xinliang David Li <davidxl@google.com> wrote:
>>>> It would be great if this can make into gcc4.8. The patch has close to
>>>> 0 impact on code stability.
>>>>
>>>> David
>>>>
>>>> On Fri, Dec 28, 2012 at 11:32 AM, Rong Xu <xur@google.com> wrote:
>>>>> Hi Honza,
>>>>>
>>>>> In the other thread of discussion (similar patch in google-4_7
>>>>> branch), you said you were thinking if to let this patch into trunk in
>>>>> stage 3. Can you give some update?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Rong
>>>>>
>>>>> On Fri, Dec 21, 2012 at 10:37 AM, Rong Xu <xur@google.com> wrote:
>>>>>> On Fri, Dec 21, 2012 at 1:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> This patch adds support of atomic update of profiles counters. The goal is to improve
>>>>>>>> the poor counter values for highly thread programs.
>>>>>>>>
>>>>>>>> The atomic update is under a new option -fprofile-gen-atomic=<N>
>>>>>>>> N=0: default, no atomic update
>>>>>>>> N=1: atomic update edge counters.
>>>>>>>> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
>>>>>>>> N=3: both edge counter and the above value profile counters.
>>>>>>>> Other value: fall back to the default.
>>>>>>>>
>>>>>>>> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
>>>>>>>> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
>>>>>>>> the indirect-call profile is the most relevant one here.
>>>>>>>>
>>>>>>>> Test with bootstrap.
>>>>>>>>
>>>>>>>> Comments and suggestions are welcomed.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> -Rong
>>>>>>>>
>>>>>>>>
>>>>>>>> 2012-12-20  Rong Xu  <xur@google.com>
>>>>>>>>
>>>>>>>>       * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>>>>>>>>         function. Atomic update profile counters.
>>>>>>>>       (__gcov_one_value_profiler_atomic): Ditto.
>>>>>>>>       (__gcov_indirect_call_profiler_atomic): Ditto.
>>>>>>>>       * gcc/gcov-io.h: Macros for atomic update.
>>>>>>>>       * gcc/common.opt: New option.
>>>>>>>>       * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>>>>>>>>         update profile counters.
>>>>>>>>       (gimple_gen_edge_profiler): Ditto.
>>>>>>>
>>>>>>> The patch looks resonable.  Eventually we probably should provide rest of the value counters
>>>>>>> in thread safe manner.  What happens on targets not having atomic operations?
>>>>>>
>>>>>> From http://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins,
>>>>>> it says:
>>>>>>       "If a particular operation cannot be implemented on the target
>>>>>> processor, a warning is generated and a call an external function is
>>>>>> generated. "
>>>>>>
>>>>>> So I think there will be a warning and eventually a link error of unsat.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Rong
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Honza

Re: atomic update of profile counters (issue7000044)

[Richard Biener]

On Thu, Jan 3, 2013 at 2:25 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Jan 2, 2013 at 5:15 PM, Rong Xu <xur@google.com> wrote:
>> Hi,
>>
>> Here is a new patch. The only difference is to declare
>> __atomic_fetch_add as weak. This is
>> needed for targets without sync/atomic builtin support. The patch
>> contains a call to the builtin regardless of the new options
>> -fprofile-gen-atomic. This results in a unsat in these targets even
>> for regular profile-gen built.
>>
>> With this new patch, if the user uses -fprofile-gen-atomic in these
>> target, the generated code will seg fault.
>>
>> We think a better solution is to emit the builtin call only in these
>> targets with the support, and give warning for non-supported target.
>> But I did not find any target hook for this. Does anyone know how to
>> do this?
>
> Why not use libatomic for those targets?

Also note that not all targets support 'weak' linkage.

Richard.

> Thanks,
> Andrew Pinski
>
>
>
>>
>> Thanks,
>>
>> -Rong
>>
>>
>> On Fri, Dec 28, 2012 at 11:35 AM, Xinliang David Li <davidxl@google.com> wrote:
>>> It would be great if this can make into gcc4.8. The patch has close to
>>> 0 impact on code stability.
>>>
>>> David
>>>
>>> On Fri, Dec 28, 2012 at 11:32 AM, Rong Xu <xur@google.com> wrote:
>>>> Hi Honza,
>>>>
>>>> In the other thread of discussion (similar patch in google-4_7
>>>> branch), you said you were thinking if to let this patch into trunk in
>>>> stage 3. Can you give some update?
>>>>
>>>> Thanks,
>>>>
>>>> -Rong
>>>>
>>>> On Fri, Dec 21, 2012 at 10:37 AM, Rong Xu <xur@google.com> wrote:
>>>>> On Fri, Dec 21, 2012 at 1:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This patch adds support of atomic update of profiles counters. The goal is to improve
>>>>>>> the poor counter values for highly thread programs.
>>>>>>>
>>>>>>> The atomic update is under a new option -fprofile-gen-atomic=<N>
>>>>>>> N=0: default, no atomic update
>>>>>>> N=1: atomic update edge counters.
>>>>>>> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
>>>>>>> N=3: both edge counter and the above value profile counters.
>>>>>>> Other value: fall back to the default.
>>>>>>>
>>>>>>> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
>>>>>>> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
>>>>>>> the indirect-call profile is the most relevant one here.
>>>>>>>
>>>>>>> Test with bootstrap.
>>>>>>>
>>>>>>> Comments and suggestions are welcomed.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> -Rong
>>>>>>>
>>>>>>>
>>>>>>> 2012-12-20  Rong Xu  <xur@google.com>
>>>>>>>
>>>>>>>       * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>>>>>>>         function. Atomic update profile counters.
>>>>>>>       (__gcov_one_value_profiler_atomic): Ditto.
>>>>>>>       (__gcov_indirect_call_profiler_atomic): Ditto.
>>>>>>>       * gcc/gcov-io.h: Macros for atomic update.
>>>>>>>       * gcc/common.opt: New option.
>>>>>>>       * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>>>>>>>         update profile counters.
>>>>>>>       (gimple_gen_edge_profiler): Ditto.
>>>>>>
>>>>>> The patch looks resonable.  Eventually we probably should provide rest of the value counters
>>>>>> in thread safe manner.  What happens on targets not having atomic operations?
>>>>>
>>>>> From http://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins,
>>>>> it says:
>>>>>       "If a particular operation cannot be implemented on the target
>>>>> processor, a warning is generated and a call an external function is
>>>>> generated. "
>>>>>
>>>>> So I think there will be a warning and eventually a link error of unsat.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Rong
>>>>>
>>>>>
>>>>>>
>>>>>> Honza

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

[-- Attachment #1: Type: text/plain, Size: 4480 bytes --]

Here is the new patch.

It links libatomic when -fprofile-gen-atomic is specified for FDO
instrumentation build. Here I assume libatomic is always installed.
Andrew: do you think if this is reasonable?

It also disables the functionality if target does not support weak
(ie. TARGET_SUPPORTS_WEAK == 0).

Thanks,

-Rong

On Thu, Jan 3, 2013 at 1:05 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, Jan 3, 2013 at 2:25 AM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Wed, Jan 2, 2013 at 5:15 PM, Rong Xu <xur@google.com> wrote:
>>> Hi,
>>>
>>> Here is a new patch. The only difference is to declare
>>> __atomic_fetch_add as weak. This is
>>> needed for targets without sync/atomic builtin support. The patch
>>> contains a call to the builtin regardless of the new options
>>> -fprofile-gen-atomic. This results in a unsat in these targets even
>>> for regular profile-gen built.
>>>
>>> With this new patch, if the user uses -fprofile-gen-atomic in these
>>> target, the generated code will seg fault.
>>>
>>> We think a better solution is to emit the builtin call only in these
>>> targets with the support, and give warning for non-supported target.
>>> But I did not find any target hook for this. Does anyone know how to
>>> do this?
>>
>> Why not use libatomic for those targets?
>
> Also note that not all targets support 'weak' linkage.

How about check the flag TARGET_SUPPORTS_WEAK, and only enable the
code when the flag is true.
>
> Richard.
>
>> Thanks,
>> Andrew Pinski
>>
>>
>>
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>>
>>> On Fri, Dec 28, 2012 at 11:35 AM, Xinliang David Li <davidxl@google.com> wrote:
>>>> It would be great if this can make into gcc4.8. The patch has close to
>>>> 0 impact on code stability.
>>>>
>>>> David
>>>>
>>>> On Fri, Dec 28, 2012 at 11:32 AM, Rong Xu <xur@google.com> wrote:
>>>>> Hi Honza,
>>>>>
>>>>> In the other thread of discussion (similar patch in google-4_7
>>>>> branch), you said you were thinking if to let this patch into trunk in
>>>>> stage 3. Can you give some update?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Rong
>>>>>
>>>>> On Fri, Dec 21, 2012 at 10:37 AM, Rong Xu <xur@google.com> wrote:
>>>>>> On Fri, Dec 21, 2012 at 1:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> This patch adds support of atomic update of profiles counters. The goal is to improve
>>>>>>>> the poor counter values for highly thread programs.
>>>>>>>>
>>>>>>>> The atomic update is under a new option -fprofile-gen-atomic=<N>
>>>>>>>> N=0: default, no atomic update
>>>>>>>> N=1: atomic update edge counters.
>>>>>>>> N=2: atomic update some of value profile counters (currently indirect-call and one value profile).
>>>>>>>> N=3: both edge counter and the above value profile counters.
>>>>>>>> Other value: fall back to the default.
>>>>>>>>
>>>>>>>> This patch is a simple porting of the version in google-4_7 branch. It uses __atomic_fetch_add
>>>>>>>> based on Andrew Pinski's suggestion. Note I did not apply to all the value profiles as
>>>>>>>> the indirect-call profile is the most relevant one here.
>>>>>>>>
>>>>>>>> Test with bootstrap.
>>>>>>>>
>>>>>>>> Comments and suggestions are welcomed.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> -Rong
>>>>>>>>
>>>>>>>>
>>>>>>>> 2012-12-20  Rong Xu  <xur@google.com>
>>>>>>>>
>>>>>>>>       * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New
>>>>>>>>         function. Atomic update profile counters.
>>>>>>>>       (__gcov_one_value_profiler_atomic): Ditto.
>>>>>>>>       (__gcov_indirect_call_profiler_atomic): Ditto.
>>>>>>>>       * gcc/gcov-io.h: Macros for atomic update.
>>>>>>>>       * gcc/common.opt: New option.
>>>>>>>>       * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
>>>>>>>>         update profile counters.
>>>>>>>>       (gimple_gen_edge_profiler): Ditto.
>>>>>>>
>>>>>>> The patch looks resonable.  Eventually we probably should provide rest of the value counters
>>>>>>> in thread safe manner.  What happens on targets not having atomic operations?
>>>>>>
>>>>>> From http://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins,
>>>>>> it says:
>>>>>>       "If a particular operation cannot be implemented on the target
>>>>>> processor, a warning is generated and a call an external function is
>>>>>> generated. "
>>>>>>
>>>>>> So I think there will be a warning and eventually a link error of unsat.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Rong
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Honza

[-- Attachment #2: patch3.diff --]
[-- Type: application/octet-stream, Size: 8250 bytes --]

2013-01-03  Rong Xu  <xur@google.com>

        * libgcc/libgcov.c (__gcov_one_value_profiler_body_atomic): New 
        function. Atomic update profile counters.
        (__gcov_one_value_profiler_atomic): Ditto.
        (__gcov_indirect_call_profiler_atomic): Ditto.
        * gcc/gcov-io.h: Macros for atomic update.
        * gcc/common.opt: New option.
	* gcc/gcc.c: Link libatomic when fprofile-gen-atomic= specified.
        * gcc/tree-profile.c (gimple_init_edge_profiler): Atomic
        update profile counters.
        (gimple_gen_edge_profiler): Ditto.

Index: gcc/gcov-io.h
===================================================================
--- gcc/gcov-io.h	(revision 194652)
+++ gcc/gcov-io.h	(working copy)
@@ -202,7 +202,21 @@ typedef unsigned gcov_type_unsigned __attribute__
 #endif
 #endif
 
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
+#else
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
+#endif
+#if TARGET_SUPPORTS_WEAK
+/* Make the atomic builtin weak. Otherwise we get link unsat 
+   if the builtin is not available.  */
+extern gcov_type GCOV_TYPE_ATOMIC_FETCH_ADD_FN
+  (gcov_type*, gcov_type, int) __attribute__ ((weak));
+#endif /* TARGET_SUPPORTS_WEAK */
 
+
 #if defined (TARGET_POSIX_IO)
 #define GCOV_LOCKED 1
 #else
@@ -212,6 +226,20 @@ typedef unsigned gcov_type_unsigned __attribute__
 #else /* !IN_LIBGCOV */
 /* About the host */
 
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
+#else
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
+#endif
+#if TARGET_SUPPORTS_WEAK
+#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)
+#endif /* TARGET_SUPPORTS_WEAK */
+
 typedef unsigned gcov_unsigned_t;
 typedef unsigned gcov_position_t;
 /* gcov_type is typedef'd elsewhere for the compiler */
Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 194652)
+++ gcc/gcc.c	(working copy)
@@ -711,7 +711,8 @@ proper position among the other output files.  */
     %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)}\
     %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
     %(mflib) " STACK_SPLIT_SPEC "\
-    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov}\
+    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov\
+      %{fprofile-gen-atomic=*:-latomic}}\
     %{fsanitize=address:" LIBASAN_SPEC "%{static:%ecannot specify -static with -fsanitize=address}}\
     %{fsanitize=thread:" LIBTSAN_SPEC "}\
     %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
Index: gcc/tree-profile.c
===================================================================
--- gcc/tree-profile.c	(revision 194652)
+++ gcc/tree-profile.c	(working copy)
@@ -147,7 +147,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;
@@ -163,9 +168,14 @@ gimple_init_edge_profiler (void)
 					  gcov_type_ptr, gcov_type_node,
 					  ptr_void,
 					  ptr_void, NULL_TREE);
-      tree_indirect_call_profiler_fn
-	      = build_fn_decl ("__gcov_indirect_call_profiler",
-				     ic_profiler_fn_type);
+      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;
       DECL_ATTRIBUTES (tree_indirect_call_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
@@ -211,8 +221,21 @@ gimple_gen_edge_profiler (int edgeno, edge e)
   tree ref, one, gcov_type_tmp_var;
   gimple stmt1, stmt2, stmt3;
 
+  one = build_int_cst (gcov_type_node, 1);
+  if (PROFILE_GEN_EDGE_ATOMIC)
+    {
+      ref = tree_coverage_counter_addr (GCOV_COUNTER_ARCS, edgeno);
+      /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
+      stmt1 = gimple_build_call (builtin_decl_explicit (
+                                   GCOV_TYPE_ATOMIC_FETCH_ADD),
+                                 3, ref, one,
+                                 build_int_cst (integer_type_node,
+                                   MEMMODEL_RELAXED));
+     gsi_insert_on_edge (e, stmt1);
+     return;
+    }
+
   ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
-  one = build_int_cst (gcov_type_node, 1);
   gcov_type_tmp_var = make_temp_ssa_name (gcov_type_node,
 					  NULL, "PROF_edge_counter");
   stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 194652)
+++ gcc/common.opt	(working copy)
@@ -1635,6 +1635,15 @@ fprofile-correction
 Common Report Var(flag_profile_correction)
 Enable correction of flow inconsistent profile data input
 
+; 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: libgcc/libgcov.c
===================================================================
--- libgcc/libgcov.c	(revision 194652)
+++ libgcc/libgcov.c	(working copy)
@@ -1113,12 +1113,35 @@ __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_ATOMIC_FETCH_ADD_FN (&counters[1], 1, MEMMODEL_RELAXED);
+  else if (counters[1] == 0)
+    {    
+      counters[1] = 1; 
+      counters[0] = value;
+    }    
+  else 
+    GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[1], -1, MEMMODEL_RELAXED);
+  GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[2], 1, MEMMODEL_RELAXED);
+}
+
 #ifdef L_gcov_one_value_profiler
 void
 __gcov_one_value_profiler (gcov_type *counters, gcov_type value)
 {
   __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
@@ -1153,6 +1176,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
 
 

Re: atomic update of profile counters (issue7000044)

[Richard Henderson]

On 01/03/2013 04:42 PM, Rong Xu wrote:
> It links libatomic when -fprofile-gen-atomic is specified for FDO
> instrumentation build. Here I assume libatomic is always installed.
> Andrew: do you think if this is reasonable?
> 
> It also disables the functionality if target does not support weak
> (ie. TARGET_SUPPORTS_WEAK == 0).

Since you're linking libatomic, you don't need weak references.

I think its ok to assume libatomic is installed, given that the
user has had to explicitly use the command-line option.


r~

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

Function __gcov_indirect_call_profiler_atomic (which contains call to
the atomic function) is always emitted in libgcov.
Since we only link libatomic when -fprofile-gen-atomic is specified,
we have to make the atomic function weak -- otherwise, there is a
unsat for regular FDO gen build (of course, when the builtin is not
expanded).

An alternative it to always link libatomic together with libgcov. Then
we don't need the weak stuff. I'm not sure when one is better.

-Rong

On Mon, Jan 7, 2013 at 12:36 PM, Richard Henderson <rth@redhat.com> wrote:
> On 01/03/2013 04:42 PM, Rong Xu wrote:
>> It links libatomic when -fprofile-gen-atomic is specified for FDO
>> instrumentation build. Here I assume libatomic is always installed.
>> Andrew: do you think if this is reasonable?
>>
>> It also disables the functionality if target does not support weak
>> (ie. TARGET_SUPPORTS_WEAK == 0).
>
> Since you're linking libatomic, you don't need weak references.
>
> I think its ok to assume libatomic is installed, given that the
> user has had to explicitly use the command-line option.
>
>
> r~

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]

Hi all,

I merged this old patch with current trunk. I also make the following changes
(1) not using weak references. Now every *profile_atomic() has it's
own .o so that none of them will be in the final binary if
-fprofile-generate-atomic is not specified.
(2) more value profilers have the atomic version.
(3) not link to libatomic. I used to link the libatomic in the
presence of -fprofile-generate-atomic, per Andrew's suggestion. It
used to work. But now if I can add -latomic in the SPEC, it cannot
find the libatomic.so.1 (unless I specify the PATH). I did not find an
easy way to statically link libatomic.a. Andrew: Do you have any
suggestion? Or should we let the user link to libatomic.a if the
builtins are not expanded?

Is this OK for trunk?

Thanks,

-Rong

On Mon, Jan 7, 2013 at 12:55 PM, Rong Xu <xur@google.com> wrote:
> Function __gcov_indirect_call_profiler_atomic (which contains call to
> the atomic function) is always emitted in libgcov.
> Since we only link libatomic when -fprofile-gen-atomic is specified,
> we have to make the atomic function weak -- otherwise, there is a
> unsat for regular FDO gen build (of course, when the builtin is not
> expanded).
>
> An alternative it to always link libatomic together with libgcov. Then
> we don't need the weak stuff. I'm not sure when one is better.
>
> -Rong
>
> On Mon, Jan 7, 2013 at 12:36 PM, Richard Henderson <rth@redhat.com> wrote:
>> On 01/03/2013 04:42 PM, Rong Xu wrote:
>>> It links libatomic when -fprofile-gen-atomic is specified for FDO
>>> instrumentation build. Here I assume libatomic is always installed.
>>> Andrew: do you think if this is reasonable?
>>>
>>> It also disables the functionality if target does not support weak
>>> (ie. TARGET_SUPPORTS_WEAK == 0).
>>
>> Since you're linking libatomic, you don't need weak references.
>>
>> I think its ok to assume libatomic is installed, given that the
>> user has had to explicitly use the command-line option.
>>
>>
>> r~

[-- Attachment #2: libgcov_atomic_new.txt --]
[-- Type: text/plain, Size: 17702 bytes --]

2013-11-19  Rong Xu  <xur@google.com>

	* gcc/gcov-io.h: Add atomic function macros for compiler use.
	* gcc/common.opt (fprofile-generate-atomic): New option.
	* gcc/tree-profile.c (gimple_init_edge_profiler): Support for
        atomic counter update.
	(gimple_gen_edge_profiler): Ditto.
	* libgcc/libgcov-profiler.c 
	(__gcov_interval_profiler_atomic): Ditto.
	(__gcov_pow2_profiler_atomic): Ditto.
	(__gcov_one_value_profiler_body_atomic): Ditto.
	(__gcov_one_value_profiler_atomic): Ditto.
	(__gcov_indirect_call_profiler_atomic): Ditto.
	(__gcov_indirect_call_profiler_v2_atomic): Ditto.
	(__gcov_time_profiler_atomic): Ditto.
	(__gcov_average_profiler_atomic): Ditto.
	* gcc/gcc.c: Link libatomic when -fprofile-generate-atomic used.
	* libgcc/Makefile.in: Add atomic objects.

Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 205053)
+++ gcc/common.opt	(working copy)
@@ -1684,6 +1684,15 @@ fprofile-correction
 Common Report Var(flag_profile_correction)
 Enable correction of flow inconsistent profile data input
 
+; fprofile-generate-atomic=0: default and disable atomical update.
+; fprofile-generate-atomic=1: atomically update edge profile counters.
+; fprofile-generate-atomic=2: atomically update value profile counters.
+; fprofile-generate-atomic=3: atomically update edge and value profile counters.
+; other values will be ignored (fall back to the default of 0).
+fprofile-generate-atomic=
+Common Joined UInteger Report Var(flag_profile_generate_atomic) Init(3) Optimization
+fprofile-generate-atomic=[0..3] Atomical increments of profile counters.
+
 fprofile-generate
 Common
 Enable common options for generating profile info for profile feedback directed optimizations
Index: gcc/tree-profile.c
===================================================================
--- gcc/tree-profile.c	(revision 205053)
+++ gcc/tree-profile.c	(working copy)
@@ -155,6 +155,9 @@ gimple_init_edge_profiler (void)
   tree ic_profiler_fn_type;
   tree average_profiler_fn_type;
   tree time_profiler_fn_type;
+  const char *fn_name;
+  bool profile_gen_value_atomic = (flag_profile_generate_atomic == 2 ||
+                                   flag_profile_generate_atomic == 3);
 
   if (!gcov_type_node)
     {
@@ -167,9 +170,10 @@ gimple_init_edge_profiler (void)
 					  gcov_type_ptr, gcov_type_node,
 					  integer_type_node,
 					  unsigned_type_node, NULL_TREE);
+      fn_name = profile_gen_value_atomic ? "__gcov_interval_profiler_atomic" : 
+                 "__gcov_interval_profiler";
       tree_interval_profiler_fn
-	      = build_fn_decl ("__gcov_interval_profiler",
-				     interval_profiler_fn_type);
+	      = build_fn_decl (fn_name, interval_profiler_fn_type);
       TREE_NOTHROW (tree_interval_profiler_fn) = 1;
       DECL_ATTRIBUTES (tree_interval_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
@@ -180,21 +184,23 @@ gimple_init_edge_profiler (void)
 	      = build_function_type_list (void_type_node,
 					  gcov_type_ptr, gcov_type_node,
 					  NULL_TREE);
-      tree_pow2_profiler_fn = build_fn_decl ("__gcov_pow2_profiler",
-						   pow2_profiler_fn_type);
+      fn_name = profile_gen_value_atomic ? "__gcov_pow2_profiler_atomic" : 
+                 "__gcov_pow2_profiler";
+      tree_pow2_profiler_fn = build_fn_decl (fn_name, pow2_profiler_fn_type);
       TREE_NOTHROW (tree_pow2_profiler_fn) = 1;
       DECL_ATTRIBUTES (tree_pow2_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
 		     DECL_ATTRIBUTES (tree_pow2_profiler_fn));
 
       /* void (*) (gcov_type *, gcov_type)  */
+      fn_name = profile_gen_value_atomic ? "__gcov_one_value_profiler_atomic" :
+                 "__gcov_one_value_profiler";
       one_value_profiler_fn_type
 	      = build_function_type_list (void_type_node,
 					  gcov_type_ptr, gcov_type_node,
 					  NULL_TREE);
       tree_one_value_profiler_fn
-	      = build_fn_decl ("__gcov_one_value_profiler",
-				     one_value_profiler_fn_type);
+	      = build_fn_decl (fn_name, one_value_profiler_fn_type);
       TREE_NOTHROW (tree_one_value_profiler_fn) = 1;
       DECL_ATTRIBUTES (tree_one_value_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
@@ -211,9 +217,10 @@ gimple_init_edge_profiler (void)
 					      gcov_type_ptr, gcov_type_node,
 					      ptr_void, ptr_void,
 					      NULL_TREE);
+          fn_name = profile_gen_value_atomic ? "__gcov_indirect_call_profiler_atomic" : 
+                     "__gcov_indirect_call_profiler";
 	  tree_indirect_call_profiler_fn
-		  = build_fn_decl ("__gcov_indirect_call_profiler",
-					 ic_profiler_fn_type);
+		  = build_fn_decl (fn_name, ic_profiler_fn_type);
         }
       else
         {
@@ -223,9 +230,10 @@ gimple_init_edge_profiler (void)
 					      gcov_type_node,
 					      ptr_void,
 					      NULL_TREE);
+          fn_name = profile_gen_value_atomic ? "__gcov_indirect_call_profiler_v2_atomic" : 
+                     "__gcov_indirect_call_profiler_v2";
 	  tree_indirect_call_profiler_fn
-		  = build_fn_decl ("__gcov_indirect_call_profiler_v2",
-					 ic_profiler_fn_type);
+		  = build_fn_decl (fn_name, ic_profiler_fn_type);
         }
       TREE_NOTHROW (tree_indirect_call_profiler_fn) = 1;
       DECL_ATTRIBUTES (tree_indirect_call_profiler_fn)
@@ -236,9 +244,10 @@ gimple_init_edge_profiler (void)
       time_profiler_fn_type
 	       = build_function_type_list (void_type_node,
 					  gcov_type_ptr, NULL_TREE);
+      fn_name = profile_gen_value_atomic ? "__gcov_time_profiler_atomic" : 
+                 "__gcov_time_profiler";
       tree_time_profiler_fn
-	      = build_fn_decl ("__gcov_time_profiler",
-				     time_profiler_fn_type);
+	      = build_fn_decl (fn_name, time_profiler_fn_type);
       TREE_NOTHROW (tree_time_profiler_fn) = 1;
       DECL_ATTRIBUTES (tree_time_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
@@ -248,9 +257,10 @@ gimple_init_edge_profiler (void)
       average_profiler_fn_type
 	      = build_function_type_list (void_type_node,
 					  gcov_type_ptr, gcov_type_node, NULL_TREE);
+      fn_name = profile_gen_value_atomic ? "__gcov_average_profiler_atomic" : 
+                 "__gcov_average_profiler";
       tree_average_profiler_fn
-	      = build_fn_decl ("__gcov_average_profiler",
-				     average_profiler_fn_type);
+	      = build_fn_decl (fn_name, average_profiler_fn_type);
       TREE_NOTHROW (tree_average_profiler_fn) = 1;
       DECL_ATTRIBUTES (tree_average_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
@@ -284,9 +294,24 @@ gimple_gen_edge_profiler (int edgeno, edge e)
 {
   tree ref, one, gcov_type_tmp_var;
   gimple stmt1, stmt2, stmt3;
+  bool profile_gen_edge_atomic = (flag_profile_generate_atomic == 1 ||
+                                  flag_profile_generate_atomic == 3);
 
+  one = build_int_cst (gcov_type_node, 1);
+  if (profile_gen_edge_atomic)
+    {
+      ref = tree_coverage_counter_addr (GCOV_COUNTER_ARCS, edgeno);
+      /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
+      stmt1 = gimple_build_call (builtin_decl_explicit (
+                                   GCOV_TYPE_ATOMIC_FETCH_ADD),
+                                 3, ref, one,
+                                 build_int_cst (integer_type_node,
+                                   MEMMODEL_RELAXED));
+      gsi_insert_on_edge (e, stmt1);
+      return;
+    }
+
   ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
-  one = build_int_cst (gcov_type_node, 1);
   gcov_type_tmp_var = make_temp_ssa_name (gcov_type_node,
 					  NULL, "PROF_edge_counter");
   stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
Index: gcc/gcov-io.h
===================================================================
--- gcc/gcov-io.h	(revision 205053)
+++ gcc/gcov-io.h	(working copy)
@@ -211,6 +211,15 @@ typedef unsigned gcov_type_unsigned __attribute__
 #else /* !IN_LIBGCOV */
 /* About the host */
 
+/* Macros for atomical counter update.  */
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
+#else
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
+#endif
+
 typedef unsigned gcov_unsigned_t;
 typedef unsigned gcov_position_t;
 /* gcov_type is typedef'd elsewhere for the compiler */
Index: libgcc/libgcov-profiler.c
===================================================================
--- libgcc/libgcov-profiler.c	(revision 205053)
+++ libgcc/libgcov-profiler.c	(working copy)
@@ -33,6 +33,15 @@ see the files COPYING3 and COPYING.RUNTIME respect
 #define IN_LIBGCOV 1
 #include "gcov-io.h"
 
+/* Macros for atomical counter update.  */
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
+#else
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
+#endif
+
 #ifdef L_gcov_interval_profiler
 /* If VALUE is in interval <START, START + STEPS - 1>, then increases the
    corresponding counter in COUNTERS.  If the VALUE is above or below
@@ -53,6 +62,21 @@ __gcov_interval_profiler (gcov_type *counters, gco
 }
 #endif
 
+#ifdef L_gcov_interval_profiler_atomic
+void
+__gcov_interval_profiler_atomic (gcov_type *counters, gcov_type value,
+                                 int start, unsigned steps)
+{
+  gcov_type delta = value - start;
+  if (delta < 0)
+    GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[steps + 1], 1, MEMMODEL_RELAXED);
+  else if (delta >= steps)
+    GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[steps], 1, MEMMODEL_RELAXED);
+  else
+    GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[delta], 1, MEMMODEL_RELAXED);
+}
+#endif
+
 #ifdef L_gcov_pow2_profiler
 /* If VALUE is a power of two, COUNTERS[1] is incremented.  Otherwise
    COUNTERS[0] is incremented.  */
@@ -67,6 +91,17 @@ __gcov_pow2_profiler (gcov_type *counters, gcov_ty
 }
 #endif
 
+#ifdef L_gcov_pow2_profiler_atomic
+void
+__gcov_pow2_profiler_atomic (gcov_type *counters, gcov_type value)
+{
+  if (value & (value - 1))
+    GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[0], 1, MEMMODEL_RELAXED);
+  else
+    GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[1], 1, MEMMODEL_RELAXED);
+}
+#endif
+
 /* Tries to determine the most common value among its inputs.  Checks if the
    value stored in COUNTERS[0] matches VALUE.  If this is the case, COUNTERS[1]
    is incremented.  If this is not the case and COUNTERS[1] is not zero,
@@ -92,6 +127,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_ATOMIC_FETCH_ADD_FN (&counters[1], 1, MEMMODEL_RELAXED);
+  else if (counters[1] == 0)
+    {    
+      counters[1] = 1; 
+      counters[0] = value;
+    }    
+  else 
+    GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[1], -1, MEMMODEL_RELAXED);
+  GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[2], 1, MEMMODEL_RELAXED);
+}
+
 #ifdef L_gcov_one_value_profiler
 void
 __gcov_one_value_profiler (gcov_type *counters, gcov_type value)
@@ -100,9 +151,13 @@ __gcov_one_value_profiler (gcov_type *counters, gc
 }
 #endif
 
-#ifdef L_gcov_indirect_call_profiler
-/* This function exist only for workaround of binutils bug 14342.
-   Once this compatibility hack is obsolette, it can be removed.  */
+#ifdef L_gcov_one_value_profiler_atomic
+void
+__gcov_one_value_profiler_atomic (gcov_type *counters, gcov_type value)
+{
+  __gcov_one_value_profiler_body_atomic (counters, value);
+}
+#endif
 
 /* By default, the C++ compiler will use function addresses in the
    vtable entries.  Setting TARGET_VTABLE_USES_DESCRIPTORS to nonzero
@@ -121,6 +176,10 @@ __gcov_one_value_profiler (gcov_type *counters, gc
 #define VTABLE_USES_DESCRIPTORS 0
 #endif
 
+#ifdef L_gcov_indirect_call_profiler
+/* This function exist only for workaround of binutils bug 14342.
+   Once this compatibility hack is obsolette, it can be removed.  */
+
 /* Tries to determine the most common value among its inputs. */
 void
 __gcov_indirect_call_profiler (gcov_type* counter, gcov_type value,
@@ -134,8 +193,21 @@ __gcov_indirect_call_profiler (gcov_type* counter,
           && *(void **) cur_func == *(void **) callee_func))
     __gcov_one_value_profiler_body (counter, value);
 }
+#endif
 
+#ifdef L_gcov_indirect_call_profiler_atomic
+/* 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
+
 #ifdef L_gcov_indirect_call_profiler_v2
 
 /* These two variables are used to actually track caller and callee.  Keep
@@ -152,23 +224,6 @@ __thread
 #endif
 gcov_type * __gcov_indirect_call_counters;
 
-/* By default, the C++ compiler will use function addresses in the
-   vtable entries.  Setting TARGET_VTABLE_USES_DESCRIPTORS to nonzero
-   tells the compiler to use function descriptors instead.  The value
-   of this macro says how many words wide the descriptor is (normally 2),
-   but it may be dependent on target flags.  Since we do not have access
-   to the target flags here we just check to see if it is set and use
-   that to set VTABLE_USES_DESCRIPTORS to 0 or 1.
-
-   It is assumed that the address of a function descriptor may be treated
-   as a pointer to a function.  */
-
-#ifdef TARGET_VTABLE_USES_DESCRIPTORS
-#define VTABLE_USES_DESCRIPTORS 1
-#else
-#define VTABLE_USES_DESCRIPTORS 0
-#endif
-
 /* Tries to determine the most common value among its inputs. */
 void
 __gcov_indirect_call_profiler_v2 (gcov_type value, void* cur_func)
@@ -183,10 +238,30 @@ __gcov_indirect_call_profiler_v2 (gcov_type value,
 }
 #endif
 
+#ifdef L_gcov_indirect_call_profiler_v2_atomic
+
+#if defined(HAVE_CC_TLS) && !defined (USE_EMUTLS)
+extern __thread void * __gcov_indirect_call_callee;
+extern __thread gcov_type * __gcov_indirect_call_counters;
+#else
+extern void * __gcov_indirect_call_callee;
+extern gcov_type * __gcov_indirect_call_counters;
+#endif
+
+void
+__gcov_indirect_call_profiler_v2_atomic (gcov_type value, void* cur_func)
+{
+  if (cur_func == __gcov_indirect_call_callee
+      || (VTABLE_USES_DESCRIPTORS && __gcov_indirect_call_callee
+          && *(void **) cur_func == *(void **) __gcov_indirect_call_callee))
+    __gcov_one_value_profiler_body_atomic (__gcov_indirect_call_counters, value);
+}
+#endif
+
 #ifdef L_gcov_time_profiler
 
 /* Counter for first visit of each function.  */
-static gcov_type function_counter;
+gcov_type function_counter;
 
 /* Sets corresponding COUNTERS if there is no value.  */
 
@@ -198,6 +273,16 @@ __gcov_time_profiler (gcov_type* counters)
 }
 #endif
 
+#ifdef L_gcov_time_profiler_atomic
+extern gcov_type function_counter;
+void
+__gcov_time_profiler_atomic (gcov_type* counters)
+{
+  if (!counters[0])
+    counters[0] = GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&function_counter, 1, MEMMODEL_RELAXED);
+}
+#endif
+
 #ifdef L_gcov_average_profiler
 /* Increase corresponding COUNTER by VALUE.  FIXME: Perhaps we want
    to saturate up.  */
@@ -205,11 +290,20 @@ __gcov_time_profiler (gcov_type* counters)
 void
 __gcov_average_profiler (gcov_type *counters, gcov_type value)
 {
-  counters[0] += value;
-  counters[1] ++;
+  GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[0], value, MEMMODEL_RELAXED);
+  GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[1], 1, MEMMODEL_RELAXED);
 }
 #endif
 
+#ifdef L_gcov_average_profiler_atomic
+void
+__gcov_average_profiler_atomic (gcov_type *counters, gcov_type value)
+{
+  GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[0], value, MEMMODEL_RELAXED);
+  GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[1], 1, MEMMODEL_RELAXED);
+}
+#endif
+
 #ifdef L_gcov_ior_profiler
 /* Bitwise-OR VALUE into COUNTER.  */
 
Index: libgcc/Makefile.in
===================================================================
--- libgcc/Makefile.in	(revision 205053)
+++ libgcc/Makefile.in	(working copy)
@@ -863,15 +863,20 @@ LIBGCOV_DRIVER = _gcov
 
 libgcov-merge-objects = $(patsubst %,%$(objext),$(LIBGCOV_MERGE))
 libgcov-profiler-objects = $(patsubst %,%$(objext),$(LIBGCOV_PROFILER))
+libgcov-profiler-atomic-objects = $(patsubst %,%_atomic$(objext),$(LIBGCOV_PROFILER))
 libgcov-interface-objects = $(patsubst %,%$(objext),$(LIBGCOV_INTERFACE))
 libgcov-driver-objects = $(patsubst %,%$(objext),$(LIBGCOV_DRIVER))
 libgcov-objects = $(libgcov-merge-objects) $(libgcov-profiler-objects) \
-                 $(libgcov-interface-objects) $(libgcov-driver-objects)
+                 $(libgcov-interface-objects) $(libgcov-driver-objects) \
+                 $(libgcov-profiler-atomic-objects)
 
+
 $(libgcov-merge-objects): %$(objext): $(srcdir)/libgcov-merge.c
 	$(gcc_compile) -DL$* -c $(srcdir)/libgcov-merge.c
 $(libgcov-profiler-objects): %$(objext): $(srcdir)/libgcov-profiler.c
 	$(gcc_compile) -DL$* -c $(srcdir)/libgcov-profiler.c
+$(libgcov-profiler-atomic-objects): %$(objext): $(srcdir)/libgcov-profiler.c
+	$(gcc_compile) -DL$* -c $(srcdir)/libgcov-profiler.c
 $(libgcov-interface-objects): %$(objext): $(srcdir)/libgcov-interface.c
 	$(gcc_compile) -DL$* -c $(srcdir)/libgcov-interface.c
 $(libgcov-driver-objects): %$(objext): $(srcdir)/libgcov-driver.c \

Re: atomic update of profile counters (issue7000044)

[Andrew Pinski]

On Tue, Nov 19, 2013 at 5:02 PM, Rong Xu <xur@google.com> wrote:
> Hi all,
>
> I merged this old patch with current trunk. I also make the following changes
> (1) not using weak references. Now every *profile_atomic() has it's
> own .o so that none of them will be in the final binary if
> -fprofile-generate-atomic is not specified.
> (2) more value profilers have the atomic version.
> (3) not link to libatomic. I used to link the libatomic in the
> presence of -fprofile-generate-atomic, per Andrew's suggestion. It
> used to work. But now if I can add -latomic in the SPEC, it cannot
> find the libatomic.so.1 (unless I specify the PATH). I did not find an
> easy way to statically link libatomic.a. Andrew: Do you have any
> suggestion? Or should we let the user link to libatomic.a if the
> builtins are not expanded?

It should work for an installed GCC.  For testing you might need
something that is included inside testsuite/lib/atomic-dg.exp which
sets the library path to include libatomic build directory.

I think now we require libatomic in more cases (C11 atomic support for
an example).

Thanks,
Andrew Pinski

>
> Is this OK for trunk?
>
> Thanks,
>
> -Rong
>
> On Mon, Jan 7, 2013 at 12:55 PM, Rong Xu <xur@google.com> wrote:
>> Function __gcov_indirect_call_profiler_atomic (which contains call to
>> the atomic function) is always emitted in libgcov.
>> Since we only link libatomic when -fprofile-gen-atomic is specified,
>> we have to make the atomic function weak -- otherwise, there is a
>> unsat for regular FDO gen build (of course, when the builtin is not
>> expanded).
>>
>> An alternative it to always link libatomic together with libgcov. Then
>> we don't need the weak stuff. I'm not sure when one is better.
>>
>> -Rong
>>
>> On Mon, Jan 7, 2013 at 12:36 PM, Richard Henderson <rth@redhat.com> wrote:
>>> On 01/03/2013 04:42 PM, Rong Xu wrote:
>>>> It links libatomic when -fprofile-gen-atomic is specified for FDO
>>>> instrumentation build. Here I assume libatomic is always installed.
>>>> Andrew: do you think if this is reasonable?
>>>>
>>>> It also disables the functionality if target does not support weak
>>>> (ie. TARGET_SUPPORTS_WEAK == 0).
>>>
>>> Since you're linking libatomic, you don't need weak references.
>>>
>>> I think its ok to assume libatomic is installed, given that the
>>> user has had to explicitly use the command-line option.
>>>
>>>
>>> r~

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

On Tue, Nov 19, 2013 at 5:11 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Tue, Nov 19, 2013 at 5:02 PM, Rong Xu <xur@google.com> wrote:
>> Hi all,
>>
>> I merged this old patch with current trunk. I also make the following changes
>> (1) not using weak references. Now every *profile_atomic() has it's
>> own .o so that none of them will be in the final binary if
>> -fprofile-generate-atomic is not specified.
>> (2) more value profilers have the atomic version.
>> (3) not link to libatomic. I used to link the libatomic in the
>> presence of -fprofile-generate-atomic, per Andrew's suggestion. It
>> used to work. But now if I can add -latomic in the SPEC, it cannot
>> find the libatomic.so.1 (unless I specify the PATH). I did not find an
>> easy way to statically link libatomic.a. Andrew: Do you have any
>> suggestion? Or should we let the user link to libatomic.a if the
>> builtins are not expanded?
>
> It should work for an installed GCC.  For testing you might need
> something that is included inside testsuite/lib/atomic-dg.exp which
> sets the library path to include libatomic build directory.

When I change the SPEC to include libatomic,
the compiler can find libatomic. I.e. using
>> gcc -O2 -fprofile-generate -fprofile-generate-atomic=3 hello_world.c
generates a.out without any problem.

But since there are both shared and static libatomic in lib64, it
chooses to use the dynamic one.
>> ldd a.out
linux-vdso.so.1 =>  (0x00007fff56bff000)
libatomic.so.1 => not found
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00002b0720261000)
/lib64/ld-linux-x86-64.so.2 (0x00002b072003c000)

>> ./a.out
./a.out: error while loading shared libraries: libatomic.so.1: cannot
open shared object file: No such file or directory

while
>> LD_LIBRARY_PATH=<gcc_install_patch>/lib64 ./a.out
works fine.

I think that's the same reason we set the library path in
testsuite/lib/atomic-dg.exp, because <gcc_install_patch>/lib64
is not in the dynamic library search list.

I could do this in the SPEC
  -Wl,-Bstatic -latomic -Wl,-Bdynamic
which would link libatomic statically.
I works for me. But it looks a little weird in gcc driver.

Index: gcc.c
===================================================================
--- gcc.c       (revision 205053)
+++ gcc.c       (working copy)
@@ -771,7 +771,8 @@
     %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)}\
     %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
     %(mflib) " STACK_SPLIT_SPEC "\
-    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} " SANITIZER_SPEC " \
+    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov\
+      %{fprofile-generate-atomic=*:-Wl,-Bstatic -latomic
-Wl,-Bdynamic}} " SANITIZER_SPEC " \
     %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
     %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}"
 #endif



> I think now we require libatomic in more cases (C11 atomic support for
> an example).
>
> Thanks,
> Andrew Pinski
>
>>
>> Is this OK for trunk?
>>
>> Thanks,
>>
>> -Rong
>>
>> On Mon, Jan 7, 2013 at 12:55 PM, Rong Xu <xur@google.com> wrote:
>>> Function __gcov_indirect_call_profiler_atomic (which contains call to
>>> the atomic function) is always emitted in libgcov.
>>> Since we only link libatomic when -fprofile-gen-atomic is specified,
>>> we have to make the atomic function weak -- otherwise, there is a
>>> unsat for regular FDO gen build (of course, when the builtin is not
>>> expanded).
>>>
>>> An alternative it to always link libatomic together with libgcov. Then
>>> we don't need the weak stuff. I'm not sure when one is better.
>>>
>>> -Rong
>>>
>>> On Mon, Jan 7, 2013 at 12:36 PM, Richard Henderson <rth@redhat.com> wrote:
>>>> On 01/03/2013 04:42 PM, Rong Xu wrote:
>>>>> It links libatomic when -fprofile-gen-atomic is specified for FDO
>>>>> instrumentation build. Here I assume libatomic is always installed.
>>>>> Andrew: do you think if this is reasonable?
>>>>>
>>>>> It also disables the functionality if target does not support weak
>>>>> (ie. TARGET_SUPPORTS_WEAK == 0).
>>>>
>>>> Since you're linking libatomic, you don't need weak references.
>>>>
>>>> I think its ok to assume libatomic is installed, given that the
>>>> user has had to explicitly use the command-line option.
>>>>
>>>>
>>>> r~

Re: atomic update of profile counters (issue7000044)

[Andrew Pinski]

On Wed, Nov 20, 2013 at 10:44 AM, Rong Xu <xur@google.com> wrote:
> On Tue, Nov 19, 2013 at 5:11 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Tue, Nov 19, 2013 at 5:02 PM, Rong Xu <xur@google.com> wrote:
>>> Hi all,
>>>
>>> I merged this old patch with current trunk. I also make the following changes
>>> (1) not using weak references. Now every *profile_atomic() has it's
>>> own .o so that none of them will be in the final binary if
>>> -fprofile-generate-atomic is not specified.
>>> (2) more value profilers have the atomic version.
>>> (3) not link to libatomic. I used to link the libatomic in the
>>> presence of -fprofile-generate-atomic, per Andrew's suggestion. It
>>> used to work. But now if I can add -latomic in the SPEC, it cannot
>>> find the libatomic.so.1 (unless I specify the PATH). I did not find an
>>> easy way to statically link libatomic.a. Andrew: Do you have any
>>> suggestion? Or should we let the user link to libatomic.a if the
>>> builtins are not expanded?
>>
>> It should work for an installed GCC.  For testing you might need
>> something that is included inside testsuite/lib/atomic-dg.exp which
>> sets the library path to include libatomic build directory.
>
> When I change the SPEC to include libatomic,
> the compiler can find libatomic. I.e. using
>>> gcc -O2 -fprofile-generate -fprofile-generate-atomic=3 hello_world.c
> generates a.out without any problem.
>
> But since there are both shared and static libatomic in lib64, it
> chooses to use the dynamic one.
>>> ldd a.out
> linux-vdso.so.1 =>  (0x00007fff56bff000)
> libatomic.so.1 => not found
> libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00002b0720261000)
> /lib64/ld-linux-x86-64.so.2 (0x00002b072003c000)
>
>>> ./a.out
> ./a.out: error while loading shared libraries: libatomic.so.1: cannot
> open shared object file: No such file or directory
>
> while
>>> LD_LIBRARY_PATH=<gcc_install_patch>/lib64 ./a.out
> works fine.


I don't see this as an issue really as you have the same issue with
all the target libraries (not limited to libatomic or libgomp or
libgfortran).

Thanks,
Andrew Pinski

>
> I think that's the same reason we set the library path in
> testsuite/lib/atomic-dg.exp, because <gcc_install_patch>/lib64
> is not in the dynamic library search list.
>
> I could do this in the SPEC
>   -Wl,-Bstatic -latomic -Wl,-Bdynamic
> which would link libatomic statically.
> I works for me. But it looks a little weird in gcc driver.
>
> Index: gcc.c
> ===================================================================
> --- gcc.c       (revision 205053)
> +++ gcc.c       (working copy)
> @@ -771,7 +771,8 @@
>      %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)}\
>      %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
>      %(mflib) " STACK_SPLIT_SPEC "\
> -    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} " SANITIZER_SPEC " \
> +    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov\
> +      %{fprofile-generate-atomic=*:-Wl,-Bstatic -latomic
> -Wl,-Bdynamic}} " SANITIZER_SPEC " \
>      %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
>      %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}"
>  #endif
>
>
>
>> I think now we require libatomic in more cases (C11 atomic support for
>> an example).
>>
>> Thanks,
>> Andrew Pinski
>>
>>>
>>> Is this OK for trunk?
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>> On Mon, Jan 7, 2013 at 12:55 PM, Rong Xu <xur@google.com> wrote:
>>>> Function __gcov_indirect_call_profiler_atomic (which contains call to
>>>> the atomic function) is always emitted in libgcov.
>>>> Since we only link libatomic when -fprofile-gen-atomic is specified,
>>>> we have to make the atomic function weak -- otherwise, there is a
>>>> unsat for regular FDO gen build (of course, when the builtin is not
>>>> expanded).
>>>>
>>>> An alternative it to always link libatomic together with libgcov. Then
>>>> we don't need the weak stuff. I'm not sure when one is better.
>>>>
>>>> -Rong
>>>>
>>>> On Mon, Jan 7, 2013 at 12:36 PM, Richard Henderson <rth@redhat.com> wrote:
>>>>> On 01/03/2013 04:42 PM, Rong Xu wrote:
>>>>>> It links libatomic when -fprofile-gen-atomic is specified for FDO
>>>>>> instrumentation build. Here I assume libatomic is always installed.
>>>>>> Andrew: do you think if this is reasonable?
>>>>>>
>>>>>> It also disables the functionality if target does not support weak
>>>>>> (ie. TARGET_SUPPORTS_WEAK == 0).
>>>>>
>>>>> Since you're linking libatomic, you don't need weak references.
>>>>>
>>>>> I think its ok to assume libatomic is installed, given that the
>>>>> user has had to explicitly use the command-line option.
>>>>>
>>>>>
>>>>> r~

Re: atomic update of profile counters (issue7000044)

[Andrew Pinski]

On Wed, Nov 20, 2013 at 10:48 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Nov 20, 2013 at 10:44 AM, Rong Xu <xur@google.com> wrote:
>> On Tue, Nov 19, 2013 at 5:11 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Tue, Nov 19, 2013 at 5:02 PM, Rong Xu <xur@google.com> wrote:
>>>> Hi all,
>>>>
>>>> I merged this old patch with current trunk. I also make the following changes
>>>> (1) not using weak references. Now every *profile_atomic() has it's
>>>> own .o so that none of them will be in the final binary if
>>>> -fprofile-generate-atomic is not specified.
>>>> (2) more value profilers have the atomic version.
>>>> (3) not link to libatomic. I used to link the libatomic in the
>>>> presence of -fprofile-generate-atomic, per Andrew's suggestion. It
>>>> used to work. But now if I can add -latomic in the SPEC, it cannot
>>>> find the libatomic.so.1 (unless I specify the PATH). I did not find an
>>>> easy way to statically link libatomic.a. Andrew: Do you have any
>>>> suggestion? Or should we let the user link to libatomic.a if the
>>>> builtins are not expanded?
>>>
>>> It should work for an installed GCC.  For testing you might need
>>> something that is included inside testsuite/lib/atomic-dg.exp which
>>> sets the library path to include libatomic build directory.
>>
>> When I change the SPEC to include libatomic,
>> the compiler can find libatomic. I.e. using
>>>> gcc -O2 -fprofile-generate -fprofile-generate-atomic=3 hello_world.c
>> generates a.out without any problem.
>>
>> But since there are both shared and static libatomic in lib64, it
>> chooses to use the dynamic one.
>>>> ldd a.out
>> linux-vdso.so.1 =>  (0x00007fff56bff000)
>> libatomic.so.1 => not found
>> libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00002b0720261000)
>> /lib64/ld-linux-x86-64.so.2 (0x00002b072003c000)
>>
>>>> ./a.out
>> ./a.out: error while loading shared libraries: libatomic.so.1: cannot
>> open shared object file: No such file or directory
>>
>> while
>>>> LD_LIBRARY_PATH=<gcc_install_patch>/lib64 ./a.out
>> works fine.
>
>
> I don't see this as an issue really as you have the same issue with
> all the target libraries (not limited to libatomic or libgomp or
> libgfortran).

You also also use --as-needed/--no-as-needed wrapped around the
-latomic too;  USE_LD_AS_NEEDED is needed to be used to check for
--as-needed support and LD_AS_NEEDED_OPTION/LD_NO_AS_NEEDED_OPTION
should be used instead of directly --as-needed/--no-as-needed.

>
> Thanks,
> Andrew Pinski
>
>>
>> I think that's the same reason we set the library path in
>> testsuite/lib/atomic-dg.exp, because <gcc_install_patch>/lib64
>> is not in the dynamic library search list.
>>
>> I could do this in the SPEC
>>   -Wl,-Bstatic -latomic -Wl,-Bdynamic
>> which would link libatomic statically.
>> I works for me. But it looks a little weird in gcc driver.
>>
>> Index: gcc.c
>> ===================================================================
>> --- gcc.c       (revision 205053)
>> +++ gcc.c       (working copy)
>> @@ -771,7 +771,8 @@
>>      %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)}\
>>      %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
>>      %(mflib) " STACK_SPLIT_SPEC "\
>> -    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} " SANITIZER_SPEC " \
>> +    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov\
>> +      %{fprofile-generate-atomic=*:-Wl,-Bstatic -latomic
>> -Wl,-Bdynamic}} " SANITIZER_SPEC " \
>>      %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
>>      %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}"
>>  #endif
>>
>>
>>
>>> I think now we require libatomic in more cases (C11 atomic support for
>>> an example).
>>>
>>> Thanks,
>>> Andrew Pinski
>>>
>>>>
>>>> Is this OK for trunk?
>>>>
>>>> Thanks,
>>>>
>>>> -Rong
>>>>
>>>> On Mon, Jan 7, 2013 at 12:55 PM, Rong Xu <xur@google.com> wrote:
>>>>> Function __gcov_indirect_call_profiler_atomic (which contains call to
>>>>> the atomic function) is always emitted in libgcov.
>>>>> Since we only link libatomic when -fprofile-gen-atomic is specified,
>>>>> we have to make the atomic function weak -- otherwise, there is a
>>>>> unsat for regular FDO gen build (of course, when the builtin is not
>>>>> expanded).
>>>>>
>>>>> An alternative it to always link libatomic together with libgcov. Then
>>>>> we don't need the weak stuff. I'm not sure when one is better.
>>>>>
>>>>> -Rong
>>>>>
>>>>> On Mon, Jan 7, 2013 at 12:36 PM, Richard Henderson <rth@redhat.com> wrote:
>>>>>> On 01/03/2013 04:42 PM, Rong Xu wrote:
>>>>>>> It links libatomic when -fprofile-gen-atomic is specified for FDO
>>>>>>> instrumentation build. Here I assume libatomic is always installed.
>>>>>>> Andrew: do you think if this is reasonable?
>>>>>>>
>>>>>>> It also disables the functionality if target does not support weak
>>>>>>> (ie. TARGET_SUPPORTS_WEAK == 0).
>>>>>>
>>>>>> Since you're linking libatomic, you don't need weak references.
>>>>>>
>>>>>> I think its ok to assume libatomic is installed, given that the
>>>>>> user has had to explicitly use the command-line option.
>>>>>>
>>>>>>
>>>>>> r~

Re: atomic update of profile counters (issue7000044)

[Joseph S. Myers]

On Wed, 20 Nov 2013, Rong Xu wrote:

> I could do this in the SPEC
>   -Wl,-Bstatic -latomic -Wl,-Bdynamic
> which would link libatomic statically.
> I works for me. But it looks a little weird in gcc driver.

I think we should generally link libatomic with --as-needed by default on 
platforms supporting --as-needed, in line with the general principle that 
C code just using language not library facilities (_Atomic in this case) 
shouldn't need any special options to link it (libatomic is like libgcc, 
which is linked in automatically); the trickier question is what to do 
with it on any systems supporting shared libraries but not --as-needed.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

Joseph and Andrew, thanks for the suggestion. That's really helpful.

Here is the new patch for gcc.c.
Basically, it's just what you have suggested: enclosing -latomic with
--as-needed, and using macros.
For the case of no --as-needed support, I use static link. (just found
that some code already using this in the SPEC).
I'm flexible on this part -- if you think this is unnecessary, I can remove.

Thanks,

-Rong

Index: gcc.c
===================================================================
--- gcc.c       (revision 205053)
+++ gcc.c       (working copy)
@@ -748,6 +748,23 @@ proper position among the other output files.  */
     %{fvtable-verify=preinit: -lvtv -u_vtable_map_vars_start
-u_vtable_map_vars_end}}"
 #endif

+/* This spec is for linking in libatomic in gcov atomic counter update.
+   We will use the atomic functions defined in libatomic, only when the builtin
+   versions are not available. In the case of no LD_AS_NEEDED support, we
+   link libatomic statically.  */
+
+#ifndef GCOV_ATOMIC_SPEC
+#if USE_LD_AS_NEEDED
+#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_AS_NEEDED_OPTION \
+  " -latomic} " LD_NO_AS_NEEDED_OPTION
+#elif defined(HAVE_LD_STATIC_DYNAMIC)
+#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_STATIC_OPTION \
+                    " -latomic " LD_DYNAMIC_OPTION "}"
+#else /* !USE_LD_AS_NEEDED && !HAVE_LD_STATIC_DYNAMIC  */
+#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:-latomic}"
+#endif
+#endif
+
 /* -u* was put back because both BSD and SysV seem to support it.  */
 /* %{static:} simply prevents an error message if the target machine
    doesn't handle -static.  */
@@ -771,7 +788,8 @@ proper position among the other output files.  */
     %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)}\
     %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
     %(mflib) " STACK_SPLIT_SPEC "\
-    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} " SANITIZER_SPEC " \
+    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov\
+      " GCOV_ATOMIC_SPEC "} " SANITIZER_SPEC " \
     %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
     %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}"



On Wed, Nov 20, 2013 at 1:04 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Wed, 20 Nov 2013, Rong Xu wrote:
>
>> I could do this in the SPEC
>>   -Wl,-Bstatic -latomic -Wl,-Bdynamic
>> which would link libatomic statically.
>> I works for me. But it looks a little weird in gcc driver.
>
> I think we should generally link libatomic with --as-needed by default on
> platforms supporting --as-needed, in line with the general principle that
> C code just using language not library facilities (_Atomic in this case)
> shouldn't need any special options to link it (libatomic is like libgcc,
> which is linked in automatically); the trickier question is what to do
> with it on any systems supporting shared libraries but not --as-needed.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com

Re: atomic update of profile counters (issue7000044)

[Andrew Pinski]

On Wed, Nov 20, 2013 at 2:07 PM, Rong Xu <xur@google.com> wrote:
> Joseph and Andrew, thanks for the suggestion. That's really helpful.
>
> Here is the new patch for gcc.c.
> Basically, it's just what you have suggested: enclosing -latomic with
> --as-needed, and using macros.
> For the case of no --as-needed support, I use static link. (just found
> that some code already using this in the SPEC).
> I'm flexible on this part -- if you think this is unnecessary, I can remove.


I think Joseph's suggestion was also to include -latomic even when not
generating atomic profiling due to the C11 code requiring it.

Thanks,
Andrew

>
> Thanks,
>
> -Rong
>
> Index: gcc.c
> ===================================================================
> --- gcc.c       (revision 205053)
> +++ gcc.c       (working copy)
> @@ -748,6 +748,23 @@ proper position among the other output files.  */
>      %{fvtable-verify=preinit: -lvtv -u_vtable_map_vars_start
> -u_vtable_map_vars_end}}"
>  #endif
>
> +/* This spec is for linking in libatomic in gcov atomic counter update.
> +   We will use the atomic functions defined in libatomic, only when the builtin
> +   versions are not available. In the case of no LD_AS_NEEDED support, we
> +   link libatomic statically.  */
> +
> +#ifndef GCOV_ATOMIC_SPEC
> +#if USE_LD_AS_NEEDED
> +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_AS_NEEDED_OPTION \
> +  " -latomic} " LD_NO_AS_NEEDED_OPTION
> +#elif defined(HAVE_LD_STATIC_DYNAMIC)
> +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_STATIC_OPTION \
> +                    " -latomic " LD_DYNAMIC_OPTION "}"
> +#else /* !USE_LD_AS_NEEDED && !HAVE_LD_STATIC_DYNAMIC  */
> +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:-latomic}"
> +#endif
> +#endif
> +
>  /* -u* was put back because both BSD and SysV seem to support it.  */
>  /* %{static:} simply prevents an error message if the target machine
>     doesn't handle -static.  */
> @@ -771,7 +788,8 @@ proper position among the other output files.  */
>      %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)}\
>      %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
>      %(mflib) " STACK_SPLIT_SPEC "\
> -    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} " SANITIZER_SPEC " \
> +    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov\
> +      " GCOV_ATOMIC_SPEC "} " SANITIZER_SPEC " \
>      %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
>      %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}"
>
>
>
> On Wed, Nov 20, 2013 at 1:04 PM, Joseph S. Myers
> <joseph@codesourcery.com> wrote:
>> On Wed, 20 Nov 2013, Rong Xu wrote:
>>
>>> I could do this in the SPEC
>>>   -Wl,-Bstatic -latomic -Wl,-Bdynamic
>>> which would link libatomic statically.
>>> I works for me. But it looks a little weird in gcc driver.
>>
>> I think we should generally link libatomic with --as-needed by default on
>> platforms supporting --as-needed, in line with the general principle that
>> C code just using language not library facilities (_Atomic in this case)
>> shouldn't need any special options to link it (libatomic is like libgcc,
>> which is linked in automatically); the trickier question is what to do
>> with it on any systems supporting shared libraries but not --as-needed.
>>
>> --
>> Joseph S. Myers
>> joseph@codesourcery.com

Re: atomic update of profile counters (issue7000044)

[Rong Xu]

OK. Sorry for miss-reading the message.
In that case, linking in libatomic becomes a separate issue. We don't
need to touch gcc.c in this patch.

Thanks,

-Rong

On Wed, Nov 20, 2013 at 2:19 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Nov 20, 2013 at 2:07 PM, Rong Xu <xur@google.com> wrote:
>> Joseph and Andrew, thanks for the suggestion. That's really helpful.
>>
>> Here is the new patch for gcc.c.
>> Basically, it's just what you have suggested: enclosing -latomic with
>> --as-needed, and using macros.
>> For the case of no --as-needed support, I use static link. (just found
>> that some code already using this in the SPEC).
>> I'm flexible on this part -- if you think this is unnecessary, I can remove.
>
>
> I think Joseph's suggestion was also to include -latomic even when not
> generating atomic profiling due to the C11 code requiring it.
>
> Thanks,
> Andrew
>
>>
>> Thanks,
>>
>> -Rong
>>
>> Index: gcc.c
>> ===================================================================
>> --- gcc.c       (revision 205053)
>> +++ gcc.c       (working copy)
>> @@ -748,6 +748,23 @@ proper position among the other output files.  */
>>      %{fvtable-verify=preinit: -lvtv -u_vtable_map_vars_start
>> -u_vtable_map_vars_end}}"
>>  #endif
>>
>> +/* This spec is for linking in libatomic in gcov atomic counter update.
>> +   We will use the atomic functions defined in libatomic, only when the builtin
>> +   versions are not available. In the case of no LD_AS_NEEDED support, we
>> +   link libatomic statically.  */
>> +
>> +#ifndef GCOV_ATOMIC_SPEC
>> +#if USE_LD_AS_NEEDED
>> +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_AS_NEEDED_OPTION \
>> +  " -latomic} " LD_NO_AS_NEEDED_OPTION
>> +#elif defined(HAVE_LD_STATIC_DYNAMIC)
>> +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_STATIC_OPTION \
>> +                    " -latomic " LD_DYNAMIC_OPTION "}"
>> +#else /* !USE_LD_AS_NEEDED && !HAVE_LD_STATIC_DYNAMIC  */
>> +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:-latomic}"
>> +#endif
>> +#endif
>> +
>>  /* -u* was put back because both BSD and SysV seem to support it.  */
>>  /* %{static:} simply prevents an error message if the target machine
>>     doesn't handle -static.  */
>> @@ -771,7 +788,8 @@ proper position among the other output files.  */
>>      %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)}\
>>      %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
>>      %(mflib) " STACK_SPLIT_SPEC "\
>> -    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} " SANITIZER_SPEC " \
>> +    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov\
>> +      " GCOV_ATOMIC_SPEC "} " SANITIZER_SPEC " \
>>      %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
>>      %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}"
>>
>>
>>
>> On Wed, Nov 20, 2013 at 1:04 PM, Joseph S. Myers
>> <joseph@codesourcery.com> wrote:
>>> On Wed, 20 Nov 2013, Rong Xu wrote:
>>>
>>>> I could do this in the SPEC
>>>>   -Wl,-Bstatic -latomic -Wl,-Bdynamic
>>>> which would link libatomic statically.
>>>> I works for me. But it looks a little weird in gcc driver.
>>>
>>> I think we should generally link libatomic with --as-needed by default on
>>> platforms supporting --as-needed, in line with the general principle that
>>> C code just using language not library facilities (_Atomic in this case)
>>> shouldn't need any special options to link it (libatomic is like libgcc,
>>> which is linked in automatically); the trickier question is what to do
>>> with it on any systems supporting shared libraries but not --as-needed.
>>>
>>> --
>>> Joseph S. Myers
>>> joseph@codesourcery.com

Re: atomic update of profile counters (issue7000044)

[Jan Hubicka]

> 2013-11-19  Rong Xu  <xur@google.com>
> 
> 	* gcc/gcov-io.h: Add atomic function macros for compiler use.
> 	* gcc/common.opt (fprofile-generate-atomic): New option.
> 	* gcc/tree-profile.c (gimple_init_edge_profiler): Support for
>         atomic counter update.
> 	(gimple_gen_edge_profiler): Ditto.
> 	* libgcc/libgcov-profiler.c 
> 	(__gcov_interval_profiler_atomic): Ditto.
> 	(__gcov_pow2_profiler_atomic): Ditto.
> 	(__gcov_one_value_profiler_body_atomic): Ditto.
> 	(__gcov_one_value_profiler_atomic): Ditto.
> 	(__gcov_indirect_call_profiler_atomic): Ditto.
> 	(__gcov_indirect_call_profiler_v2_atomic): Ditto.
> 	(__gcov_time_profiler_atomic): Ditto.
> 	(__gcov_average_profiler_atomic): Ditto.
> 	* gcc/gcc.c: Link libatomic when -fprofile-generate-atomic used.
> 	* libgcc/Makefile.in: Add atomic objects.
> 
> Index: gcc/common.opt
> ===================================================================
> --- gcc/common.opt	(revision 205053)
> +++ gcc/common.opt	(working copy)
> @@ -1684,6 +1684,15 @@ fprofile-correction
>  Common Report Var(flag_profile_correction)
>  Enable correction of flow inconsistent profile data input
>  
> +; fprofile-generate-atomic=0: default and disable atomical update.
> +; fprofile-generate-atomic=1: atomically update edge profile counters.
> +; fprofile-generate-atomic=2: atomically update value profile counters.
> +; fprofile-generate-atomic=3: atomically update edge and value profile counters.
> +; other values will be ignored (fall back to the default of 0).
> +fprofile-generate-atomic=
> +Common Joined UInteger Report Var(flag_profile_generate_atomic) Init(3) Optimization
> +fprofile-generate-atomic=[0..3] Atomical increments of profile counters.

Instead magic numbers I would preffer flags, like "edge" and "value"
and permiting combinations like -ffprofile-generate-atomic=edge,value

I wonder about the option name, I suppose this option also combine with -ftest-coverage
(and no longer too useful -fprofile-arcs), so the profile-generate prefix may be bit
misleading here (giving an impression that it is not useful in this case).
But I can't think of something really better.

Other thing I wonder about is that we may want to implement alternative solution with
TLS or smaller per-thread buffers and locked updates.  It would be bit difficult to 
extend -fprofile-generate-atomic to this...

> @@ -155,6 +155,9 @@ gimple_init_edge_profiler (void)
>    tree ic_profiler_fn_type;
>    tree average_profiler_fn_type;
>    tree time_profiler_fn_type;
> +  const char *fn_name;
> +  bool profile_gen_value_atomic = (flag_profile_generate_atomic == 2 ||
> +                                   flag_profile_generate_atomic == 3);
>  
>    if (!gcov_type_node)
>      {

Will this work with optimization attributes?

The rest of patch looks OK, lets settle option name and get it in.
Sorry for dropping the ball for 4.8.