As part of implementing P3111 Atomic reductions in libc++ (#186935), i’ve added support for these operations to LLVM IR in a local branch. How these are exposed in the IR significantly influences how the different backends override them and how clang builtins are lowered. Therefore I’m opening this RFC to gain feedback on the preferred way to model these in the IR.
Background
Store atomics (C++26 void std::atomic<int>::store_add(int v, memory_order);) are atomic read-modify-write operations that do not return the value read (they return void), and whose read operation is excluded from building acquire patterns with acquire fences. They are therefore subtly different from fetch atomics (int std::atomic<int>::fetch_add(int v, memory_order) in terms of semantics, optimizations, memory orders supported (e.g. acquire is not supported), etc.
Multiple LLVM backends support them (NVPTX red, Aarch64 STADD, ppc64le AMOs, probably others…).
In particular, optimizing a fetch atomic operation into a store atomic operation is - in general - unsound, i.e., programs that need to generate these reliably, need an opt-in mechanism. On the other hand, store atomic operations are always correct to lower to fetch atomic operations, and the AtomicExpand pass can expand them to fetch atomics to provide generic support for them for all backends that choose to not override them.
IR exposure
My preference is to add a storermw (or similarly named) option to atomicrmw.
- PRO: avoids adding a new IR instruction, can tweak pre-existing optimization passes, ensures the same operations are supported with/without the option, can use
syncscopeand memory ordering fields, etc. - CON: bolts subtly different semantics on top of
atomicrmw
The other viable alternative (which i’ve implemented locally) is to add a new instruction, e.g., atomicred that keeps these operations separate from atomicrmw. The main CONs w.r.t an option on atomicrmw is: higher impl/maintenance effort, higher effort to support same operations in both. I think adding a modifier to atomicrmw has better ROI.
From the point-of-view of frontend and backends, there is not a huge difference between these two options in my opinion.
As part of this effort I tried a few other alternatives, that turned out to not be viable. The main one was exposing these new operations via LLVM intrinsics (llvm.atomicred...). This approach avoids adding/modifying pre-existing instructions, but since we don’t support memory orders, syncscope, … arguments on intrinsics, it turned out to be a pretty painful change (its hard to map synscope to integers in a frontend, would need to use i32 for memory orders, …).
Does anyone have any feedback on the best way to proceed?
Or in the absence of feedback, does anyone object to adding a new option to atomicrmw?
Thank you
Gonzalo