public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: Hans-Peter Nilsson <hp@axis.com>
Cc: christophe.lyon@linaro.org, gcc-patches@gcc.gnu.org,
	libstdc++@gcc.gnu.org
Subject: Re: [PATCH 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-exchange
Date: Wed, 4 Oct 2023 17:01:18 +0100	[thread overview]
Message-ID: <CACb0b4=rkomRHWsdJT695n+=Lgo3BWzdFMT+yDpmcWrC7=AM-g@mail.gmail.com> (raw)
In-Reply-To: <20231004151528.2ADB12043D@pchp3.se.axis.com>

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
>


  reply	other threads:[~2023-10-04 16:01 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   ` [PATCH 1/2] testsuite: Add dg-require-atomic-exchange non-atomic code Hans-Peter Nilsson
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 [this message]
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='CACb0b4=rkomRHWsdJT695n+=Lgo3BWzdFMT+yDpmcWrC7=AM-g@mail.gmail.com' \
    --to=jwakely@redhat.com \
    --cc=christophe.lyon@linaro.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hp@axis.com \
    --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).