> From: Jonathan Wakely <jwak...@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

Reply via email to