[llvm] [RISCV] Support isel for Zacas for 2*XLen types. (PR #77814)

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 16 03:34:13 PST 2024


asb wrote:

> I didn't see any way to signal AtomicExpandPass to convert them to the same libcalls using any of the shouldExpandAtomic* hooks. So I've forced them to use to CmpXChg expansion. I've disabled the insertion of fences for atomic load/store when we use CmpXChg.
> 
> I've very unsure if this the right thing to do or if we should make changes to AtomicExpand to get back the original libcalls.

I believe that the correct thing is to either consistently generate lock-free implementations for all operations of a given bitwidth, or to dispatch to the libcall. So using the zacas instructions is correct. We may want to update MaxAtomicInlineWidth in clang/lib/Basic/Targets/RISCV.h too.

Looking at other targets that do setMaxAtomicSizeInBitsSupported differently based on a target feature, I'm a bit surprised there's no consideration for potential ABI compatibility. @jyknight, isn't it right that if you have use of atomics cross an ABI boundary then mixing translation units compiled with different maximum atomic width may generate undesirable results? e.g. one TU might perform operations on a memory location using lock-based libcalls while the other uses native lock-free instructions inline. Given distros are looking at rolling out x86-64_v2 as a default this might become more common. But there seems to be no strategy to mitigate this (e.g. always going through a helper function in libatomic). Perhaps this kind of use of double-width atomics across ABI boundaries just doesn't happen in practice?

https://github.com/llvm/llvm-project/pull/77814


More information about the llvm-commits mailing list