On Wed, 4 Oct 2023 at 16:15, Hans-Peter Nilsson <h...@axis.com> wrote: > > > 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.
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 >
