-
Notifications
You must be signed in to change notification settings - Fork 15k
Description
In multiple places throughout the llvm codebase, it is assumed that syncscope("system")
is the default.
As an example, when an atomic operation is created with syncscope("system")
printing it omits that syncscope.
llvm-project/llvm/lib/CodeGen/MachineOperand.cpp
Lines 530 to 531 in 8a229f5
case SyncScope::System: | |
break; |
When creating an atomic load instruction without a syncscope specifiy it is assumed to be system
llvm-project/llvm/lib/IR/Instructions.cpp
Lines 1271 to 1275 in 8a229f5
LoadInst::LoadInst(Type *Ty, Value *Ptr, const Twine &Name, bool isVolatile, | |
Align Align, InsertPosition InsertBef) | |
: LoadInst(Ty, Ptr, Name, isVolatile, Align, AtomicOrdering::NotAtomic, | |
SyncScope::System, InsertBef) {} | |
This default nature of system
should be documented in the language reference, where currently only target-specific
(e.g. what backends like AMDGPU have) and single-threaded
exist.
A wrinkle is the PTX backend. PTX does mostly use NVVM intrinsics, but also some LLVM atomic operations.
Eventually, I would love to see a move away from the nvvm intrinsics and instead fully use the LLVM atomic operation,
but currently PTX does not handle syncscopes and thus "mistranslates" the syncscope("system")
to the PTX syncscope device
...
This essentially means that there is an ambiguity between syncscope("system")
and the syncscope being left undefined.
I wanted to start work on supporting atomics with a syncscope argument in the PTX backend, but this conflict in defaults would potentially break users who have been emitting atomic operations on PTX. To be clear, it would be a performance issue and not a semantic one.