[PATCH] gcov: No atomic ops for -fprofile-update=single

[PATCH] gcov: No atomic ops for -fprofile-update=single

[Sebastian Huber]

gcc/ChangeLog:

	PR tree-optimization/112678

	* tree-profile.cc (tree_profiling): Do not use atomic operations
	for -fprofile-update=single.
---
 gcc/tree-profile.cc | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
index 1ac0fdb3bc98..9c8fdb8b18f4 100644
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -767,6 +767,7 @@ tree_profiling (void)
     = HAVE_sync_compare_and_swapsi || HAVE_atomic_compare_and_swapsi;
   bool have_atomic_8
     = HAVE_sync_compare_and_swapdi || HAVE_atomic_compare_and_swapdi;
+  bool needs_split = gcov_type_size == 8 && !have_atomic_8 && have_atomic_4;
   if (!can_support_atomic)
     {
       if (gcov_type_size == 4)
@@ -775,6 +776,9 @@ tree_profiling (void)
 	can_support_atomic = have_atomic_8;
     }
 
+  if (flag_profile_update != PROFILE_UPDATE_SINGLE && needs_split)
+    counter_update = COUNTER_UPDATE_ATOMIC_PARTIAL;
+
   if (flag_profile_update == PROFILE_UPDATE_ATOMIC
       && !can_support_atomic)
     {
@@ -788,13 +792,11 @@ tree_profiling (void)
 
   if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
     {
-      if (gcov_type_size == 8 && !have_atomic_8 && have_atomic_4)
+      if (needs_split)
 	counter_update = COUNTER_UPDATE_ATOMIC_SPLIT;
       else
 	counter_update = COUNTER_UPDATE_ATOMIC_BUILTIN;
     }
-  else if (gcov_type_size == 8 && have_atomic_4)
-    counter_update = COUNTER_UPDATE_ATOMIC_PARTIAL;
 
   /* This is a small-ipa pass that gets called only once, from
      cgraphunit.cc:ipa_passes().  */
-- 
2.35.3

Re: [PATCH] gcov: No atomic ops for -fprofile-update=single

[Richard Biener]

On Thu, Nov 23, 2023 at 2:47 PM Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
>
> gcc/ChangeLog:
>         PR tree-optimization/112678
>
>         * tree-profile.cc (tree_profiling): Do not use atomic operations
>         for -fprofile-update=single.
> ---
>  gcc/tree-profile.cc | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
> index 1ac0fdb3bc98..9c8fdb8b18f4 100644
> --- a/gcc/tree-profile.cc
> +++ b/gcc/tree-profile.cc
> @@ -767,6 +767,7 @@ tree_profiling (void)
>      = HAVE_sync_compare_and_swapsi || HAVE_atomic_compare_and_swapsi;
>    bool have_atomic_8
>      = HAVE_sync_compare_and_swapdi || HAVE_atomic_compare_and_swapdi;
> +  bool needs_split = gcov_type_size == 8 && !have_atomic_8 && have_atomic_4;
>    if (!can_support_atomic)
>      {
>        if (gcov_type_size == 4)
> @@ -775,6 +776,9 @@ tree_profiling (void)
>         can_support_atomic = have_atomic_8;
>      }
>
> +  if (flag_profile_update != PROFILE_UPDATE_SINGLE && needs_split)
> +    counter_update = COUNTER_UPDATE_ATOMIC_PARTIAL;
> +

I wonder if it's cleaner to set can_support_atomic when we can support
it with splitting instead, avoiding a != PROFILE_UPDATE_SINGLE check
here?

Otherwise looks OK.

Richard.

>    if (flag_profile_update == PROFILE_UPDATE_ATOMIC
>        && !can_support_atomic)
>      {
> @@ -788,13 +792,11 @@ tree_profiling (void)
>
>    if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
>      {
> -      if (gcov_type_size == 8 && !have_atomic_8 && have_atomic_4)
> +      if (needs_split)
>         counter_update = COUNTER_UPDATE_ATOMIC_SPLIT;
>        else
>         counter_update = COUNTER_UPDATE_ATOMIC_BUILTIN;
>      }
> -  else if (gcov_type_size == 8 && have_atomic_4)
> -    counter_update = COUNTER_UPDATE_ATOMIC_PARTIAL;
>
>    /* This is a small-ipa pass that gets called only once, from
>       cgraphunit.cc:ipa_passes().  */
> --
> 2.35.3
>

Re: [PATCH] gcov: No atomic ops for -fprofile-update=single

[Sebastian Huber]

On 23.11.23 15:19, Richard Biener wrote:
> On Thu, Nov 23, 2023 at 2:47 PM Sebastian Huber
> <sebastian.huber@embedded-brains.de>  wrote:
>> gcc/ChangeLog:
>>          PR tree-optimization/112678
>>
>>          * tree-profile.cc (tree_profiling): Do not use atomic operations
>>          for -fprofile-update=single.
>> ---
>>   gcc/tree-profile.cc | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
>> index 1ac0fdb3bc98..9c8fdb8b18f4 100644
>> --- a/gcc/tree-profile.cc
>> +++ b/gcc/tree-profile.cc
>> @@ -767,6 +767,7 @@ tree_profiling (void)
>>       = HAVE_sync_compare_and_swapsi || HAVE_atomic_compare_and_swapsi;
>>     bool have_atomic_8
>>       = HAVE_sync_compare_and_swapdi || HAVE_atomic_compare_and_swapdi;
>> +  bool needs_split = gcov_type_size == 8 && !have_atomic_8 && have_atomic_4;
>>     if (!can_support_atomic)
>>       {
>>         if (gcov_type_size == 4)
>> @@ -775,6 +776,9 @@ tree_profiling (void)
>>          can_support_atomic = have_atomic_8;
>>       }
>>
>> +  if (flag_profile_update != PROFILE_UPDATE_SINGLE && needs_split)
>> +    counter_update = COUNTER_UPDATE_ATOMIC_PARTIAL;
>> +
> I wonder if it's cleaner to set can_support_atomic when we can support
> it with splitting instead, avoiding a != PROFILE_UPDATE_SINGLE check
> here?

The bug was that counter_update was set to COUNTER_UPDATE_ATOMIC_PARTIAL 
for -fprofile-update=single. I don't think we can get rid of the 
flag_profile_update != PROFILE_UPDATE_SINGLE without changing this whole 
code block considerably.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/