Re: [PATCH] Detect whether target can use -fprofile-update=atomic

["Martin Liška" <mliska@suse.cz>]

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

On 09/06/2016 02:51 PM, Jakub Jelinek wrote:
> On Tue, Sep 06, 2016 at 02:45:32PM +0200, Martin Liška wrote:
>> --- a/gcc/tree-profile.c
>> +++ b/gcc/tree-profile.c
>> @@ -528,6 +528,13 @@ gimple_gen_ior_profiler (histogram_value value, unsigned tag, unsigned base)
>>    gsi_insert_before (&gsi, call, GSI_NEW_STMT);
>>  }
>>  
>> +#ifndef HAVE_sync_compare_and_swapsi
>> +#define HAVE_sync_compare_and_swapsi 0
>> +#endif
>> +#ifndef HAVE_atomic_compare_and_swapsi
>> +#define HAVE_atomic_compare_and_swapsi 0
>> +#endif
>> +
>>  /* Profile all functions in the callgraph.  */
>>  
>>  static unsigned int
>> @@ -535,6 +542,16 @@ tree_profiling (void)
>>  {
>>    struct cgraph_node *node;
>>  
>> +  /* Verify whether we can utilize atomic update operations.  */
>> +  if (flag_profile_update == PROFILE_UPDATE_ATOMIC
>> +      && !HAVE_sync_compare_and_swapsi
>> +      && !HAVE_atomic_compare_and_swapsi)
> 
> This isn't in sync with:
> 
>> +/* Detect whether target can support atomic update of profilers.  */
>> +#if LONG_LONG_TYPE_SIZE <= 32 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
>> +#define GCOV_SUPPORTS_ATOMIC 1
>> +#else
>> +#if LONG_LONG_TYPE_SIZE > 32 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
>> +#define GCOV_SUPPORTS_ATOMIC 1
>> +#else
>> +#define GCOV_SUPPORTS_ATOMIC 0
>> +#endif
>> +#endif
> 
> this.  Either you implement the poor man's 64-bit atomics with 32-bit cas
> and adjust the latter, or the former needs to look at the target's gcov type
> (long long always?) and depending on its size either test the HAVE_*si or
> HAVE_*di macros.
> 
> 	Jakub
> 

Ok, thanks, this should be the proper patch, where I distinguish sizeof(gcov_type) and
use appropriate GAVE_*{s,d}i macros.

Ready for trunk?
Thanks,
Martin

[-- Attachment #2: 0001-PATCH-Detect-whether-target-can-use-fprofile-update-.patch --]
[-- Type: text/x-patch, Size: 6027 bytes --]

From 41bef1e975042071c973c3cb733a0e0d9a59fec6 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 6 Sep 2016 14:35:52 +0200
Subject: [PATCH] [PATCH] Detect whether target can use -fprofile-update=atomic

libgcc/ChangeLog:

2016-09-06  Martin Liska  <mliska@suse.cz>

	* libgcov-profiler.c: Use __GCC_HAVE_SYNC_COMPARE_AND_SWAP_{4,8} to
	conditionaly enable/disable *_atomic functions.

gcc/ChangeLog:

2016-09-06  Martin Liska  <mliska@suse.cz>

	* tree-profile.c (tree_profiling): Detect whether target can use
	-fprofile-update=atomic.

gcc/testsuite/ChangeLog:

2016-09-06  Martin Liska  <mliska@suse.cz>

	* gcc.dg/profile-update-warning.c: New test.
---
 gcc/testsuite/gcc.dg/profile-update-warning.c |  7 ++++++
 gcc/tree-profile.c                            | 35 +++++++++++++++++++++++++++
 libgcc/libgcov-profiler.c                     | 24 ++++++++++++++----
 3 files changed, 61 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/profile-update-warning.c

diff --git a/gcc/testsuite/gcc.dg/profile-update-warning.c b/gcc/testsuite/gcc.dg/profile-update-warning.c
new file mode 100644
index 0000000..0614fad
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/profile-update-warning.c
@@ -0,0 +1,7 @@
+/* { dg-do compile { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-options "-fprofile-update=atomic -fprofile-generate -march=i386 -m32" } */
+
+int main(int argc, char *argv[])
+{
+  return 0;
+} /* { dg-warning "target does not support atomic profile update, single mode is selected" } */
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index 622869e..a3e6dca 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -528,6 +528,20 @@ gimple_gen_ior_profiler (histogram_value value, unsigned tag, unsigned base)
   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
 }
 
+#ifndef HAVE_sync_compare_and_swapsi
+#define HAVE_sync_compare_and_swapsi 0
+#endif
+#ifndef HAVE_atomic_compare_and_swapsi
+#define HAVE_atomic_compare_and_swapsi 0
+#endif
+
+#ifndef HAVE_sync_compare_and_swapdi
+#define HAVE_sync_compare_and_swapdi 0
+#endif
+#ifndef HAVE_atomic_compare_and_swapdi
+#define HAVE_atomic_compare_and_swapdi 0
+#endif
+
 /* Profile all functions in the callgraph.  */
 
 static unsigned int
@@ -535,6 +549,27 @@ tree_profiling (void)
 {
   struct cgraph_node *node;
 
+  /* Verify whether we can utilize atomic update operations.  */
+  if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
+    {
+      bool can_support = false;
+      if (sizeof (gcov_type) == 4)
+	can_support
+	  = HAVE_sync_compare_and_swapsi || HAVE_atomic_compare_and_swapsi;
+      else if (sizeof (gcov_type) == 8)
+	can_support
+	  = HAVE_sync_compare_and_swapdi || HAVE_atomic_compare_and_swapdi;
+      else
+	gcc_unreachable ();
+
+      if (!can_support)
+      {
+	warning (0, "target does not support atomic profile update, "
+		 "single mode is selected");
+	flag_profile_update = PROFILE_UPDATE_SINGLE;
+      }
+    }
+
   /* This is a small-ipa pass that gets called only once, from
      cgraphunit.c:ipa_passes().  */
   gcc_assert (symtab->state == IPA_SSA);
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index 70a821d..887041f 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -24,8 +24,20 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 <http://www.gnu.org/licenses/>.  */
 
 #include "libgcov.h"
+#include "auto-target.h"
 #if !defined(inhibit_libc)
 
+/* Detect whether target can support atomic update of profilers.  */
+#if LONG_LONG_TYPE_SIZE <= 32 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
+#define GCOV_SUPPORTS_ATOMIC 1
+#else
+#if LONG_LONG_TYPE_SIZE > 32 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
+#define GCOV_SUPPORTS_ATOMIC 1
+#else
+#define GCOV_SUPPORTS_ATOMIC 0
+#endif
+#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
@@ -46,7 +58,7 @@ __gcov_interval_profiler (gcov_type *counters, gcov_type value,
 }
 #endif
 
-#ifdef L_gcov_interval_profiler_atomic
+#if defined(L_gcov_interval_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
 /* If VALUE is in interval <START, START + STEPS - 1>, then increases the
    corresponding counter in COUNTERS.  If the VALUE is above or below
    the interval, COUNTERS[STEPS] or COUNTERS[STEPS + 1] is increased
@@ -80,7 +92,7 @@ __gcov_pow2_profiler (gcov_type *counters, gcov_type value)
 }
 #endif
 
-#ifdef L_gcov_pow2_profiler_atomic
+#if defined(L_gcov_pow2_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
 /* If VALUE is a power of two, COUNTERS[1] is incremented.  Otherwise
    COUNTERS[0] is incremented.  Function is thread-safe.  */
 
@@ -134,7 +146,7 @@ __gcov_one_value_profiler (gcov_type *counters, gcov_type value)
 }
 #endif
 
-#ifdef L_gcov_one_value_profiler_atomic
+#if defined(L_gcov_one_value_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
 
 /* Update one value profilers (COUNTERS) for a given VALUE.
 
@@ -342,6 +354,7 @@ __gcov_time_profiler (gcov_type* counters)
     counters[0] = ++function_counter;
 }
 
+#if GCOV_SUPPORTS_ATOMIC
 /* Sets corresponding COUNTERS if there is no value.
    Function is thread-safe.  */
 
@@ -352,6 +365,7 @@ __gcov_time_profiler_atomic (gcov_type* counters)
     counters[0] = __atomic_add_fetch (&function_counter, 1, MEMMODEL_RELAXED);
 }
 #endif
+#endif
 
 
 #ifdef L_gcov_average_profiler
@@ -366,7 +380,7 @@ __gcov_average_profiler (gcov_type *counters, gcov_type value)
 }
 #endif
 
-#ifdef L_gcov_average_profiler_atomic
+#if defined(L_gcov_average_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
 /* Increase corresponding COUNTER by VALUE.  FIXME: Perhaps we want
    to saturate up.  Function is thread-safe.  */
 
@@ -388,7 +402,7 @@ __gcov_ior_profiler (gcov_type *counters, gcov_type value)
 }
 #endif
 
-#ifdef L_gcov_ior_profiler_atomic
+#if defined(L_gcov_ior_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
 /* Bitwise-OR VALUE into COUNTER.  Function is thread-safe.  */
 
 void
-- 
2.9.2