[PATCH] __atomic_test_and_set: Fall back to library, not non-atomic code

[PATCH] __atomic_test_and_set: Fall back to library, not non-atomic code

[Hans-Peter Nilsson]

Tested cris-elf, native x86_64-pc-linux-gnu and arm-eabi.

For arm-eabi, notably lacking any atomic support for the
default multilib, with --target_board=arm-sim it regressed
29_atomics/atomic_flag/cons/value_init.cc with the expected
linker failure due to lack of __atomic_test_and_set - which
is a good thing.  With this one, there are 44 unexpected
FAILs for libstdc+++ at r14-4210-g94982a6b9cf4.  This number
was 206 as late as r14-3470-g721f7e2c4e5e, but mitigated by
r14-3980-g62b29347c38394, deliberately.  To fix the
regression, I'll do the same and follow up with adding
dg-require-thread-fence on
29_atomics/atomic_flag/cons/value_init.cc (and if approved,
commit it before this one).

Incidentally, the fortran test-results for arm-eabi are
riddled with missing-__sync_synchronize linker errors
causing some 18134 unexpected failures, where cris-elf has
121.

Ok to commit?

-- >8 --
Make __atomic_test_and_set consistent with other __atomic_ and __sync_
builtins: call a matching library function instead of emitting
non-atomic code when the target has no direct insn support.

There's special-case code handling targetm.atomic_test_and_set_trueval
!= 1 trying a modified maybe_emit_sync_lock_test_and_set.  Previously,
if that worked but its matching emit_store_flag_force returned NULL,
we'd segfault later on.  Now that the caller handles NULL, gcc_assert
here instead.

While the referenced PR:s are ARM-specific, the issue is general.

	PR target/107567
	PR target/109166
	* builtins.cc (expand_builtin) <case BUILT_IN_ATOMIC_TEST_AND_SET>:
	Handle failure from expand_builtin_atomic_test_and_set.
	* optabs.cc (expand_atomic_test_and_set): When all attempts fail to
	generate atomic code through target support, return NULL
	instead of emitting non-atomic code.  Also, for code handling
	targetm.atomic_test_and_set_trueval != 1, gcc_assert result
	from calling emit_store_flag_force instead of returning NULL.
---
 gcc/builtins.cc |  5 ++++-
 gcc/optabs.cc   | 22 +++++++---------------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 6e4274bb2a4e..40dfd36a3197 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -8387,7 +8387,10 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
       break;
 
     case BUILT_IN_ATOMIC_TEST_AND_SET:
-      return expand_builtin_atomic_test_and_set (exp, target);
+      target = expand_builtin_atomic_test_and_set (exp, target);
+      if (target)
+	return target;
+      break;
 
     case BUILT_IN_ATOMIC_CLEAR:
       return expand_builtin_atomic_clear (exp);
diff --git a/gcc/optabs.cc b/gcc/optabs.cc
index 8b96f23aec05..e1898da22808 100644
--- a/gcc/optabs.cc
+++ b/gcc/optabs.cc
@@ -7080,25 +7080,17 @@ expand_atomic_test_and_set (rtx target, rtx mem, enum memmodel model)
   /* Recall that the legacy lock_test_and_set optab was allowed to do magic
      things with the value 1.  Thus we try again without trueval.  */
   if (!ret && targetm.atomic_test_and_set_trueval != 1)
-    ret = maybe_emit_sync_lock_test_and_set (subtarget, mem, const1_rtx, model);
-
-  /* Failing all else, assume a single threaded environment and simply
-     perform the operation.  */
-  if (!ret)
     {
-      /* If the result is ignored skip the move to target.  */
-      if (subtarget != const0_rtx)
-        emit_move_insn (subtarget, mem);
+      ret = maybe_emit_sync_lock_test_and_set (subtarget, mem, const1_rtx, model);
 
-      emit_move_insn (mem, trueval);
-      ret = subtarget;
+      if (ret)
+	{
+	  /* Rectify the not-one trueval.  */
+	  ret = emit_store_flag_force (target, NE, ret, const0_rtx, mode, 0, 1);
+	  gcc_assert (ret);
+	}
     }
 
-  /* Recall that have to return a boolean value; rectify if trueval
-     is not exactly one.  */
-  if (targetm.atomic_test_and_set_trueval != 1)
-    ret = emit_store_flag_force (target, NE, ret, const0_rtx, mode, 0, 1);
-  
   return ret;
 }
 
-- 
2.30.2

Re: [PATCH] __atomic_test_and_set: Fall back to library, not non-atomic code

[Jeff Law]

On 9/26/23 08:34, Hans-Peter Nilsson wrote:
> Tested cris-elf, native x86_64-pc-linux-gnu and arm-eabi.
> 
> For arm-eabi, notably lacking any atomic support for the
> default multilib, with --target_board=arm-sim it regressed
> 29_atomics/atomic_flag/cons/value_init.cc with the expected
> linker failure due to lack of __atomic_test_and_set - which
> is a good thing.  With this one, there are 44 unexpected
> FAILs for libstdc+++ at r14-4210-g94982a6b9cf4.  This number
> was 206 as late as r14-3470-g721f7e2c4e5e, but mitigated by
> r14-3980-g62b29347c38394, deliberately.  To fix the
> regression, I'll do the same and follow up with adding
> dg-require-thread-fence on
> 29_atomics/atomic_flag/cons/value_init.cc (and if approved,
> commit it before this one).
> 
> Incidentally, the fortran test-results for arm-eabi are
> riddled with missing-__sync_synchronize linker errors
> causing some 18134 unexpected failures, where cris-elf has
> 121.
> 
> Ok to commit?
> 
> -- >8 --
> Make __atomic_test_and_set consistent with other __atomic_ and __sync_
> builtins: call a matching library function instead of emitting
> non-atomic code when the target has no direct insn support.
> 
> There's special-case code handling targetm.atomic_test_and_set_trueval
> != 1 trying a modified maybe_emit_sync_lock_test_and_set.  Previously,
> if that worked but its matching emit_store_flag_force returned NULL,
> we'd segfault later on.  Now that the caller handles NULL, gcc_assert
> here instead.
> 
> While the referenced PR:s are ARM-specific, the issue is general.
> 
> 	PR target/107567
> 	PR target/109166
> 	* builtins.cc (expand_builtin) <case BUILT_IN_ATOMIC_TEST_AND_SET>:
> 	Handle failure from expand_builtin_atomic_test_and_set.
> 	* optabs.cc (expand_atomic_test_and_set): When all attempts fail to
> 	generate atomic code through target support, return NULL
> 	instead of emitting non-atomic code.  Also, for code handling
> 	targetm.atomic_test_and_set_trueval != 1, gcc_assert result
> 	from calling emit_store_flag_force instead of returning NULL.
OK
jeff

Re: [PATCH] __atomic_test_and_set: Fall back to library, not non-atomic code

[Christophe Lyon]

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

Hi!

On Tue, 26 Sept 2023 at 16:34, Hans-Peter Nilsson <hp@axis.com> wrote:

> Tested cris-elf, native x86_64-pc-linux-gnu and arm-eabi.
>
> For arm-eabi, notably lacking any atomic support for the
> default multilib, with --target_board=arm-sim it regressed
> 29_atomics/atomic_flag/cons/value_init.cc with the expected
> linker failure due to lack of __atomic_test_and_set - which
> is a good thing.  With this one, there are 44 unexpected
> FAILs for libstdc+++ at r14-4210-g94982a6b9cf4.  This number
> was 206 as late as r14-3470-g721f7e2c4e5e, but mitigated by
> r14-3980-g62b29347c38394, deliberately.  To fix the
> regression, I'll do the same and follow up with adding
> dg-require-thread-fence on
> 29_atomics/atomic_flag/cons/value_init.cc (and if approved,
> commit it before this one).
>
> Incidentally, the fortran test-results for arm-eabi are
> riddled with missing-__sync_synchronize linker errors
> causing some 18134 unexpected failures, where cris-elf has
> 121.
>
>
The patch passed almost all our CI configurations, except arm-eabi when
testing with
 -mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto
where is causes these failures:
FAIL: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 (test for excess
errors)
UNRESOLVED: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 compilation
failed to produce executable
FAIL: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++20 (test for
excess errors)
UNRESOLVED: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++20
compilation failed to produce executable
FAIL: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++26 (test for
excess errors)
UNRESOLVED: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++26
compilation failed to produce executable
FAIL: 29_atomics/atomic_flag/test_and_set/explicit.cc -std=gnu++17 (test
for excess errors)
UNRESOLVED: 29_atomics/atomic_flag/test_and_set/explicit.cc -std=gnu++17
compilation failed to produce executable
FAIL: 29_atomics/atomic_flag/test_and_set/implicit.cc -std=gnu++17 (test
for excess errors)
UNRESOLVED: 29_atomics/atomic_flag/test_and_set/implicit.cc -std=gnu++17
compilation failed to produce executable

The linker error is:
undefined reference to `__atomic_test_and_set'

Maybe we need a new variant of dg-require-thread-fence ?

Thanks,

Christophe


Ok to commit?
>
> -- >8 --
> Make __atomic_test_and_set consistent with other __atomic_ and __sync_
> builtins: call a matching library function instead of emitting
> non-atomic code when the target has no direct insn support.
>
> There's special-case code handling targetm.atomic_test_and_set_trueval
> != 1 trying a modified maybe_emit_sync_lock_test_and_set.  Previously,
> if that worked but its matching emit_store_flag_force returned NULL,
> we'd segfault later on.  Now that the caller handles NULL, gcc_assert
> here instead.
>
> While the referenced PR:s are ARM-specific, the issue is general.
>
>         PR target/107567
>         PR target/109166
>         * builtins.cc (expand_builtin) <case BUILT_IN_ATOMIC_TEST_AND_SET>:
>         Handle failure from expand_builtin_atomic_test_and_set.
>         * optabs.cc (expand_atomic_test_and_set): When all attempts fail to
>         generate atomic code through target support, return NULL
>         instead of emitting non-atomic code.  Also, for code handling
>         targetm.atomic_test_and_set_trueval != 1, gcc_assert result
>         from calling emit_store_flag_force instead of returning NULL.
> ---
>  gcc/builtins.cc |  5 ++++-
>  gcc/optabs.cc   | 22 +++++++---------------
>  2 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> index 6e4274bb2a4e..40dfd36a3197 100644
> --- a/gcc/builtins.cc
> +++ b/gcc/builtins.cc
> @@ -8387,7 +8387,10 @@ expand_builtin (tree exp, rtx target, rtx
> subtarget, machine_mode mode,
>        break;
>
>      case BUILT_IN_ATOMIC_TEST_AND_SET:
> -      return expand_builtin_atomic_test_and_set (exp, target);
> +      target = expand_builtin_atomic_test_and_set (exp, target);
> +      if (target)
> +       return target;
> +      break;
>
>      case BUILT_IN_ATOMIC_CLEAR:
>        return expand_builtin_atomic_clear (exp);
> diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> index 8b96f23aec05..e1898da22808 100644
> --- a/gcc/optabs.cc
> +++ b/gcc/optabs.cc
> @@ -7080,25 +7080,17 @@ expand_atomic_test_and_set (rtx target, rtx mem,
> enum memmodel model)
>    /* Recall that the legacy lock_test_and_set optab was allowed to do
> magic
>       things with the value 1.  Thus we try again without trueval.  */
>    if (!ret && targetm.atomic_test_and_set_trueval != 1)
> -    ret = maybe_emit_sync_lock_test_and_set (subtarget, mem, const1_rtx,
> model);
> -
> -  /* Failing all else, assume a single threaded environment and simply
> -     perform the operation.  */
> -  if (!ret)
>      {
> -      /* If the result is ignored skip the move to target.  */
> -      if (subtarget != const0_rtx)
> -        emit_move_insn (subtarget, mem);
> +      ret = maybe_emit_sync_lock_test_and_set (subtarget, mem,
> const1_rtx, model);
>
> -      emit_move_insn (mem, trueval);
> -      ret = subtarget;
> +      if (ret)
> +       {
> +         /* Rectify the not-one trueval.  */
> +         ret = emit_store_flag_force (target, NE, ret, const0_rtx, mode,
> 0, 1);
> +         gcc_assert (ret);
> +       }
>      }
>
> -  /* Recall that have to return a boolean value; rectify if trueval
> -     is not exactly one.  */
> -  if (targetm.atomic_test_and_set_trueval != 1)
> -    ret = emit_store_flag_force (target, NE, ret, const0_rtx, mode, 0, 1);
> -
>    return ret;
>  }
>
> --
> 2.30.2
>
>

Re: [PATCH] __atomic_test_and_set: Fall back to library, not non-atomic code

[Hans-Peter Nilsson]

> From: Christophe Lyon <christophe.lyon@linaro.org>
> Date: Tue, 3 Oct 2023 15:20:39 +0200

> The patch passed almost all our CI configurations, except arm-eabi when
> testing with
>  -mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto
> where is causes these failures:
> FAIL: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 (test for excess
> errors)
> UNRESOLVED: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 compilation
> failed to produce executable
> FAIL: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++20 (test for
> excess errors)
> UNRESOLVED: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++20
> compilation failed to produce executable
> FAIL: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++26 (test for
> excess errors)
> UNRESOLVED: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++26
> compilation failed to produce executable
> FAIL: 29_atomics/atomic_flag/test_and_set/explicit.cc -std=gnu++17 (test
> for excess errors)
> UNRESOLVED: 29_atomics/atomic_flag/test_and_set/explicit.cc -std=gnu++17
> compilation failed to produce executable
> FAIL: 29_atomics/atomic_flag/test_and_set/implicit.cc -std=gnu++17 (test
> for excess errors)
> UNRESOLVED: 29_atomics/atomic_flag/test_and_set/implicit.cc -std=gnu++17
> compilation failed to produce executable

For which set of multilibs in that set, do you get these
errors?  I'm guessing -march=armv6s-m, but I'm checking.

> The linker error is:
> undefined reference to `__atomic_test_and_set'

I read that as you're saying you have a multilib combination
where you currently don't emit __sync_synchronize but also
don't emit anything for __atomic_test_and_set.

> Maybe we need a new variant of dg-require-thread-fence ?

Perhaps.  Unless of course, there's a multilib combination
for which you *can* emit the proper atomic spell; missing it
because the need for it, has been hidden!

(At first I thought it was related to caching the
thread-fence property across multilib testing, but I don't
think that was correct.)

> 
> Thanks,
> 
> Christophe
> 
> 
> Ok to commit?

ENOPATCH

brgds, H-P

Re: [PATCH] __atomic_test_and_set: Fall back to library, not non-atomic code

[Christophe Lyon]

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

On Wed, 4 Oct 2023 at 02:49, Hans-Peter Nilsson <hp@axis.com> wrote:

> (Just before sending, I noticed you replied off-list; I
> won't add back gcc-patches to cc here myself, but please
> feel free to do it, if you choose to reply.)
>

Sorry, it was a typo of mine, I meant to reply to the list


>
> > From: Christophe Lyon <christophe.lyon@linaro.org>
> > Date: Tue, 3 Oct 2023 18:06:21 +0200
>
> > On Tue, 3 Oct 2023 at 17:16, Hans-Peter Nilsson <hp@axis.com> wrote:
> >
> > > > From: Christophe Lyon <christophe.lyon@linaro.org>
> > > > Date: Tue, 3 Oct 2023 15:20:39 +0200
> > >
> > > > The patch passed almost all our CI configurations, except arm-eabi
> when
> > > > testing with
> > > >  -mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto
> > > > where is causes these failures:
> > > > FAIL: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 (test for excess
> > > > errors)
> > > > UNRESOLVED: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17
> compilation
> > > > failed to produce executable
> [...]
> > > For which set of multilibs in that set, do you get these
> > > errors?  I'm guessing -march=armv6s-m, but I'm checking.
> > >
> >
> > Not sure to understand the question?
>
> By your "testing with
> -mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto"
> I presume you mean "testing with make check
>
> 'RUNTESTFLAGS=--target_board=arm-sim/-mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto'
> (as that's where you usually put that expression)
>
Yes


>
> - but I misremembered what "/" means in RUNTESTFLAGS (it's
> combining options in one single set of options passed to
> gcc, not delimiting separate options used to form
> combinations).  Sorry for the confusion!
>
I see.

Actually I always find "multilib" confusing in the context of running the
tests, where in fact we generally mean we add some flags in
RUNTESTFLAGS and/or --target_board.
To me, multilib refers to the set of lib variants we build, but I noticed
"multilib" is often used in the context of running the tests...


>
> > > > Maybe we need a new variant of dg-require-thread-fence ?
> > >
> > > Perhaps.
>
> Or rather: most certainly.  IIUC, ARMv6 (or whatever you
> prefer to call it) can load and store atomically, but only
> as separate events; it can't atomically exchange a stored
> value and therefore arm-eabi calls out to a library
> function.
>
In this case, it's armv6s-m (which is different from armv6....)
And yes, it seems so, as your patch showed, assuming there's
no bug in the target description ;-)


> I think I'll help and replace the obvious uses of
> dg-require-thread-fence where actually an atomic exchange is
> required; replacing those with a new directive
> dg-require-atomic-exchange.  That will however not be *all*
> places where such a guard should be added.
>
Indeed.


> I also see lots of undefined references to *other* outlined
> atomic builtins, for example __atomic_compare_exchange_4 and
> __atomic_fetch_add_4.  Though, those likely aren't
> regressions.  I understand you focus on regressions here.
>
Yes, my reply to your patch was meant to look at the regressions.

As a separate action, I plan to look at the remaining existing such
failures.


> By the way, how do you test; what simulator, what baseboard
> file?  Please share!  Also, please send *some*
> contrib/test_summary reports for arm-eabi to
> gcc-testresults@ every now and then.  (But also, please
>
We use qemu, with qemu.exp from:
https://git.linaro.org/toolchain/abe.git/tree/config/boards/qemu.exp
nothing fancy ;-)



> don't post multiple results several times a day for similar
> configurations.  Looking at you, powerpc people!)
>
We have plans to restart sending such results, like I was doing several
years ago
(with many results every day, too ;-))


> I can't test *anything* newer than default arm-eabi (armv4t)
> on arm-sim (the one next to gdb), or else execution tests
> get lost and time out while also complaining about "Unknown
> machine type".  I noticed there's a qemu.exp in ToT dejagnu,
> but it doesn't work for arm-eabi for at least two reasons.
> (I might get to that yak later, I just take it as a sign
> that qemu-arm isn't what I look for.)

We do use qemu-arm, depending on how you want to test,
maybe you need to add a -cpu flag such that it supports
the required instructions.


> >  Unless of course, there's a multilib combination
>
> > for which you *can* emit the proper atomic spell; missing it
> > > because the need for it, has been hidden!
> > >
> > > (At first I thought it was related to caching the
> > > thread-fence property across multilib testing, but I don't
> > > think that was correct.)
> > >
> > Not sure what you mean? We run the tests for a single multilib here
> > (mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto)
> > so the cached value should always be correct?
>
> Yeah, part of my RUNTESTFLAGS confusion per above, please
> ignore that.
>
> brgds, H-P
>

[PATCH 1/2] testsuite: Add dg-require-atomic-exchange non-atomic code

[Hans-Peter Nilsson]

> From: Christophe Lyon <christophe.lyon@linaro.org>
> Date: Tue, 3 Oct 2023 15:20:39 +0200

> Maybe we need a new variant of dg-require-thread-fence ?

Yes: many of the dg-require-thread-fence users need
something stronger.  Tested arm-eabi together with the next
patch (2/2) with
RUNTESTFLAGS=--target_board=arm-sim/-mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto\
conformance.exp=29_atomics/\*

(Incidentally, in the patch context is seen
dg-require-atomic-builtins which is a misnomer: it should
rather be named "dg-require-lock-atomic-builtins-free".)

Ok to commit?

-- >8 --
Some targets (armv6) support inline atomic load and store,
i.e. dg-require-thread-fence matches, but not atomic like
atomic exchange.  This directive will replace uses of
dg-require-thread-fence where an atomic exchange operation
is actually used.

	* testsuite/lib/dg-options.exp (dg-require-atomic-exchange): New proc.
	* testsuite/lib/libstdc++.exp (check_v3_target_atomic_exchange): Ditto.
---
 libstdc++-v3/testsuite/lib/dg-options.exp |  9 ++++++
 libstdc++-v3/testsuite/lib/libstdc++.exp  | 35 +++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp b/libstdc++-v3/testsuite/lib/dg-options.exp
index 84ad0c65330b..b13c2f244c63 100644
--- a/libstdc++-v3/testsuite/lib/dg-options.exp
+++ b/libstdc++-v3/testsuite/lib/dg-options.exp
@@ -133,6 +133,15 @@ proc dg-require-thread-fence { args } {
     return
 }
 
+proc dg-require-atomic-exchange { args } {
+    if { ![ check_v3_target_atomic_exchange ] } {
+	upvar dg-do-what dg-do-what
+	set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+	return
+    }
+    return
+}
+
 proc dg-require-atomic-builtins { args } {
     if { ![ check_v3_target_atomic_builtins ] } {
 	upvar dg-do-what dg-do-what
diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
index 608056e5068e..481f81711074 100644
--- a/libstdc++-v3/testsuite/lib/libstdc++.exp
+++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
@@ -1221,6 +1221,41 @@ proc check_v3_target_thread_fence { } {
     }]
 }
 
+proc check_v3_target_atomic_exchange { } {
+    return [check_v3_target_prop_cached et_atomic_exchange {
+	global cxxflags
+	global DEFAULT_CXXFLAGS
+
+	# Set up and link a C++11 test program that depends
+	# on atomic exchange be available for "int".
+	set src atomic_exchange[pid].cc
+
+	set f [open $src "w"]
+	puts $f "
+        int i, j, k;
+	int main() {
+	__atomic_exchange (&i, &j, &k, __ATOMIC_SEQ_CST);
+	return 0;
+	}"
+	close $f
+
+	set cxxflags_saved $cxxflags
+	set cxxflags "$cxxflags $DEFAULT_CXXFLAGS -Werror -std=gnu++11"
+
+	set lines [v3_target_compile $src /dev/null executable ""]
+	set cxxflags $cxxflags_saved
+	file delete $src
+
+	if [string match "" $lines] {
+	    # No error message, linking succeeded.
+	    return 1
+	} else {
+	    verbose "check_v3_target_atomic_exchange: compilation failed" 2
+	    return 0
+	}
+    }]
+}
+
 # Return 1 if atomics_bool and atomic_int are always lock-free, 0 otherwise.
 proc check_v3_target_atomic_builtins { } {
     return [check_v3_target_prop_cached et_atomic_builtins {
-- 
2.30.2


> 
> Thanks,
> 
> Christophe
> 
> 
> Ok to commit?
> >
> > -- >8 --
> > Make __atomic_test_and_set consistent with other __atomic_ and __sync_
> > builtins: call a matching library function instead of emitting
> > non-atomic code when the target has no direct insn support.
> >
> > There's special-case code handling targetm.atomic_test_and_set_trueval
> > != 1 trying a modified maybe_emit_sync_lock_test_and_set.  Previously,
> > if that worked but its matching emit_store_flag_force returned NULL,
> > we'd segfault later on.  Now that the caller handles NULL, gcc_assert
> > here instead.
> >
> > While the referenced PR:s are ARM-specific, the issue is general.
> >
> >         PR target/107567
> >         PR target/109166
> >         * builtins.cc (expand_builtin) <case BUILT_IN_ATOMIC_TEST_AND_SET>:
> >         Handle failure from expand_builtin_atomic_test_and_set.
> >         * optabs.cc (expand_atomic_test_and_set): When all attempts fail to
> >         generate atomic code through target support, return NULL
> >         instead of emitting non-atomic code.  Also, for code handling
> >         targetm.atomic_test_and_set_trueval != 1, gcc_assert result
> >         from calling emit_store_flag_force instead of returning NULL.
> > ---
> >  gcc/builtins.cc |  5 ++++-
> >  gcc/optabs.cc   | 22 +++++++---------------
> >  2 files changed, 11 insertions(+), 16 deletions(-)
> >
> > diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> > index 6e4274bb2a4e..40dfd36a3197 100644
> > --- a/gcc/builtins.cc
> > +++ b/gcc/builtins.cc
> > @@ -8387,7 +8387,10 @@ expand_builtin (tree exp, rtx target, rtx
> > subtarget, machine_mode mode,
> >        break;
> >
> >      case BUILT_IN_ATOMIC_TEST_AND_SET:
> > -      return expand_builtin_atomic_test_and_set (exp, target);
> > +      target = expand_builtin_atomic_test_and_set (exp, target);
> > +      if (target)
> > +       return target;
> > +      break;
> >
> >      case BUILT_IN_ATOMIC_CLEAR:
> >        return expand_builtin_atomic_clear (exp);
> > diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> > index 8b96f23aec05..e1898da22808 100644
> > --- a/gcc/optabs.cc
> > +++ b/gcc/optabs.cc
> > @@ -7080,25 +7080,17 @@ expand_atomic_test_and_set (rtx target, rtx mem,
> > enum memmodel model)
> >    /* Recall that the legacy lock_test_and_set optab was allowed to do
> > magic
> >       things with the value 1.  Thus we try again without trueval.  */
> >    if (!ret && targetm.atomic_test_and_set_trueval != 1)
> > -    ret = maybe_emit_sync_lock_test_and_set (subtarget, mem, const1_rtx,
> > model);
> > -
> > -  /* Failing all else, assume a single threaded environment and simply
> > -     perform the operation.  */
> > -  if (!ret)
> >      {
> > -      /* If the result is ignored skip the move to target.  */
> > -      if (subtarget != const0_rtx)
> > -        emit_move_insn (subtarget, mem);
> > +      ret = maybe_emit_sync_lock_test_and_set (subtarget, mem,
> > const1_rtx, model);
> >
> > -      emit_move_insn (mem, trueval);
> > -      ret = subtarget;
> > +      if (ret)
> > +       {
> > +         /* Rectify the not-one trueval.  */
> > +         ret = emit_store_flag_force (target, NE, ret, const0_rtx, mode,
> > 0, 1);
> > +         gcc_assert (ret);
> > +       }
> >      }
> >
> > -  /* Recall that have to return a boolean value; rectify if trueval
> > -     is not exactly one.  */
> > -  if (targetm.atomic_test_and_set_trueval != 1)
> > -    ret = emit_store_flag_force (target, NE, ret, const0_rtx, mode, 0, 1);
> > -
> >    return ret;
> >  }
> >
> > --
> > 2.30.2
> >
> >
> 

Re: [PATCH 1/2] testsuite: Add dg-require-atomic-exchange non-atomic code

[Jonathan Wakely]

On Wed, 4 Oct 2023 at 03:56, Hans-Peter Nilsson <hp@axis.com> wrote:
>
> > From: Christophe Lyon <christophe.lyon@linaro.org>
> > Date: Tue, 3 Oct 2023 15:20:39 +0200
>
> > Maybe we need a new variant of dg-require-thread-fence ?
>
> Yes: many of the dg-require-thread-fence users need
> something stronger.  Tested arm-eabi together with the next
> patch (2/2) with
> RUNTESTFLAGS=--target_board=arm-sim/-mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto\
> conformance.exp=29_atomics/\*
>
> (Incidentally, in the patch context is seen
> dg-require-atomic-builtins which is a misnomer: it should
> rather be named "dg-require-lock-atomic-builtins-free".)

dg-require-lock-free-atomic-builtins or
dg-require-atomic-builtins-lock-free, surely?


>
> Ok to commit?
>
> -- >8 --
> Some targets (armv6) support inline atomic load and store,
> i.e. dg-require-thread-fence matches, but not atomic like
> atomic exchange.  This directive will replace uses of
> dg-require-thread-fence where an atomic exchange operation
> is actually used.
>
>         * testsuite/lib/dg-options.exp (dg-require-atomic-exchange): New proc.
>         * testsuite/lib/libstdc++.exp (check_v3_target_atomic_exchange): Ditto.

OK

> ---
>  libstdc++-v3/testsuite/lib/dg-options.exp |  9 ++++++
>  libstdc++-v3/testsuite/lib/libstdc++.exp  | 35 +++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
>
> diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp b/libstdc++-v3/testsuite/lib/dg-options.exp
> index 84ad0c65330b..b13c2f244c63 100644
> --- a/libstdc++-v3/testsuite/lib/dg-options.exp
> +++ b/libstdc++-v3/testsuite/lib/dg-options.exp
> @@ -133,6 +133,15 @@ proc dg-require-thread-fence { args } {
>      return
>  }
>
> +proc dg-require-atomic-exchange { args } {
> +    if { ![ check_v3_target_atomic_exchange ] } {
> +       upvar dg-do-what dg-do-what
> +       set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
> +       return
> +    }
> +    return
> +}
> +
>  proc dg-require-atomic-builtins { args } {
>      if { ![ check_v3_target_atomic_builtins ] } {
>         upvar dg-do-what dg-do-what
> diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
> index 608056e5068e..481f81711074 100644
> --- a/libstdc++-v3/testsuite/lib/libstdc++.exp
> +++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
> @@ -1221,6 +1221,41 @@ proc check_v3_target_thread_fence { } {
>      }]
>  }
>
> +proc check_v3_target_atomic_exchange { } {
> +    return [check_v3_target_prop_cached et_atomic_exchange {
> +       global cxxflags
> +       global DEFAULT_CXXFLAGS
> +
> +       # Set up and link a C++11 test program that depends
> +       # on atomic exchange be available for "int".
> +       set src atomic_exchange[pid].cc
> +
> +       set f [open $src "w"]
> +       puts $f "
> +        int i, j, k;
> +       int main() {
> +       __atomic_exchange (&i, &j, &k, __ATOMIC_SEQ_CST);
> +       return 0;
> +       }"
> +       close $f
> +
> +       set cxxflags_saved $cxxflags
> +       set cxxflags "$cxxflags $DEFAULT_CXXFLAGS -Werror -std=gnu++11"
> +
> +       set lines [v3_target_compile $src /dev/null executable ""]
> +       set cxxflags $cxxflags_saved
> +       file delete $src
> +
> +       if [string match "" $lines] {
> +           # No error message, linking succeeded.
> +           return 1
> +       } else {
> +           verbose "check_v3_target_atomic_exchange: compilation failed" 2
> +           return 0
> +       }
> +    }]
> +}
> +
>  # Return 1 if atomics_bool and atomic_int are always lock-free, 0 otherwise.
>  proc check_v3_target_atomic_builtins { } {
>      return [check_v3_target_prop_cached et_atomic_builtins {
> --
> 2.30.2
>
>
> >
> > Thanks,
> >
> > Christophe
> >
> >
> > Ok to commit?
> > >
> > > -- >8 --
> > > Make __atomic_test_and_set consistent with other __atomic_ and __sync_
> > > builtins: call a matching library function instead of emitting
> > > non-atomic code when the target has no direct insn support.
> > >
> > > There's special-case code handling targetm.atomic_test_and_set_trueval
> > > != 1 trying a modified maybe_emit_sync_lock_test_and_set.  Previously,
> > > if that worked but its matching emit_store_flag_force returned NULL,
> > > we'd segfault later on.  Now that the caller handles NULL, gcc_assert
> > > here instead.
> > >
> > > While the referenced PR:s are ARM-specific, the issue is general.
> > >
> > >         PR target/107567
> > >         PR target/109166
> > >         * builtins.cc (expand_builtin) <case BUILT_IN_ATOMIC_TEST_AND_SET>:
> > >         Handle failure from expand_builtin_atomic_test_and_set.
> > >         * optabs.cc (expand_atomic_test_and_set): When all attempts fail to
> > >         generate atomic code through target support, return NULL
> > >         instead of emitting non-atomic code.  Also, for code handling
> > >         targetm.atomic_test_and_set_trueval != 1, gcc_assert result
> > >         from calling emit_store_flag_force instead of returning NULL.
> > > ---
> > >  gcc/builtins.cc |  5 ++++-
> > >  gcc/optabs.cc   | 22 +++++++---------------
> > >  2 files changed, 11 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> > > index 6e4274bb2a4e..40dfd36a3197 100644
> > > --- a/gcc/builtins.cc
> > > +++ b/gcc/builtins.cc
> > > @@ -8387,7 +8387,10 @@ expand_builtin (tree exp, rtx target, rtx
> > > subtarget, machine_mode mode,
> > >        break;
> > >
> > >      case BUILT_IN_ATOMIC_TEST_AND_SET:
> > > -      return expand_builtin_atomic_test_and_set (exp, target);
> > > +      target = expand_builtin_atomic_test_and_set (exp, target);
> > > +      if (target)
> > > +       return target;
> > > +      break;
> > >
> > >      case BUILT_IN_ATOMIC_CLEAR:
> > >        return expand_builtin_atomic_clear (exp);
> > > diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> > > index 8b96f23aec05..e1898da22808 100644
> > > --- a/gcc/optabs.cc
> > > +++ b/gcc/optabs.cc
> > > @@ -7080,25 +7080,17 @@ expand_atomic_test_and_set (rtx target, rtx mem,
> > > enum memmodel model)
> > >    /* Recall that the legacy lock_test_and_set optab was allowed to do
> > > magic
> > >       things with the value 1.  Thus we try again without trueval.  */
> > >    if (!ret && targetm.atomic_test_and_set_trueval != 1)
> > > -    ret = maybe_emit_sync_lock_test_and_set (subtarget, mem, const1_rtx,
> > > model);
> > > -
> > > -  /* Failing all else, assume a single threaded environment and simply
> > > -     perform the operation.  */
> > > -  if (!ret)
> > >      {
> > > -      /* If the result is ignored skip the move to target.  */
> > > -      if (subtarget != const0_rtx)
> > > -        emit_move_insn (subtarget, mem);
> > > +      ret = maybe_emit_sync_lock_test_and_set (subtarget, mem,
> > > const1_rtx, model);
> > >
> > > -      emit_move_insn (mem, trueval);
> > > -      ret = subtarget;
> > > +      if (ret)
> > > +       {
> > > +         /* Rectify the not-one trueval.  */
> > > +         ret = emit_store_flag_force (target, NE, ret, const0_rtx, mode,
> > > 0, 1);
> > > +         gcc_assert (ret);
> > > +       }
> > >      }
> > >
> > > -  /* Recall that have to return a boolean value; rectify if trueval
> > > -     is not exactly one.  */
> > > -  if (targetm.atomic_test_and_set_trueval != 1)
> > > -    ret = emit_store_flag_force (target, NE, ret, const0_rtx, mode, 0, 1);
> > > -
> > >    return ret;
> > >  }
> > >
> > > --
> > > 2.30.2
> > >
> > >
> >
>

[PATCH 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-exchange

[Hans-Peter Nilsson]

> From: Christophe Lyon <christophe.lyon@linaro.org>
> Date: Tue, 3 Oct 2023 15:20:39 +0200

> The patch passed almost all our CI configurations, except arm-eabi when
> testing with
>  -mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto
> where is causes these failures:
> FAIL: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 (test for excess
> errors)
> UNRESOLVED: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 compilation
> failed to produce executable
> FAIL: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++20 (test for
> excess errors)
> UNRESOLVED: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++20
> compilation failed to produce executable
> FAIL: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++26 (test for
> excess errors)
> UNRESOLVED: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++26
> compilation failed to produce executable
> FAIL: 29_atomics/atomic_flag/test_and_set/explicit.cc -std=gnu++17 (test
> for excess errors)
> UNRESOLVED: 29_atomics/atomic_flag/test_and_set/explicit.cc -std=gnu++17
> compilation failed to produce executable
> FAIL: 29_atomics/atomic_flag/test_and_set/implicit.cc -std=gnu++17 (test
> for excess errors)
> UNRESOLVED: 29_atomics/atomic_flag/test_and_set/implicit.cc -std=gnu++17
> compilation failed to produce executable
> 
> The linker error is:
> undefined reference to `__atomic_test_and_set'

Here's 2/2, fixing those regressions by, after code
inspection, gating those test-case users of
dg-require-thread-fence that actually use not just
atomic-load/store functionality, but a form of
compare-exchange, including the test-and-set cases listed
above as testsuite regressions.  That neatly includes the
regressions above.

Again, other libstdc++ test-cases should likely also use
this gate, from what I see of "undefined references" in
libstdc++.log.

Tested together with 1/2.
Ok to commit?

(N.B. there was a stray suffix "non-atomic code" in the
subject of 1/2; that's just a typo which will not be
committed.)

-- >8 --
These tests actually use a form of atomic exchange, not just
atomic loading and storing.  Some target have the latter,
but not the former, yielding linker errors for missing
library functions (and not supported by libatomic).

This change is just for existing uses of
dg-require-thread-fence.  It does not fix any other tests
that should also be gated on dg-require-atomic-exchange.

	* testsuite/29_atomics/atomic/compare_exchange_padding.cc,
	testsuite/29_atomics/atomic_flag/clear/1.cc,
	testsuite/29_atomics/atomic_flag/cons/value_init.cc,
	testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc,
	testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc,
	testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc,
	testsuite/29_atomics/atomic_ref/generic.cc,
	testsuite/29_atomics/atomic_ref/integral.cc,
	testsuite/29_atomics/atomic_ref/pointer.cc: Replace
	dg-require-thread-fence with dg-require-atomic-exchange.
---
 .../testsuite/29_atomics/atomic/compare_exchange_padding.cc     | 2 +-
 libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc        | 2 +-
 .../testsuite/29_atomics/atomic_flag/cons/value_init.cc         | 2 +-
 .../testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc   | 2 +-
 .../testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc   | 2 +-
 .../testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc | 2 +-
 libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc         | 2 +-
 libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc        | 2 +-
 libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc         | 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
index 01f7475631e6..14698bb82456 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-exchange "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
index 89ed381fe057..0d8a11899ef1 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++11 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-exchange "" }
 
 // Copyright (C) 2009-2023 Free Software Foundation, Inc.
 //
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
index f3f38b54dbcd..f95818532107 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
@@ -16,7 +16,7 @@
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-exchange "" }
 
 #include <atomic>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
index 6f723eb5f4e7..65ccee32142d 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++11 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-exchange "" }
 
 // Copyright (C) 2008-2023 Free Software Foundation, Inc.
 //
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
index 6f723eb5f4e7..65ccee32142d 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++11 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-exchange "" }
 
 // Copyright (C) 2008-2023 Free Software Foundation, Inc.
 //
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
index 2a3d1d468c22..1573a11d07e8 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-exchange "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
index f8751756d02c..1e0aef7ba5f6 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
@@ -16,7 +16,7 @@
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-exchange "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
index eb22afca03a2..3d81f0886399 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
@@ -16,7 +16,7 @@
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-exchange "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
index 6fe00b557567..9c4afd61c365 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
@@ -16,7 +16,7 @@
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-exchange "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
-- 
2.30.2

Re: [PATCH 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-exchange

[Jonathan Wakely]

On Wed, 4 Oct 2023 at 04:11, Hans-Peter Nilsson <hp@axis.com> wrote:
>
> > From: Christophe Lyon <christophe.lyon@linaro.org>
> > Date: Tue, 3 Oct 2023 15:20:39 +0200
>
> > The patch passed almost all our CI configurations, except arm-eabi when
> > testing with
> >  -mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto
> > where is causes these failures:
> > FAIL: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 (test for excess
> > errors)
> > UNRESOLVED: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 compilation
> > failed to produce executable
> > FAIL: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++20 (test for
> > excess errors)
> > UNRESOLVED: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++20
> > compilation failed to produce executable
> > FAIL: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++26 (test for
> > excess errors)
> > UNRESOLVED: 29_atomics/atomic_flag/cons/value_init.cc -std=gnu++26
> > compilation failed to produce executable
> > FAIL: 29_atomics/atomic_flag/test_and_set/explicit.cc -std=gnu++17 (test
> > for excess errors)
> > UNRESOLVED: 29_atomics/atomic_flag/test_and_set/explicit.cc -std=gnu++17
> > compilation failed to produce executable
> > FAIL: 29_atomics/atomic_flag/test_and_set/implicit.cc -std=gnu++17 (test
> > for excess errors)
> > UNRESOLVED: 29_atomics/atomic_flag/test_and_set/implicit.cc -std=gnu++17
> > compilation failed to produce executable
> >
> > The linker error is:
> > undefined reference to `__atomic_test_and_set'
>
> Here's 2/2, fixing those regressions by, after code
> inspection, gating those test-case users of
> dg-require-thread-fence that actually use not just
> atomic-load/store functionality, but a form of
> compare-exchange, including the test-and-set cases listed
> above as testsuite regressions.  That neatly includes the
> regressions above.

The new dg-require proc checks for __atomic_exchange, which is not the
same as compare-exchange, and not the same as test-and-set on
atomic_flag. Does it just happen to be true for arm that the presence
of __atomic_exchange also implies the other atomic operations?

The new proc also only tests it for int, which presumably means none
of these tests rely on atomics for long long being present. Some of
the tests use atomics on pointers, which should work for ILP32 targets
if atomics work on int, but the new dg-require-atomic-exchange isn't
sufficient to check that it will work for pointers on LP64 targets.

Maybe it happens to work today for the targets where we have issues,
but we seem to be assuming that there will be no LP64 targets where
these atomics are absent for 64-bit pointers. Are there supported
risc-v ISA subsets where that isn't true? And we're assuming that
__atomic_exchange being present implies all other ops are present,
which seems like a bad assumption to me. I would be more confident
testing for __atomic_compare_exchange, because with a wait-free CAS
you can implement any other atomic operation, but the same isn't true
for __atomic_exchange.

>
> Again, other libstdc++ test-cases should likely also use
> this gate, from what I see of "undefined references" in
> libstdc++.log.
>
> Tested together with 1/2.
> Ok to commit?
>
> (N.B. there was a stray suffix "non-atomic code" in the
> subject of 1/2; that's just a typo which will not be
> committed.)
>
> -- >8 --
> These tests actually use a form of atomic exchange, not just
> atomic loading and storing.  Some target have the latter,
> but not the former, yielding linker errors for missing
> library functions (and not supported by libatomic).
>
> This change is just for existing uses of
> dg-require-thread-fence.  It does not fix any other tests
> that should also be gated on dg-require-atomic-exchange.
>
>         * testsuite/29_atomics/atomic/compare_exchange_padding.cc,
>         testsuite/29_atomics/atomic_flag/clear/1.cc,
>         testsuite/29_atomics/atomic_flag/cons/value_init.cc,
>         testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc,
>         testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc,
>         testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc,
>         testsuite/29_atomics/atomic_ref/generic.cc,
>         testsuite/29_atomics/atomic_ref/integral.cc,
>         testsuite/29_atomics/atomic_ref/pointer.cc: Replace
>         dg-require-thread-fence with dg-require-atomic-exchange.
> ---
>  .../testsuite/29_atomics/atomic/compare_exchange_padding.cc     | 2 +-
>  libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc        | 2 +-
>  .../testsuite/29_atomics/atomic_flag/cons/value_init.cc         | 2 +-
>  .../testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc   | 2 +-
>  .../testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc   | 2 +-
>  .../testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc | 2 +-
>  libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc         | 2 +-
>  libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc        | 2 +-
>  libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc         | 2 +-
>  9 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
> index 01f7475631e6..14698bb82456 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-exchange "" }
>  // { dg-add-options libatomic }
>
>  #include <atomic>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
> index 89ed381fe057..0d8a11899ef1 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++11 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-exchange "" }
>
>  // Copyright (C) 2009-2023 Free Software Foundation, Inc.
>  //
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
> index f3f38b54dbcd..f95818532107 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
> @@ -16,7 +16,7 @@
>  // <http://www.gnu.org/licenses/>.
>
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-exchange "" }
>
>  #include <atomic>
>  #include <testsuite_hooks.h>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
> index 6f723eb5f4e7..65ccee32142d 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++11 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-exchange "" }
>
>  // Copyright (C) 2008-2023 Free Software Foundation, Inc.
>  //
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
> index 6f723eb5f4e7..65ccee32142d 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++11 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-exchange "" }
>
>  // Copyright (C) 2008-2023 Free Software Foundation, Inc.
>  //
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
> index 2a3d1d468c22..1573a11d07e8 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-exchange "" }
>  // { dg-add-options libatomic }
>
>  #include <atomic>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
> index f8751756d02c..1e0aef7ba5f6 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
> @@ -16,7 +16,7 @@
>  // <http://www.gnu.org/licenses/>.
>
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-exchange "" }
>  // { dg-add-options libatomic }
>
>  #include <atomic>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
> index eb22afca03a2..3d81f0886399 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
> @@ -16,7 +16,7 @@
>  // <http://www.gnu.org/licenses/>.
>
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-exchange "" }
>  // { dg-add-options libatomic }
>
>  #include <atomic>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
> index 6fe00b557567..9c4afd61c365 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
> @@ -16,7 +16,7 @@
>  // <http://www.gnu.org/licenses/>.
>
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-exchange "" }
>  // { dg-add-options libatomic }
>
>  #include <atomic>
> --
> 2.30.2
>

Re: [PATCH 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-exchange

[Hans-Peter Nilsson]

> From: Jonathan Wakely <jwakely@redhat.com>
> Date: Wed, 4 Oct 2023 09:29:43 +0100

> The new dg-require proc checks for __atomic_exchange, which is not the
> same as compare-exchange, and not the same as test-and-set on
> atomic_flag. Does it just happen to be true for arm that the presence
> of __atomic_exchange also implies the other atomic operations?

I missed pointing out that if the target implements
something that emits actual insns for __atomic_exchange
(and/or __atomic_compare_exchange), it has also implemented
test-and-set.  Cf. optabs.cc:expand_atomic_test_and_set (at
r14-4395-g027a94cf32be0b).

Similarly a insn-level __atomic_compare_exchange
implementation (atomic_compare_and_swapM) also does it for
__atomic_exchange.

> The new proc also only tests it for int, which presumably means none
> of these tests rely on atomics for long long being present. Some of
> the tests use atomics on pointers, which should work for ILP32 targets
> if atomics work on int, but the new dg-require-atomic-exchange isn't
> sufficient to check that it will work for pointers on LP64 targets.

Right, I'll amend to test a uintptr_t...

> Maybe it happens to work today for the targets where we have issues,
> but we seem to be assuming that there will be no LP64 targets where
> these atomics are absent for 64-bit pointers. Are there supported
> risc-v ISA subsets where that isn't true?

...but generally, I'd like to leave future amendments like
that to the Next Guy, just like the Previous Guy left
dg-require-thread-fence for me (us) to split up.  But,
perhaps we can prepare for such amendments by giving it a
more specific name: suggesting
dg-require-atomic-cmpxchg-word (bikeshedding opportunity),
testing __atomic_compare_exchange.

IOW, I don't think we should make a distinction, looking for
other operations and sizes at this time.

> And we're assuming that
> __atomic_exchange being present implies all other ops are present,
> which seems like a bad assumption to me. I would be more confident
> testing for __atomic_compare_exchange, because with a wait-free CAS
> you can implement any other atomic operation, but the same isn't true
> for __atomic_exchange.

Yes, I switched them around.

New version coming up.

brgds, H-P

Re: [PATCH 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-exchange

[Jonathan Wakely]

On Wed, 4 Oct 2023 at 16:15, Hans-Peter Nilsson <hp@axis.com> wrote:
>
> > From: Jonathan Wakely <jwakely@redhat.com>
> > Date: Wed, 4 Oct 2023 09:29:43 +0100
>
> > The new dg-require proc checks for __atomic_exchange, which is not the
> > same as compare-exchange, and not the same as test-and-set on
> > atomic_flag. Does it just happen to be true for arm that the presence
> > of __atomic_exchange also implies the other atomic operations?
>
> I missed pointing out that if the target implements
> something that emits actual insns for __atomic_exchange
> (and/or __atomic_compare_exchange), it has also implemented
> test-and-set.  Cf. optabs.cc:expand_atomic_test_and_set (at
> r14-4395-g027a94cf32be0b).
>
> Similarly a insn-level __atomic_compare_exchange
> implementation (atomic_compare_and_swapM) also does it for
> __atomic_exchange.

Ah great, that makes testing __atomic_exchange good enough then.

>
> > The new proc also only tests it for int, which presumably means none
> > of these tests rely on atomics for long long being present. Some of
> > the tests use atomics on pointers, which should work for ILP32 targets
> > if atomics work on int, but the new dg-require-atomic-exchange isn't
> > sufficient to check that it will work for pointers on LP64 targets.
>
> Right, I'll amend to test a uintptr_t...
>
> > Maybe it happens to work today for the targets where we have issues,
> > but we seem to be assuming that there will be no LP64 targets where
> > these atomics are absent for 64-bit pointers. Are there supported
> > risc-v ISA subsets where that isn't true?
>
> ...but generally, I'd like to leave future amendments like
> that to the Next Guy, just like the Previous Guy left
> dg-require-thread-fence for me (us) to split up.  But,
> perhaps we can prepare for such amendments by giving it a
> more specific name: suggesting
> dg-require-atomic-cmpxchg-word (bikeshedding opportunity),
> testing __atomic_compare_exchange.
>
> IOW, I don't think we should make a distinction, looking for
> other operations and sizes at this time.

Yep, that's fine. Worse is better. Maybe just add a comment before the
definition of the new effective target check noting that we only test
for one atomic op, but the implementation in gcc means that's good
enough.

>
> > And we're assuming that
> > __atomic_exchange being present implies all other ops are present,
> > which seems like a bad assumption to me. I would be more confident
> > testing for __atomic_compare_exchange, because with a wait-free CAS
> > you can implement any other atomic operation, but the same isn't true
> > for __atomic_exchange.
>
> Yes, I switched them around.
>
> New version coming up.

Thanks!


>
> brgds, H-P
>

[PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Wed, 4 Oct 2023 17:15:28 +0200

> New version coming up.

Using pointer-sized int instead of int,
__atomic_compare_exchange instead of __atomic_exchange,
renamed to atomic-cmpxchg-word from atomic-exchange, and
updating a comment that already seemed reasonably well
placed.

Tested as with v1 1/2.

Ok to commit?

-- >8 --
Some targets (armv6-m) support inline atomic load and store,
i.e. dg-require-thread-fence matches, but not atomic operations like
compare and exchange.

This directive can be used to replace uses of dg-require-thread-fence
where an atomic operation is actually used.

	* testsuite/lib/dg-options.exp (dg-require-atomic-cmpxchg-word):
	New proc.
	* testsuite/lib/libstdc++.exp (check_v3_target_atomic_cmpxchg_word):
	Ditto.
---
 libstdc++-v3/testsuite/lib/dg-options.exp |  9 ++++++
 libstdc++-v3/testsuite/lib/libstdc++.exp  | 37 +++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp b/libstdc++-v3/testsuite/lib/dg-options.exp
index 84ad0c65330b..850442b6b7c1 100644
--- a/libstdc++-v3/testsuite/lib/dg-options.exp
+++ b/libstdc++-v3/testsuite/lib/dg-options.exp
@@ -133,6 +133,15 @@ proc dg-require-thread-fence { args } {
     return
 }
 
+proc dg-require-atomic-cmpxchg-word { args } {
+    if { ![ check_v3_target_atomic_cmpxchg_word ] } {
+	upvar dg-do-what dg-do-what
+	set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+	return
+    }
+    return
+}
+
 proc dg-require-atomic-builtins { args } {
     if { ![ check_v3_target_atomic_builtins ] } {
 	upvar dg-do-what dg-do-what
diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
index 608056e5068e..4bedb36dc6f9 100644
--- a/libstdc++-v3/testsuite/lib/libstdc++.exp
+++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
@@ -1221,6 +1221,43 @@ proc check_v3_target_thread_fence { } {
     }]
 }
 
+proc check_v3_target_atomic_cmpxchg_word { } {
+    return [check_v3_target_prop_cached et_atomic_cmpxchg_word {
+	global cxxflags
+	global DEFAULT_CXXFLAGS
+
+	# Set up and link a C++11 test program that depends on
+	# atomic-compare-exchange being available for a pointer-sized
+	# integer.  It should be sufficient as gcc can derive all
+	# other operations when a target implements this operation.
+	set src atomic_cmpxchg_word[pid].cc
+
+	set f [open $src "w"]
+	puts $f "
+	__UINTPTR_TYPE__ i, j, k;
+	int main() {
+	__atomic_compare_exchange (&i, &j, &k, 1, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
+	return 0;
+	}"
+	close $f
+
+	set cxxflags_saved $cxxflags
+	set cxxflags "$cxxflags $DEFAULT_CXXFLAGS -Werror -std=gnu++11"
+
+	set lines [v3_target_compile $src /dev/null executable ""]
+	set cxxflags $cxxflags_saved
+	file delete $src
+
+	if [string match "" $lines] {
+	    # No error message, linking succeeded.
+	    return 1
+	} else {
+	    verbose "check_v3_target_atomic_cmpxchg_word: compilation failed" 2
+	    return 0
+	}
+    }]
+}
+
 # Return 1 if atomics_bool and atomic_int are always lock-free, 0 otherwise.
 proc check_v3_target_atomic_builtins { } {
     return [check_v3_target_prop_cached et_atomic_builtins {
-- 
2.30.2

Ping: [PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word

[Hans-Peter Nilsson]

Ping.

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Wed, 4 Oct 2023 19:04:55 +0200
> 
> > From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Wed, 4 Oct 2023 17:15:28 +0200
> 
> > New version coming up.
> 
> Using pointer-sized int instead of int,
> __atomic_compare_exchange instead of __atomic_exchange,
> renamed to atomic-cmpxchg-word from atomic-exchange, and
> updating a comment that already seemed reasonably well
> placed.
> 
> Tested as with v1 1/2.
> 
> Ok to commit?
> 
> -- >8 --
> Some targets (armv6-m) support inline atomic load and store,
> i.e. dg-require-thread-fence matches, but not atomic operations like
> compare and exchange.
> 
> This directive can be used to replace uses of dg-require-thread-fence
> where an atomic operation is actually used.
> 
> 	* testsuite/lib/dg-options.exp (dg-require-atomic-cmpxchg-word):
> 	New proc.
> 	* testsuite/lib/libstdc++.exp (check_v3_target_atomic_cmpxchg_word):
> 	Ditto.
> ---
>  libstdc++-v3/testsuite/lib/dg-options.exp |  9 ++++++
>  libstdc++-v3/testsuite/lib/libstdc++.exp  | 37 +++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp b/libstdc++-v3/testsuite/lib/dg-options.exp
> index 84ad0c65330b..850442b6b7c1 100644
> --- a/libstdc++-v3/testsuite/lib/dg-options.exp
> +++ b/libstdc++-v3/testsuite/lib/dg-options.exp
> @@ -133,6 +133,15 @@ proc dg-require-thread-fence { args } {
>      return
>  }
>  
> +proc dg-require-atomic-cmpxchg-word { args } {
> +    if { ![ check_v3_target_atomic_cmpxchg_word ] } {
> +	upvar dg-do-what dg-do-what
> +	set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
> +	return
> +    }
> +    return
> +}
> +
>  proc dg-require-atomic-builtins { args } {
>      if { ![ check_v3_target_atomic_builtins ] } {
>  	upvar dg-do-what dg-do-what
> diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
> index 608056e5068e..4bedb36dc6f9 100644
> --- a/libstdc++-v3/testsuite/lib/libstdc++.exp
> +++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
> @@ -1221,6 +1221,43 @@ proc check_v3_target_thread_fence { } {
>      }]
>  }
>  
> +proc check_v3_target_atomic_cmpxchg_word { } {
> +    return [check_v3_target_prop_cached et_atomic_cmpxchg_word {
> +	global cxxflags
> +	global DEFAULT_CXXFLAGS
> +
> +	# Set up and link a C++11 test program that depends on
> +	# atomic-compare-exchange being available for a pointer-sized
> +	# integer.  It should be sufficient as gcc can derive all
> +	# other operations when a target implements this operation.
> +	set src atomic_cmpxchg_word[pid].cc
> +
> +	set f [open $src "w"]
> +	puts $f "
> +	__UINTPTR_TYPE__ i, j, k;
> +	int main() {
> +	__atomic_compare_exchange (&i, &j, &k, 1, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
> +	return 0;
> +	}"
> +	close $f
> +
> +	set cxxflags_saved $cxxflags
> +	set cxxflags "$cxxflags $DEFAULT_CXXFLAGS -Werror -std=gnu++11"
> +
> +	set lines [v3_target_compile $src /dev/null executable ""]
> +	set cxxflags $cxxflags_saved
> +	file delete $src
> +
> +	if [string match "" $lines] {
> +	    # No error message, linking succeeded.
> +	    return 1
> +	} else {
> +	    verbose "check_v3_target_atomic_cmpxchg_word: compilation failed" 2
> +	    return 0
> +	}
> +    }]
> +}
> +
>  # Return 1 if atomics_bool and atomic_int are always lock-free, 0 otherwise.
>  proc check_v3_target_atomic_builtins { } {
>      return [check_v3_target_prop_cached et_atomic_builtins {
> -- 
> 2.30.2
> 

Re: Ping: [PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word

[Christophe Lyon]

LGTM but I'm not a maintainer ;-)

On Thu, 12 Oct 2023 at 04:21, Hans-Peter Nilsson <hp@axis.com> wrote:
>
> Ping.
>
> > From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Wed, 4 Oct 2023 19:04:55 +0200
> >
> > > From: Hans-Peter Nilsson <hp@axis.com>
> > > Date: Wed, 4 Oct 2023 17:15:28 +0200
> >
> > > New version coming up.
> >
> > Using pointer-sized int instead of int,
> > __atomic_compare_exchange instead of __atomic_exchange,
> > renamed to atomic-cmpxchg-word from atomic-exchange, and
> > updating a comment that already seemed reasonably well
> > placed.
> >
> > Tested as with v1 1/2.
> >
> > Ok to commit?
> >
> > -- >8 --
> > Some targets (armv6-m) support inline atomic load and store,
> > i.e. dg-require-thread-fence matches, but not atomic operations like
> > compare and exchange.
> >
> > This directive can be used to replace uses of dg-require-thread-fence
> > where an atomic operation is actually used.
> >
> >       * testsuite/lib/dg-options.exp (dg-require-atomic-cmpxchg-word):
> >       New proc.
> >       * testsuite/lib/libstdc++.exp (check_v3_target_atomic_cmpxchg_word):
> >       Ditto.
> > ---
> >  libstdc++-v3/testsuite/lib/dg-options.exp |  9 ++++++
> >  libstdc++-v3/testsuite/lib/libstdc++.exp  | 37 +++++++++++++++++++++++
> >  2 files changed, 46 insertions(+)
> >
> > diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp b/libstdc++-v3/testsuite/lib/dg-options.exp
> > index 84ad0c65330b..850442b6b7c1 100644
> > --- a/libstdc++-v3/testsuite/lib/dg-options.exp
> > +++ b/libstdc++-v3/testsuite/lib/dg-options.exp
> > @@ -133,6 +133,15 @@ proc dg-require-thread-fence { args } {
> >      return
> >  }
> >
> > +proc dg-require-atomic-cmpxchg-word { args } {
> > +    if { ![ check_v3_target_atomic_cmpxchg_word ] } {
> > +     upvar dg-do-what dg-do-what
> > +     set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
> > +     return
> > +    }
> > +    return
> > +}
> > +
> >  proc dg-require-atomic-builtins { args } {
> >      if { ![ check_v3_target_atomic_builtins ] } {
> >       upvar dg-do-what dg-do-what
> > diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
> > index 608056e5068e..4bedb36dc6f9 100644
> > --- a/libstdc++-v3/testsuite/lib/libstdc++.exp
> > +++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
> > @@ -1221,6 +1221,43 @@ proc check_v3_target_thread_fence { } {
> >      }]
> >  }
> >
> > +proc check_v3_target_atomic_cmpxchg_word { } {
> > +    return [check_v3_target_prop_cached et_atomic_cmpxchg_word {
> > +     global cxxflags
> > +     global DEFAULT_CXXFLAGS
> > +
> > +     # Set up and link a C++11 test program that depends on
> > +     # atomic-compare-exchange being available for a pointer-sized
> > +     # integer.  It should be sufficient as gcc can derive all
> > +     # other operations when a target implements this operation.
> > +     set src atomic_cmpxchg_word[pid].cc
> > +
> > +     set f [open $src "w"]
> > +     puts $f "
> > +     __UINTPTR_TYPE__ i, j, k;
> > +     int main() {
> > +     __atomic_compare_exchange (&i, &j, &k, 1, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
> > +     return 0;
> > +     }"
> > +     close $f
> > +
> > +     set cxxflags_saved $cxxflags
> > +     set cxxflags "$cxxflags $DEFAULT_CXXFLAGS -Werror -std=gnu++11"
> > +
> > +     set lines [v3_target_compile $src /dev/null executable ""]
> > +     set cxxflags $cxxflags_saved
> > +     file delete $src
> > +
> > +     if [string match "" $lines] {
> > +         # No error message, linking succeeded.
> > +         return 1
> > +     } else {
> > +         verbose "check_v3_target_atomic_cmpxchg_word: compilation failed" 2
> > +         return 0
> > +     }
> > +    }]
> > +}
> > +
> >  # Return 1 if atomics_bool and atomic_int are always lock-free, 0 otherwise.
> >  proc check_v3_target_atomic_builtins { } {
> >      return [check_v3_target_prop_cached et_atomic_builtins {
> > --
> > 2.30.2
> >

Re: Ping: [PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word

[Jeff Law]

On 10/12/23 08:38, Christophe Lyon wrote:
> LGTM but I'm not a maintainer ;-)
LGTM to as well -- I usually try to stay out of libstdc++, but this 
looks simple enough.  Both patches in this series are OK.

jeff

Re: Ping: [PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word

[Jonathan Wakely]

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

On Thu, 12 Oct 2023, 17:11 Jeff Law, <jeffreyalaw@gmail.com> wrote:

>
>
> On 10/12/23 08:38, Christophe Lyon wrote:
> > LGTM but I'm not a maintainer ;-)
> LGTM to as well -- I usually try to stay out of libstdc++, but this
> looks simple enough.  Both patches in this series are OK.
>

Thanks for stepping in, Jeff. The patches are indeed fine, I'm just offline
due to circumstances beyond my control. I hope normal service will resume
soon.

Re: Ping: [PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word

[Torbjorn SVENSSON]

Hi,

Is it okay to backport 62b29347c38394ae32858f2301aa9aa65205984e, 
2a4d9e4f533c77870cc0eb60fbbd8047da4c7386 and 
ba0cde8ba2d93b7193050eb5ef3cc6f7a2cdfe61 to releases/gcc-13?

Without this backport, I see these failures on arm-none-eabi:

FAIL: 29_atomics/atomic/compare_exchange_padding.cc (test for excess errors)
FAIL: 29_atomics/atomic_ref/compare_exchange_padding.cc (test for excess 
errors)

Kind regards,
Torbjörn

On 2023-10-13 00:23, Jonathan Wakely wrote:
> 
> 
> On Thu, 12 Oct 2023, 17:11 Jeff Law, <jeffreyalaw@gmail.com 
> <mailto:jeffreyalaw@gmail.com>> wrote:
> 
> 
> 
>     On 10/12/23 08:38, Christophe Lyon wrote:
>      > LGTM but I'm not a maintainer ;-)
>     LGTM to as well -- I usually try to stay out of libstdc++, but this
>     looks simple enough.  Both patches in this series are OK.
> 
> 
> Thanks for stepping in, Jeff. The patches are indeed fine, I'm just 
> offline due to circumstances beyond my control. I hope normal service 
> will resume soon.
> 
> 

Re: Ping: [PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word

[Jonathan Wakely]

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

On Wed, 7 Feb 2024 at 16:31, Torbjorn SVENSSON <
torbjorn.svensson@foss.st.com> wrote:

> Hi,
>
> Is it okay to backport 62b29347c38394ae32858f2301aa9aa65205984e,
> 2a4d9e4f533c77870cc0eb60fbbd8047da4c7386 and
> ba0cde8ba2d93b7193050eb5ef3cc6f7a2cdfe61 to releases/gcc-13?
>
> Without this backport, I see these failures on arm-none-eabi:
>
> FAIL: 29_atomics/atomic/compare_exchange_padding.cc (test for excess
> errors)
> FAIL: 29_atomics/atomic_ref/compare_exchange_padding.cc (test for excess
> errors)
>

Yes, OK


>
> Kind regards,
> Torbjörn
>
> On 2023-10-13 00:23, Jonathan Wakely wrote:
> >
> >
> > On Thu, 12 Oct 2023, 17:11 Jeff Law, <jeffreyalaw@gmail.com
> > <mailto:jeffreyalaw@gmail.com>> wrote:
> >
> >
> >
> >     On 10/12/23 08:38, Christophe Lyon wrote:
> >      > LGTM but I'm not a maintainer ;-)
> >     LGTM to as well -- I usually try to stay out of libstdc++, but this
> >     looks simple enough.  Both patches in this series are OK.
> >
> >
> > Thanks for stepping in, Jeff. The patches are indeed fine, I'm just
> > offline due to circumstances beyond my control. I hope normal service
> > will resume soon.
> >
> >
>
>

Re: Ping: [PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word

[Torbjorn SVENSSON]

On 2024-02-07 17:33, Jonathan Wakely wrote:
> 
> 
> On Wed, 7 Feb 2024 at 16:31, Torbjorn SVENSSON 
> <torbjorn.svensson@foss.st.com <mailto:torbjorn.svensson@foss.st.com>> 
> wrote:
> 
>     Hi,
> 
>     Is it okay to backport 62b29347c38394ae32858f2301aa9aa65205984e,
>     2a4d9e4f533c77870cc0eb60fbbd8047da4c7386 and
>     ba0cde8ba2d93b7193050eb5ef3cc6f7a2cdfe61 to releases/gcc-13?
> 
>     Without this backport, I see these failures on arm-none-eabi:
> 
>     FAIL: 29_atomics/atomic/compare_exchange_padding.cc (test for excess
>     errors)
>     FAIL: 29_atomics/atomic_ref/compare_exchange_padding.cc (test for
>     excess
>     errors)
> 
> 
> Yes, OK

Pushed as b937b1189069b24f8d1a5c25d8f062029784fb0c, 
fa0e0c28ee8f6ca2f8d5c50737647bef734dc898 and 
18fd8d29b27ec9ace70098b4a451f5296276812e.

[PATCH v2 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-cmpxchg-word

[Hans-Peter Nilsson]

s/atomic-exchange/atomic-cmpxchg-word/g.
Tested as v1.

Ok to commit?
-- >8 --
These tests actually use a form of atomic compare and exchange
operation, not just atomic loading and storing.  Some targets (not
supported by e.g. libatomic) have atomic loading and storing, but not
compare and exchange, yielding linker errors for missing library
functions.

This change is just for existing uses of
dg-require-thread-fence.  It does not fix any other tests
that should also be gated on dg-require-atomic-cmpxchg-word.

	* testsuite/29_atomics/atomic/compare_exchange_padding.cc,
	testsuite/29_atomics/atomic_flag/clear/1.cc,
	testsuite/29_atomics/atomic_flag/cons/value_init.cc,
	testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc,
	testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc,
	testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc,
	testsuite/29_atomics/atomic_ref/generic.cc,
	testsuite/29_atomics/atomic_ref/integral.cc,
	testsuite/29_atomics/atomic_ref/pointer.cc: Replace
	dg-require-thread-fence with dg-require-atomic-cmpxchg-word.
---
 .../testsuite/29_atomics/atomic/compare_exchange_padding.cc     | 2 +-
 libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc        | 2 +-
 .../testsuite/29_atomics/atomic_flag/cons/value_init.cc         | 2 +-
 .../testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc   | 2 +-
 .../testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc   | 2 +-
 .../testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc | 2 +-
 libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc         | 2 +-
 libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc        | 2 +-
 libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc         | 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
index 01f7475631e6..859629e625f8 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-cmpxchg-word "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
index 89ed381fe057..2e154178dbd7 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++11 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-cmpxchg-word "" }
 
 // Copyright (C) 2009-2023 Free Software Foundation, Inc.
 //
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
index f3f38b54dbcd..6439873be133 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
@@ -16,7 +16,7 @@
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-cmpxchg-word "" }
 
 #include <atomic>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
index 6f723eb5f4e7..6cb1ae2b6dda 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++11 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-cmpxchg-word "" }
 
 // Copyright (C) 2008-2023 Free Software Foundation, Inc.
 //
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
index 6f723eb5f4e7..6cb1ae2b6dda 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++11 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-cmpxchg-word "" }
 
 // Copyright (C) 2008-2023 Free Software Foundation, Inc.
 //
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
index 2a3d1d468c22..25ccd2e94336 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
@@ -1,5 +1,5 @@
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-cmpxchg-word "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
index f8751756d02c..c342b1aae292 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
@@ -16,7 +16,7 @@
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-cmpxchg-word "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
index eb22afca03a2..134fb16506c3 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
@@ -16,7 +16,7 @@
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-cmpxchg-word "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
index 6fe00b557567..fd26a053151f 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
@@ -16,7 +16,7 @@
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++20 } }
-// { dg-require-thread-fence "" }
+// { dg-require-atomic-cmpxchg-word "" }
 // { dg-add-options libatomic }
 
 #include <atomic>
-- 
2.30.2

Ping: [PATCH v2 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-cmpxchg-word

[Hans-Peter Nilsson]

Ping.

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Wed, 4 Oct 2023 19:08:16 +0200
> 
> s/atomic-exchange/atomic-cmpxchg-word/g.
> Tested as v1.
> 
> Ok to commit?
> -- >8 --
> These tests actually use a form of atomic compare and exchange
> operation, not just atomic loading and storing.  Some targets (not
> supported by e.g. libatomic) have atomic loading and storing, but not
> compare and exchange, yielding linker errors for missing library
> functions.
> 
> This change is just for existing uses of
> dg-require-thread-fence.  It does not fix any other tests
> that should also be gated on dg-require-atomic-cmpxchg-word.
> 
> 	* testsuite/29_atomics/atomic/compare_exchange_padding.cc,
> 	testsuite/29_atomics/atomic_flag/clear/1.cc,
> 	testsuite/29_atomics/atomic_flag/cons/value_init.cc,
> 	testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc,
> 	testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc,
> 	testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc,
> 	testsuite/29_atomics/atomic_ref/generic.cc,
> 	testsuite/29_atomics/atomic_ref/integral.cc,
> 	testsuite/29_atomics/atomic_ref/pointer.cc: Replace
> 	dg-require-thread-fence with dg-require-atomic-cmpxchg-word.
> ---
>  .../testsuite/29_atomics/atomic/compare_exchange_padding.cc     | 2 +-
>  libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc        | 2 +-
>  .../testsuite/29_atomics/atomic_flag/cons/value_init.cc         | 2 +-
>  .../testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc   | 2 +-
>  .../testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc   | 2 +-
>  .../testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc | 2 +-
>  libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc         | 2 +-
>  libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc        | 2 +-
>  libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc         | 2 +-
>  9 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
> index 01f7475631e6..859629e625f8 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-cmpxchg-word "" }
>  // { dg-add-options libatomic }
>  
>  #include <atomic>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
> index 89ed381fe057..2e154178dbd7 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++11 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-cmpxchg-word "" }
>  
>  // Copyright (C) 2009-2023 Free Software Foundation, Inc.
>  //
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
> index f3f38b54dbcd..6439873be133 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
> @@ -16,7 +16,7 @@
>  // <http://www.gnu.org/licenses/>.
>  
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-cmpxchg-word "" }
>  
>  #include <atomic>
>  #include <testsuite_hooks.h>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
> index 6f723eb5f4e7..6cb1ae2b6dda 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++11 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-cmpxchg-word "" }
>  
>  // Copyright (C) 2008-2023 Free Software Foundation, Inc.
>  //
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
> index 6f723eb5f4e7..6cb1ae2b6dda 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++11 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-cmpxchg-word "" }
>  
>  // Copyright (C) 2008-2023 Free Software Foundation, Inc.
>  //
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
> index 2a3d1d468c22..25ccd2e94336 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
> @@ -1,5 +1,5 @@
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-cmpxchg-word "" }
>  // { dg-add-options libatomic }
>  
>  #include <atomic>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
> index f8751756d02c..c342b1aae292 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
> @@ -16,7 +16,7 @@
>  // <http://www.gnu.org/licenses/>.
>  
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-cmpxchg-word "" }
>  // { dg-add-options libatomic }
>  
>  #include <atomic>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
> index eb22afca03a2..134fb16506c3 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
> @@ -16,7 +16,7 @@
>  // <http://www.gnu.org/licenses/>.
>  
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-cmpxchg-word "" }
>  // { dg-add-options libatomic }
>  
>  #include <atomic>
> diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
> index 6fe00b557567..fd26a053151f 100644
> --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
> +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
> @@ -16,7 +16,7 @@
>  // <http://www.gnu.org/licenses/>.
>  
>  // { dg-do run { target c++20 } }
> -// { dg-require-thread-fence "" }
> +// { dg-require-atomic-cmpxchg-word "" }
>  // { dg-add-options libatomic }
>  
>  #include <atomic>
> -- 
> 2.30.2
> 

Re: Ping: [PATCH v2 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-cmpxchg-word

[Christophe Lyon]

LGTM but I'm not a maintainer ;-)

On Thu, 12 Oct 2023 at 04:22, Hans-Peter Nilsson <hp@axis.com> wrote:
>
> Ping.
>
> > From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Wed, 4 Oct 2023 19:08:16 +0200
> >
> > s/atomic-exchange/atomic-cmpxchg-word/g.
> > Tested as v1.
> >
> > Ok to commit?
> > -- >8 --
> > These tests actually use a form of atomic compare and exchange
> > operation, not just atomic loading and storing.  Some targets (not
> > supported by e.g. libatomic) have atomic loading and storing, but not
> > compare and exchange, yielding linker errors for missing library
> > functions.
> >
> > This change is just for existing uses of
> > dg-require-thread-fence.  It does not fix any other tests
> > that should also be gated on dg-require-atomic-cmpxchg-word.
> >
> >       * testsuite/29_atomics/atomic/compare_exchange_padding.cc,
> >       testsuite/29_atomics/atomic_flag/clear/1.cc,
> >       testsuite/29_atomics/atomic_flag/cons/value_init.cc,
> >       testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc,
> >       testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc,
> >       testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc,
> >       testsuite/29_atomics/atomic_ref/generic.cc,
> >       testsuite/29_atomics/atomic_ref/integral.cc,
> >       testsuite/29_atomics/atomic_ref/pointer.cc: Replace
> >       dg-require-thread-fence with dg-require-atomic-cmpxchg-word.
> > ---
> >  .../testsuite/29_atomics/atomic/compare_exchange_padding.cc     | 2 +-
> >  libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc        | 2 +-
> >  .../testsuite/29_atomics/atomic_flag/cons/value_init.cc         | 2 +-
> >  .../testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc   | 2 +-
> >  .../testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc   | 2 +-
> >  .../testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc | 2 +-
> >  libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc         | 2 +-
> >  libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc        | 2 +-
> >  libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc         | 2 +-
> >  9 files changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
> > index 01f7475631e6..859629e625f8 100644
> > --- a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
> > +++ b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc
> > @@ -1,5 +1,5 @@
> >  // { dg-do run { target c++20 } }
> > -// { dg-require-thread-fence "" }
> > +// { dg-require-atomic-cmpxchg-word "" }
> >  // { dg-add-options libatomic }
> >
> >  #include <atomic>
> > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
> > index 89ed381fe057..2e154178dbd7 100644
> > --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
> > +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc
> > @@ -1,5 +1,5 @@
> >  // { dg-do run { target c++11 } }
> > -// { dg-require-thread-fence "" }
> > +// { dg-require-atomic-cmpxchg-word "" }
> >
> >  // Copyright (C) 2009-2023 Free Software Foundation, Inc.
> >  //
> > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
> > index f3f38b54dbcd..6439873be133 100644
> > --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
> > +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/value_init.cc
> > @@ -16,7 +16,7 @@
> >  // <http://www.gnu.org/licenses/>.
> >
> >  // { dg-do run { target c++20 } }
> > -// { dg-require-thread-fence "" }
> > +// { dg-require-atomic-cmpxchg-word "" }
> >
> >  #include <atomic>
> >  #include <testsuite_hooks.h>
> > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
> > index 6f723eb5f4e7..6cb1ae2b6dda 100644
> > --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
> > +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc
> > @@ -1,5 +1,5 @@
> >  // { dg-do run { target c++11 } }
> > -// { dg-require-thread-fence "" }
> > +// { dg-require-atomic-cmpxchg-word "" }
> >
> >  // Copyright (C) 2008-2023 Free Software Foundation, Inc.
> >  //
> > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
> > index 6f723eb5f4e7..6cb1ae2b6dda 100644
> > --- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
> > +++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc
> > @@ -1,5 +1,5 @@
> >  // { dg-do run { target c++11 } }
> > -// { dg-require-thread-fence "" }
> > +// { dg-require-atomic-cmpxchg-word "" }
> >
> >  // Copyright (C) 2008-2023 Free Software Foundation, Inc.
> >  //
> > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
> > index 2a3d1d468c22..25ccd2e94336 100644
> > --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
> > +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc
> > @@ -1,5 +1,5 @@
> >  // { dg-do run { target c++20 } }
> > -// { dg-require-thread-fence "" }
> > +// { dg-require-atomic-cmpxchg-word "" }
> >  // { dg-add-options libatomic }
> >
> >  #include <atomic>
> > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
> > index f8751756d02c..c342b1aae292 100644
> > --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
> > +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/generic.cc
> > @@ -16,7 +16,7 @@
> >  // <http://www.gnu.org/licenses/>.
> >
> >  // { dg-do run { target c++20 } }
> > -// { dg-require-thread-fence "" }
> > +// { dg-require-atomic-cmpxchg-word "" }
> >  // { dg-add-options libatomic }
> >
> >  #include <atomic>
> > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
> > index eb22afca03a2..134fb16506c3 100644
> > --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
> > +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/integral.cc
> > @@ -16,7 +16,7 @@
> >  // <http://www.gnu.org/licenses/>.
> >
> >  // { dg-do run { target c++20 } }
> > -// { dg-require-thread-fence "" }
> > +// { dg-require-atomic-cmpxchg-word "" }
> >  // { dg-add-options libatomic }
> >
> >  #include <atomic>
> > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
> > index 6fe00b557567..fd26a053151f 100644
> > --- a/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
> > +++ b/libstdc++-v3/testsuite/29_atomics/atomic_ref/pointer.cc
> > @@ -16,7 +16,7 @@
> >  // <http://www.gnu.org/licenses/>.
> >
> >  // { dg-do run { target c++20 } }
> > -// { dg-require-thread-fence "" }
> > +// { dg-require-atomic-cmpxchg-word "" }
> >  // { dg-add-options libatomic }
> >
> >  #include <atomic>
> > --
> > 2.30.2
> >