public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Hans-Peter Nilsson <hp@axis.com>
To: Christophe Lyon <christophe.lyon@linaro.org>
Cc: <gcc-patches@gcc.gnu.org>, <libstdc++@gcc.gnu.org>
Subject: [PATCH 1/2] testsuite: Add dg-require-atomic-exchange non-atomic code
Date: Wed, 4 Oct 2023 04:55:38 +0200	[thread overview]
Message-ID: <20231004025538.489F220439@pchp3.se.axis.com> (raw)
In-Reply-To: <CAPS5khYXtGYLr-pqAQRy_UAsOJATted1Gs0xY4ytTWppFPVJaQ@mail.gmail.com> (message from Christophe Lyon on Tue, 3 Oct 2023 15:20:39 +0200)

> 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
> >
> >
> 

       reply	other threads:[~2023-10-04  2:55 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20230926143439.B589920431@pchp3.se.axis.com>
     [not found] ` <CAPS5khYXtGYLr-pqAQRy_UAsOJATted1Gs0xY4ytTWppFPVJaQ@mail.gmail.com>
2023-10-04  2:55   ` Hans-Peter Nilsson [this message]
2023-10-04  8:13     ` Jonathan Wakely
2023-10-04  3:11   ` [PATCH 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-exchange Hans-Peter Nilsson
2023-10-04  8:29     ` Jonathan Wakely
2023-10-04 15:15       ` Hans-Peter Nilsson
2023-10-04 16:01         ` Jonathan Wakely
2023-10-04 17:04         ` [PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word Hans-Peter Nilsson
2023-10-12  2:21           ` Ping: " Hans-Peter Nilsson
2023-10-12 14:38             ` Christophe Lyon
2023-10-12 16:10               ` Jeff Law
2023-10-12 22:23                 ` Jonathan Wakely
2024-02-07 16:31                   ` Torbjorn SVENSSON
2024-02-07 16:33                     ` Jonathan Wakely
2024-02-07 17:37                       ` Torbjorn SVENSSON
2023-10-04 17:08         ` [PATCH v2 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-cmpxchg-word Hans-Peter Nilsson
2023-10-12  2:22           ` Ping: " Hans-Peter Nilsson
2023-10-12 14:40             ` Christophe Lyon
     [not found]   ` <20231003151633.CADF520410@pchp3.se.axis.com>
     [not found]     ` <CAPS5khY5fNB+AuOJOJPT7U6SgyfnvBKc+PNE4jY9oVG7UNOTCg@mail.gmail.com>
     [not found]       ` <20231004004929.9F76B2042E@pchp3.se.axis.com>
2023-10-04  8:53         ` [PATCH] __atomic_test_and_set: Fall back to library, not non-atomic code Christophe Lyon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231004025538.489F220439@pchp3.se.axis.com \
    --to=hp@axis.com \
    --cc=christophe.lyon@linaro.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).