gcov: Add -fprofile-update=force-atomic

If get_gcov_type() returns a 64-bit type, then 64-bit atomic operations in
hardware are required for the "atomic" method.  Add a new method to force
atomic operations even if a library implementation (libatomic) must be used.

gcc/

	* common.opt (fprofile-update): Add force-atomic method.
	* coretypes.h (profile_update): Add PROFILE_UPDATE_FORCE_ATOMIC.
	* doc/invoke.texi (fprofile-update): Document force-atomic method.
	* tree-profile.c (tree_profiling): Support force-atomic method.
---
 gcc/common.opt      |  5 ++++-
 gcc/coretypes.h     |  3 ++-
 gcc/doc/invoke.texi | 14 +++++++++-----
 gcc/tree-profile.c  |  2 ++
 4 files changed, 17 insertions(+), 7 deletions(-)

On Mon, Aug 9, 2021 at 10:01 AM Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
>
> If get_gcov_type() returns a 64-bit type, then 64-bit atomic operations in
> hardware are required for the "atomic" method.  Add a new method to force
> atomic operations even if a library implementation (libatomic) must be used.

I do wonder about the =atomic behavior here - I'd expected that to
be equivalent to force-atomic...

With =force-atomic the user will need to eventually link to -latomic
himself, correct?  What happens for targets that do not have libatomic support?
See libatomic/configure.tgt - is that something we can test for and error/warn?

Thanks,
Richard.

> gcc/
>
>         * common.opt (fprofile-update): Add force-atomic method.
>         * coretypes.h (profile_update): Add PROFILE_UPDATE_FORCE_ATOMIC.
>         * doc/invoke.texi (fprofile-update): Document force-atomic method.
>         * tree-profile.c (tree_profiling): Support force-atomic method.
> ---
>  gcc/common.opt      |  5 ++++-
>  gcc/coretypes.h     |  3 ++-
>  gcc/doc/invoke.texi | 14 +++++++++-----
>  gcc/tree-profile.c  |  2 ++
>  4 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index d9da1131eda..ea887c987af 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2247,7 +2247,7 @@ Enable correction of flow inconsistent profile data input.
>
>  fprofile-update=
>  Common Joined RejectNegative Enum(profile_update) Var(flag_profile_update) Init(PROFILE_UPDATE_SINGLE)
> --fprofile-update=[single|atomic|prefer-atomic] Set the profile update method.
> +-fprofile-update=[single|atomic|prefer-atomic|force-atomic]    Set the profile update method.
>
>  fprofile-filter-files=
>  Common Joined RejectNegative Var(flag_profile_filter_files)
> @@ -2285,6 +2285,9 @@ Enum(profile_update) String(atomic) Value(PROFILE_UPDATE_ATOMIC)
>  EnumValue
>  Enum(profile_update) String(prefer-atomic) Value(PROFILE_UPDATE_PREFER_ATOMIC)
>
> +EnumValue
> +Enum(profile_update) String(force-atomic) Value(PROFILE_UPDATE_FORCE_ATOMIC)
> +
>  fprofile-prefix-path=
>  Common Joined RejectNegative Var(profile_prefix_path)
>  Remove prefix from absolute path before mangling name for -fprofile-generate= and -fprofile-use=.
> diff --git a/gcc/coretypes.h b/gcc/coretypes.h
> index 406572e947d..ded8e718994 100644
> --- a/gcc/coretypes.h
> +++ b/gcc/coretypes.h
> @@ -209,7 +209,8 @@ enum offload_abi {
>  enum profile_update {
>    PROFILE_UPDATE_SINGLE,
>    PROFILE_UPDATE_ATOMIC,
> -  PROFILE_UPDATE_PREFER_ATOMIC
> +  PROFILE_UPDATE_PREFER_ATOMIC,
> +  PROFILE_UPDATE_FORCE_ATOMIC
>  };
>
>  /* Type of profile reproducibility methods.  */
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index a64cec5387e..df920545ac1 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -14905,11 +14905,11 @@ part of the path and keep all file names relative to the main build directory.
>  @item -fprofile-update=@var{method}
>  @opindex fprofile-update
>
> -Alter the update method for an application instrumented for profile
> -feedback based optimization.  The @var{method} argument should be one of
> -@samp{single}, @samp{atomic} or @samp{prefer-atomic}.
> -The first one is useful for single-threaded applications,
> -while the second one prevents profile corruption by emitting thread-safe code.
> +Alter the update method for an application instrumented for profile feedback
> +based optimization or code-coverage.  The @var{method} argument should be one
> +of @samp{single}, @samp{atomic}, @samp{prefer-atomic} or @samp{force-atomic}.
> +The first one is useful for single-threaded applications, while the second one
> +prevents profile corruption by emitting thread-safe code.
>
>  @strong{Warning:} When an application does not properly join all threads
>  (or creates an detached thread), a profile file can be still corrupted.
> @@ -14919,6 +14919,10 @@ when supported by a target, or to @samp{single} otherwise.  The GCC driver
>  automatically selects @samp{prefer-atomic} when @option{-pthread}
>  is present in the command line.
>
> +Using @samp{force-atomic} forces the use of an @samp{atomic} method even if the
> +target does not support it directly in hardware and a library implementation
> +must be used instead.
> +
>  @item -fprofile-filter-files=@var{regex}
>  @opindex fprofile-filter-files
>
> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
> index 5a74cc96e13..ac2ef4ccdf2 100644
> --- a/gcc/tree-profile.c
> +++ b/gcc/tree-profile.c
> @@ -718,6 +718,8 @@ tree_profiling (void)
>    else if (flag_profile_update == PROFILE_UPDATE_PREFER_ATOMIC)
>      flag_profile_update = can_support_atomic
>        ? PROFILE_UPDATE_ATOMIC : PROFILE_UPDATE_SINGLE;
> +  else if (flag_profile_update == PROFILE_UPDATE_FORCE_ATOMIC)
> +    flag_profile_update = PROFILE_UPDATE_ATOMIC;
>
>    /* This is a small-ipa pass that gets called only once, from
>       cgraphunit.c:ipa_passes().  */
> --
> 2.26.2
>

On 09/08/2021 12:19, Richard Biener wrote:
> On Mon, Aug 9, 2021 at 10:01 AM Sebastian Huber
> <sebastian.huber@embedded-brains.de>  wrote:
>> If get_gcov_type() returns a 64-bit type, then 64-bit atomic operations in
>> hardware are required for the "atomic" method.  Add a new method to force
>> atomic operations even if a library implementation (libatomic) must be used.
> I do wonder about the =atomic behavior here - I'd expected that to
> be equivalent to force-atomic...

Yes, this method just maps to the "atomic" method:

+  else if (flag_profile_update == PROFILE_UPDATE_FORCE_ATOMIC)
+    flag_profile_update = PROFILE_UPDATE_ATOMIC;

> 
> With =force-atomic the user will need to eventually link to -latomic
> himself, correct?  What happens for targets that do not have libatomic support?
> See libatomic/configure.tgt - is that something we can test for and error/warn?

The compiler will generate a library call:

main:
         save    %sp, -96, %sp
         mov     0, %o3
         mov     1, %o2
         mov     0, %o1
         sethi   %hi(__gcov0.main), %o0
         call    __atomic_fetch_add_8, 0
          or     %o0, %lo(__gcov0.main), %o0
         sethi   %hi(x), %g1
         ld      [%g1+%lo(x)], %i0
         jmp     %i7+8
          restore

The __atomic_fetch_add_8 is usually provided by libatomic. If a target 
doesn't have it, then you get a linker error. I am not sure how the 
compiler could warn, since it is built before libatomic.

On 09/08/2021 10:00, Sebastian Huber wrote:
> If get_gcov_type() returns a 64-bit type, then 64-bit atomic operations in
> hardware are required for the "atomic" method.  Add a new method to force
> atomic operations even if a library implementation (libatomic) must be used.
> 
> gcc/
> 
> 	* common.opt (fprofile-update): Add force-atomic method.
> 	* coretypes.h (profile_update): Add PROFILE_UPDATE_FORCE_ATOMIC.
> 	* doc/invoke.texi (fprofile-update): Document force-atomic method.
> 	* tree-profile.c (tree_profiling): Support force-atomic method.

This patch was replaced by a different approach:

https://gcc.gnu.org/pipermail/gcc-patches/2021-August/576993.html