[llvm] [LangRef] Clarify what "not ordered" means in the elementwise description and reject seq_cst elementwise atomics (PR #209931)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 15 18:04:26 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Yonah Goldberg (YonahGoldberg)
<details>
<summary>Changes</summary>
This is a follow-up on the discussion here: https://github.com/llvm/llvm-project/pull/204329
The problem with specifying `seq_cst` on `elementwise `atomics is that the LangRef states:
> there is a global total order on all sequentially-consistent operations on all addresses.
But the individual accesses of `elementwise` `seq_cst` atomics are not consistent, which contradicts this. I think we could refine this in the future, but start off by saying `seq_cst` is not permitted.
I think my clarification of "not ordered" makes the following example clearer:
```
// thread 0
store non-atomic [x]
store elementwise atomic release [flag0, flag1] , <1, 1> // writes one to each flag
// thread1
load atomic acquire flag0 == 1
load non-atomic [flag1]
```
there is no execution in which loading from `flag1` in the end is NOT a data race because there is no consistent ordering of the stores of `flag0` and `flag1` and therefore acquiring `flag0` does not give us any information on `flag1`.
My understanding is that in the formal spec working group (which I should probably attend) people were unsure about allowing `acquire`/`release`/`acq_rel` on `elementwise`.
Does this clear up the semantics enough to be comfortable with it?
---
Full diff: https://github.com/llvm/llvm-project/pull/209931.diff
8 Files Affected:
- (modified) llvm/docs/LangRef.md (+8-1)
- (modified) llvm/lib/AsmParser/LLParser.cpp (+2)
- (modified) llvm/lib/IR/Verifier.cpp (+4)
- (modified) llvm/test/Assembler/atomic.ll (+2-2)
- (modified) llvm/test/Assembler/invalid-atomicrmw-elementwise.ll (+8)
- (modified) llvm/test/Bitcode/atomicrmw-elementwise.ll (+2-2)
- (modified) llvm/test/Bitcode/compatibility.ll (+2-2)
- (modified) llvm/unittests/IR/VerifierTest.cpp (+25)
``````````diff
diff --git a/llvm/docs/LangRef.md b/llvm/docs/LangRef.md
index f555ca543ffdc..ca3d32e900dff 100644
--- a/llvm/docs/LangRef.md
+++ b/llvm/docs/LangRef.md
@@ -12130,7 +12130,14 @@ An `atomicrmw` instruction can also take an optional
"{ref}`syncscope <syncscope>`" argument.
If the `elementwise` modifier is present, the instruction has per-element vector
-atomic semantics. It behaves as if it were expanded into one scalar `atomicrmw` per element, that are not ordered with respect to each other.
+atomic semantics. It behaves as if it were expanded into one scalar `atomicrmw`
+per element, that are not ordered with respect to each other. In other words, a
+consistent ordering may not exist between the individual scalar operations of
+the same `elementwise` instruction. Synchronizing with one of the scalar
+operations does not, by itself, establish a happens-before relationship with another scalar operation from the same `elementwise` instruction.
+
+If the `elementwise` modifier is present, the `<ordering>` applies independently to each scalar operation and the `<ordering>` may not be `seq_cst`.
+
Without `elementwise`, vector `atomicrmw` keeps whole-value atomic semantics.
##### Semantics:
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 38d10587b104e..618abb1bd423a 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -9204,6 +9204,8 @@ int LLParser::parseAtomicRMW(Instruction *&Inst, PerFunctionState &PFS) {
if (Ordering == AtomicOrdering::Unordered)
return tokError("atomicrmw cannot be unordered");
+ if (IsElementwise && Ordering == AtomicOrdering::SequentiallyConsistent)
+ return tokError("atomicrmw elementwise cannot be sequentially consistent");
if (!Ptr->getType()->isPointerTy())
return error(PtrLoc, "atomicrmw operand must be a pointer");
if (Val->getType()->isScalableTy())
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index fa0fe7a2e6092..48b04466088fd 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -4706,6 +4706,10 @@ void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
Type *ElTy = RMWI.getOperand(1)->getType();
Type *ScalarTy = ElTy;
if (RMWI.isElementwise()) {
+ Check(RMWI.getOrdering() != AtomicOrdering::SequentiallyConsistent,
+ "atomicrmw elementwise instructions cannot be sequentially "
+ "consistent.",
+ &RMWI);
auto *VecTy = dyn_cast<FixedVectorType>(ElTy);
Check(VecTy, "atomicrmw elementwise operand must have fixed vector type!",
&RMWI, ElTy);
diff --git a/llvm/test/Assembler/atomic.ll b/llvm/test/Assembler/atomic.ll
index 609cd33f61b88..fab483d1cfd27 100644
--- a/llvm/test/Assembler/atomic.ll
+++ b/llvm/test/Assembler/atomic.ll
@@ -154,8 +154,8 @@ define void @fp_vector_atomicrmw(ptr %x, <2 x half> %val) {
; CHECK: %atomic.elem.fadd = atomicrmw elementwise fadd ptr %x, <2 x half> %val monotonic
%atomic.elem.fadd = atomicrmw elementwise fadd ptr %x, <2 x half> %val monotonic
- ; CHECK: %atomic.elem.fadd.vol = atomicrmw volatile elementwise fadd ptr %x, <2 x half> %val seq_cst
- %atomic.elem.fadd.vol = atomicrmw volatile elementwise fadd ptr %x, <2 x half> %val seq_cst
+ ; CHECK: %atomic.elem.fadd.vol = atomicrmw volatile elementwise fadd ptr %x, <2 x half> %val acq_rel
+ %atomic.elem.fadd.vol = atomicrmw volatile elementwise fadd ptr %x, <2 x half> %val acq_rel
ret void
}
diff --git a/llvm/test/Assembler/invalid-atomicrmw-elementwise.ll b/llvm/test/Assembler/invalid-atomicrmw-elementwise.ll
index 2900779420b0c..3c134cbb3b8c2 100644
--- a/llvm/test/Assembler/invalid-atomicrmw-elementwise.ll
+++ b/llvm/test/Assembler/invalid-atomicrmw-elementwise.ll
@@ -3,6 +3,7 @@
; RUN: not llvm-as -disable-output %t/odd-sized.ll 2>&1 | FileCheck %t/odd-sized.ll
; RUN: not llvm-as -disable-output %t/add-must-be-integer.ll 2>&1 | FileCheck %t/add-must-be-integer.ll
; RUN: not llvm-as -disable-output %t/fadd-must-be-fp.ll 2>&1 | FileCheck %t/fadd-must-be-fp.ll
+; RUN: not llvm-as -disable-output %t/seq-cst.ll 2>&1 | FileCheck %t/seq-cst.ll
;--- scalar.ll
; CHECK: atomicrmw elementwise operand must be a fixed vector type
@@ -31,3 +32,10 @@ define <4 x i32> @bad_fadd(ptr %p, <4 x i32> %v) {
%old = atomicrmw elementwise fadd ptr %p, <4 x i32> %v monotonic
ret <4 x i32> %old
}
+
+;--- seq-cst.ll
+; CHECK: atomicrmw elementwise cannot be sequentially consistent
+define <4 x i32> @bad_seq_cst(ptr %p, <4 x i32> %v) {
+ %old = atomicrmw elementwise add ptr %p, <4 x i32> %v seq_cst
+ ret <4 x i32> %old
+}
diff --git a/llvm/test/Bitcode/atomicrmw-elementwise.ll b/llvm/test/Bitcode/atomicrmw-elementwise.ll
index db9c48a80047e..c150a7bf14470 100644
--- a/llvm/test/Bitcode/atomicrmw-elementwise.ll
+++ b/llvm/test/Bitcode/atomicrmw-elementwise.ll
@@ -10,7 +10,7 @@ define <4 x i32> @elem_add(ptr %p, <4 x i32> %v) {
define <4 x float> @elem_fadd(ptr %p, <4 x float> %v) {
; CHECK-LABEL: @elem_fadd(
-; CHECK: %old = atomicrmw elementwise fadd ptr %p, <4 x float> %v seq_cst, align 16
- %old = atomicrmw elementwise fadd ptr %p, <4 x float> %v seq_cst
+; CHECK: %old = atomicrmw elementwise fadd ptr %p, <4 x float> %v acq_rel, align 16
+ %old = atomicrmw elementwise fadd ptr %p, <4 x float> %v acq_rel
ret <4 x float> %old
}
diff --git a/llvm/test/Bitcode/compatibility.ll b/llvm/test/Bitcode/compatibility.ll
index 0ac6da0c9ed29..f2e73395617de 100644
--- a/llvm/test/Bitcode/compatibility.ll
+++ b/llvm/test/Bitcode/compatibility.ll
@@ -1033,8 +1033,8 @@ define void @elementwise_atomics(ptr %word, <4 x i32> %ival, <4 x float> %fval)
; CHECK: %atomicrmw.add = atomicrmw elementwise add ptr %word, <4 x i32> %ival monotonic, align 16
%atomicrmw.add = atomicrmw elementwise add ptr %word, <4 x i32> %ival monotonic, align 16
-; CHECK: %atomicrmw.fadd = atomicrmw elementwise fadd ptr %word, <4 x float> %fval seq_cst, align 16
- %atomicrmw.fadd = atomicrmw elementwise fadd ptr %word, <4 x float> %fval seq_cst, align 16
+; CHECK: %atomicrmw.fadd = atomicrmw elementwise fadd ptr %word, <4 x float> %fval acq_rel, align 16
+ %atomicrmw.fadd = atomicrmw elementwise fadd ptr %word, <4 x float> %fval acq_rel, align 16
ret void
}
diff --git a/llvm/unittests/IR/VerifierTest.cpp b/llvm/unittests/IR/VerifierTest.cpp
index d680dd250d66e..cd323777d755b 100644
--- a/llvm/unittests/IR/VerifierTest.cpp
+++ b/llvm/unittests/IR/VerifierTest.cpp
@@ -429,6 +429,31 @@ TEST(VerifierTest, AtomicRMWElementwiseScalar) {
<< Error;
}
+TEST(VerifierTest, AtomicRMWElementwiseSequentiallyConsistent) {
+ LLVMContext C;
+ Module M("M", C);
+ FunctionType *FTy = FunctionType::get(Type::getVoidTy(C), /*isVarArg=*/false);
+ Function *F = Function::Create(FTy, Function::ExternalLinkage, "foo", M);
+ BasicBlock *Entry = BasicBlock::Create(C, "entry", F);
+ Value *Ptr = PoisonValue::get(PointerType::get(C, 0));
+
+ Type *I32Ty = Type::getInt32Ty(C);
+ Constant *CV = ConstantVector::getSplat(ElementCount::getFixed(2),
+ ConstantInt::get(I32Ty, 0));
+
+ new AtomicRMWInst(AtomicRMWInst::Add, Ptr, CV, Align(8),
+ AtomicOrdering::SequentiallyConsistent, SyncScope::System,
+ /*Elementwise=*/true, Entry);
+ ReturnInst::Create(C, Entry);
+
+ std::string Error;
+ raw_string_ostream ErrorOS(Error);
+ EXPECT_TRUE(verifyFunction(*F, &ErrorOS));
+ EXPECT_TRUE(StringRef(Error).starts_with(
+ "atomicrmw elementwise instructions cannot be sequentially consistent."))
+ << Error;
+}
+
TEST(VerifierTest, AtomicRMWElementwiseIntOpOnFPVector) {
LLVMContext C;
Module M("M", C);
``````````
</details>
https://github.com/llvm/llvm-project/pull/209931
More information about the llvm-commits
mailing list