[llvm] 819e0d1 - [CGAtomic] Lift strong requirement for remaining compare_exchange combinations
Bruno Cardoso Lopes via llvm-commits
llvm-commits at lists.llvm.org
Thu May 6 21:09:02 PDT 2021
Author: Bruno Cardoso Lopes
Date: 2021-05-06T21:05:20-07:00
New Revision: 819e0d105e84c6081cfcfa0e38fd257b6124553a
URL: https://github.com/llvm/llvm-project/commit/819e0d105e84c6081cfcfa0e38fd257b6124553a
DIFF: https://github.com/llvm/llvm-project/commit/819e0d105e84c6081cfcfa0e38fd257b6124553a.diff
LOG: [CGAtomic] Lift strong requirement for remaining compare_exchange combinations
Follow up on 431e3138a and complete the other possible combinations.
Besides enforcing the new behavior, it also mitigates TSAN false positives when
combining orders that used to be stronger.
Added:
Modified:
clang/lib/CodeGen/CGAtomic.cpp
clang/test/CodeGen/atomic-ops.c
clang/test/CodeGenOpenCL/atomic-ops.cl
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/IR/Verifier.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index ef016cf24f134..cc85375f08ddf 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -451,47 +451,38 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction &CGF, AtomicExpr *E,
}
// Create all the relevant BB's
- llvm::BasicBlock *MonotonicBB = nullptr, *AcquireBB = nullptr,
- *SeqCstBB = nullptr;
- MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
- if (SuccessOrder != llvm::AtomicOrdering::Monotonic)
- AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
- if (SuccessOrder == llvm::AtomicOrdering::SequentiallyConsistent)
- SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
-
- llvm::BasicBlock *ContBB = CGF.createBasicBlock("atomic.continue", CGF.CurFn);
-
- llvm::SwitchInst *SI = CGF.Builder.CreateSwitch(FailureOrderVal, MonotonicBB);
-
- // Emit all the
diff erent atomics
+ auto *MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
+ auto *AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
+ auto *SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
+ auto *ContBB = CGF.createBasicBlock("atomic.continue", CGF.CurFn);
// MonotonicBB is arbitrarily chosen as the default case; in practice, this
// doesn't matter unless someone is crazy enough to use something that
// doesn't fold to a constant for the ordering.
+ llvm::SwitchInst *SI = CGF.Builder.CreateSwitch(FailureOrderVal, MonotonicBB);
+ // Implemented as acquire, since it's the closest in LLVM.
+ SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
+ AcquireBB);
+ SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
+ AcquireBB);
+ SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::seq_cst),
+ SeqCstBB);
+
+ // Emit all the
diff erent atomics
CGF.Builder.SetInsertPoint(MonotonicBB);
emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
Size, SuccessOrder, llvm::AtomicOrdering::Monotonic, Scope);
CGF.Builder.CreateBr(ContBB);
- if (AcquireBB) {
- CGF.Builder.SetInsertPoint(AcquireBB);
- emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
- Size, SuccessOrder, llvm::AtomicOrdering::Acquire, Scope);
- CGF.Builder.CreateBr(ContBB);
- if (SuccessOrder != llvm::AtomicOrdering::Release)
- SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
- AcquireBB);
- SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
- AcquireBB);
- }
- if (SeqCstBB) {
- CGF.Builder.SetInsertPoint(SeqCstBB);
- emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
- llvm::AtomicOrdering::SequentiallyConsistent, Scope);
- CGF.Builder.CreateBr(ContBB);
- SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::seq_cst),
- SeqCstBB);
- }
+ CGF.Builder.SetInsertPoint(AcquireBB);
+ emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
+ llvm::AtomicOrdering::Acquire, Scope);
+ CGF.Builder.CreateBr(ContBB);
+
+ CGF.Builder.SetInsertPoint(SeqCstBB);
+ emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
+ llvm::AtomicOrdering::SequentiallyConsistent, Scope);
+ CGF.Builder.CreateBr(ContBB);
CGF.Builder.SetInsertPoint(ContBB);
}
diff --git a/clang/test/CodeGen/atomic-ops.c b/clang/test/CodeGen/atomic-ops.c
index 269406fc3c50f..02edec19bca93 100644
--- a/clang/test/CodeGen/atomic-ops.c
+++ b/clang/test/CodeGen/atomic-ops.c
@@ -490,29 +490,36 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int success, int fail) {
// CHECK: [[MONOTONIC]]
// CHECK: switch {{.*}}, label %[[MONOTONIC_MONOTONIC:[0-9a-zA-Z._]+]] [
+ // CHECK-NEXT: i32 1, label %[[MONOTONIC_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 2, label %[[MONOTONIC_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 5, label %[[MONOTONIC_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[ACQUIRE]]
// CHECK: switch {{.*}}, label %[[ACQUIRE_MONOTONIC:[0-9a-zA-Z._]+]] [
// CHECK-NEXT: i32 1, label %[[ACQUIRE_ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: i32 2, label %[[ACQUIRE_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 5, label %[[ACQUIRE_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[RELEASE]]
// CHECK: switch {{.*}}, label %[[RELEASE_MONOTONIC:[0-9a-zA-Z._]+]] [
+ // CHECK-NEXT: i32 1, label %[[RELEASE_ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: i32 2, label %[[RELEASE_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 5, label %[[RELEASE_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[ACQREL]]
// CHECK: switch {{.*}}, label %[[ACQREL_MONOTONIC:[0-9a-zA-Z._]+]] [
// CHECK-NEXT: i32 1, label %[[ACQREL_ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: i32 2, label %[[ACQREL_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 5, label %[[ACQREL_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[SEQCST]]
// CHECK: switch {{.*}}, label %[[SEQCST_MONOTONIC:[0-9a-zA-Z._]+]] [
// CHECK-NEXT: i32 1, label %[[SEQCST_ACQUIRE:[0-9a-zA-Z._]+]]
- // CHECK-NEXT: i32 2, label %[[SEQCST_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 2, label %[[SEQCST_ACQUIRE]]
// CHECK-NEXT: i32 5, label %[[SEQCST_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
@@ -520,6 +527,14 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} monotonic monotonic, align
// CHECK: br
+ // CHECK: [[MONOTONIC_ACQUIRE]]
+ // CHECK: cmpxchg {{.*}} monotonic acquire, align
+ // CHECK: br
+
+ // CHECK: [[MONOTONIC_SEQCST]]
+ // CHECK: cmpxchg {{.*}} monotonic seq_cst, align
+ // CHECK: br
+
// CHECK: [[ACQUIRE_MONOTONIC]]
// CHECK: cmpxchg {{.*}} acquire monotonic, align
// CHECK: br
@@ -528,6 +543,10 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} acquire acquire, align
// CHECK: br
+ // CHECK: [[ACQUIRE_SEQCST]]
+ // CHECK: cmpxchg {{.*}} acquire seq_cst, align
+ // CHECK: br
+
// CHECK: [[RELEASE_MONOTONIC]]
// CHECK: cmpxchg {{.*}} release monotonic, align
// CHECK: br
@@ -536,6 +555,10 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} release acquire, align
// CHECK: br
+ // CHECK: [[RELEASE_SEQCST]]
+ // CHECK: cmpxchg {{.*}} release seq_cst, align
+ // CHECK: br
+
// CHECK: [[ACQREL_MONOTONIC]]
// CHECK: cmpxchg {{.*}} acq_rel monotonic, align
// CHECK: br
@@ -544,6 +567,10 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} acq_rel acquire, align
// CHECK: br
+ // CHECK: [[ACQREL_SEQCST]]
+ // CHECK: cmpxchg {{.*}} acq_rel seq_cst, align
+ // CHECK: br
+
// CHECK: [[SEQCST_MONOTONIC]]
// CHECK: cmpxchg {{.*}} seq_cst monotonic, align
// CHECK: br
@@ -593,19 +620,29 @@ void EMIT_ALL_THE_THINGS(int *ptr, int *ptr2, int new, _Bool weak, int success,
__atomic_compare_exchange(ptr, ptr2, &new, weak, success, fail);
// CHECK: = cmpxchg {{.*}} monotonic monotonic, align
+ // CHECK: = cmpxchg {{.*}} monotonic acquire, align
+ // CHECK: = cmpxchg {{.*}} monotonic seq_cst, align
// CHECK: = cmpxchg weak {{.*}} monotonic monotonic, align
+ // CHECK: = cmpxchg weak {{.*}} monotonic acquire, align
+ // CHECK: = cmpxchg weak {{.*}} monotonic seq_cst, align
// CHECK: = cmpxchg {{.*}} acquire monotonic, align
// CHECK: = cmpxchg {{.*}} acquire acquire, align
+ // CHECK: = cmpxchg {{.*}} acquire seq_cst, align
// CHECK: = cmpxchg weak {{.*}} acquire monotonic, align
// CHECK: = cmpxchg weak {{.*}} acquire acquire, align
+ // CHECK: = cmpxchg weak {{.*}} acquire seq_cst, align
// CHECK: = cmpxchg {{.*}} release monotonic, align
// CHECK: = cmpxchg {{.*}} release acquire, align
+ // CHECK: = cmpxchg {{.*}} release seq_cst, align
// CHECK: = cmpxchg weak {{.*}} release monotonic, align
// CHECK: = cmpxchg weak {{.*}} release acquire, align
+ // CHECK: = cmpxchg weak {{.*}} release seq_cst, align
// CHECK: = cmpxchg {{.*}} acq_rel monotonic, align
// CHECK: = cmpxchg {{.*}} acq_rel acquire, align
+ // CHECK: = cmpxchg {{.*}} acq_rel seq_cst, align
// CHECK: = cmpxchg weak {{.*}} acq_rel monotonic, align
// CHECK: = cmpxchg weak {{.*}} acq_rel acquire, align
+ // CHECK: = cmpxchg weak {{.*}} acq_rel seq_cst, align
// CHECK: = cmpxchg {{.*}} seq_cst monotonic, align
// CHECK: = cmpxchg {{.*}} seq_cst acquire, align
// CHECK: = cmpxchg {{.*}} seq_cst seq_cst, align
diff --git a/clang/test/CodeGenOpenCL/atomic-ops.cl b/clang/test/CodeGenOpenCL/atomic-ops.cl
index b5cfad8be7aee..17d7735004f44 100644
--- a/clang/test/CodeGenOpenCL/atomic-ops.cl
+++ b/clang/test/CodeGenOpenCL/atomic-ops.cl
@@ -225,7 +225,7 @@ void failureOrder(atomic_int *ptr, int *ptr2) {
// CHECK-LABEL: @generalFailureOrder
void generalFailureOrder(atomic_int *ptr, int *ptr2, int success, int fail) {
__opencl_atomic_compare_exchange_strong(ptr, ptr2, 42, success, fail, memory_scope_work_group);
- // CHECK: switch i32 {{.*}}, label %[[MONOTONIC:[0-9a-zA-Z._]+]] [
+// CHECK: switch i32 {{.*}}, label %[[MONOTONIC:[0-9a-zA-Z._]+]] [
// CHECK-NEXT: i32 1, label %[[ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: i32 2, label %[[ACQUIRE]]
// CHECK-NEXT: i32 3, label %[[RELEASE:[0-9a-zA-Z._]+]]
@@ -234,29 +234,36 @@ void generalFailureOrder(atomic_int *ptr, int *ptr2, int success, int fail) {
// CHECK: [[MONOTONIC]]
// CHECK: switch {{.*}}, label %[[MONOTONIC_MONOTONIC:[0-9a-zA-Z._]+]] [
+ // CHECK-NEXT: i32 1, label %[[MONOTONIC_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 2, label %[[MONOTONIC_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 5, label %[[MONOTONIC_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[ACQUIRE]]
// CHECK: switch {{.*}}, label %[[ACQUIRE_MONOTONIC:[0-9a-zA-Z._]+]] [
// CHECK-NEXT: i32 1, label %[[ACQUIRE_ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: i32 2, label %[[ACQUIRE_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 5, label %[[ACQUIRE_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[RELEASE]]
// CHECK: switch {{.*}}, label %[[RELEASE_MONOTONIC:[0-9a-zA-Z._]+]] [
+ // CHECK-NEXT: i32 1, label %[[RELEASE_ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: i32 2, label %[[RELEASE_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 5, label %[[RELEASE_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[ACQREL]]
// CHECK: switch {{.*}}, label %[[ACQREL_MONOTONIC:[0-9a-zA-Z._]+]] [
// CHECK-NEXT: i32 1, label %[[ACQREL_ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: i32 2, label %[[ACQREL_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 5, label %[[ACQREL_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[SEQCST]]
// CHECK: switch {{.*}}, label %[[SEQCST_MONOTONIC:[0-9a-zA-Z._]+]] [
// CHECK-NEXT: i32 1, label %[[SEQCST_ACQUIRE:[0-9a-zA-Z._]+]]
- // CHECK-NEXT: i32 2, label %[[SEQCST_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 2, label %[[SEQCST_ACQUIRE]]
// CHECK-NEXT: i32 5, label %[[SEQCST_SEQCST:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
@@ -264,6 +271,14 @@ void generalFailureOrder(atomic_int *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} monotonic monotonic, align 4
// CHECK: br
+ // CHECK: [[MONOTONIC_ACQUIRE]]
+ // CHECK: cmpxchg {{.*}} monotonic acquire, align 4
+ // CHECK: br
+
+ // CHECK: [[MONOTONIC_SEQCST]]
+ // CHECK: cmpxchg {{.*}} monotonic seq_cst, align 4
+ // CHECK: br
+
// CHECK: [[ACQUIRE_MONOTONIC]]
// CHECK: cmpxchg {{.*}} acquire monotonic, align 4
// CHECK: br
@@ -272,6 +287,10 @@ void generalFailureOrder(atomic_int *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} acquire acquire, align 4
// CHECK: br
+ // CHECK: [[ACQUIRE_SEQCST]]
+ // CHECK: cmpxchg {{.*}} acquire seq_cst, align 4
+ // CHECK: br
+
// CHECK: [[RELEASE_MONOTONIC]]
// CHECK: cmpxchg {{.*}} release monotonic, align 4
// CHECK: br
@@ -280,6 +299,10 @@ void generalFailureOrder(atomic_int *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} release acquire, align 4
// CHECK: br
+ // CHECK: [[RELEASE_SEQCST]]
+ // CHECK: cmpxchg {{.*}} release seq_cst, align 4
+ // CHECK: br
+
// CHECK: [[ACQREL_MONOTONIC]]
// CHECK: cmpxchg {{.*}} acq_rel monotonic, align 4
// CHECK: br
@@ -288,6 +311,10 @@ void generalFailureOrder(atomic_int *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} acq_rel acquire, align 4
// CHECK: br
+ // CHECK: [[ACQREL_SEQCST]]
+ // CHECK: cmpxchg {{.*}} acq_rel seq_cst, align 4
+ // CHECK: br
+
// CHECK: [[SEQCST_MONOTONIC]]
// CHECK: cmpxchg {{.*}} seq_cst monotonic, align 4
// CHECK: br
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index e5017cea859f6..df2bea043ddc4 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -7571,9 +7571,6 @@ int LLParser::parseCmpXchg(Instruction *&Inst, PerFunctionState &PFS) {
if (SuccessOrdering == AtomicOrdering::Unordered ||
FailureOrdering == AtomicOrdering::Unordered)
return tokError("cmpxchg cannot be unordered");
- if (isStrongerThan(FailureOrdering, SuccessOrdering))
- return tokError("cmpxchg failure argument shall be no stronger than the "
- "success argument");
if (FailureOrdering == AtomicOrdering::Release ||
FailureOrdering == AtomicOrdering::AcquireRelease)
return tokError(
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index 8ddadf410d56b..6abe9135614a3 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -1558,9 +1558,6 @@ void AtomicCmpXchgInst::Init(Value *Ptr, Value *Cmp, Value *NewVal,
"AtomicCmpXchg instructions must be atomic!");
assert(FailureOrdering != AtomicOrdering::NotAtomic &&
"AtomicCmpXchg instructions must be atomic!");
- assert(!isStrongerThan(FailureOrdering, SuccessOrdering) &&
- "AtomicCmpXchg failure argument shall be no stronger than the success "
- "argument");
assert(FailureOrdering != AtomicOrdering::Release &&
FailureOrdering != AtomicOrdering::AcquireRelease &&
"AtomicCmpXchg failure ordering cannot include release semantics");
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 856d1896489ff..117c707f9e839 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -3831,10 +3831,6 @@ void Verifier::visitAtomicCmpXchgInst(AtomicCmpXchgInst &CXI) {
"cmpxchg instructions cannot be unordered.", &CXI);
Assert(CXI.getFailureOrdering() != AtomicOrdering::Unordered,
"cmpxchg instructions cannot be unordered.", &CXI);
- Assert(!isStrongerThan(CXI.getFailureOrdering(), CXI.getSuccessOrdering()),
- "cmpxchg instructions failure argument shall be no stronger than the "
- "success argument",
- &CXI);
Assert(CXI.getFailureOrdering() != AtomicOrdering::Release &&
CXI.getFailureOrdering() != AtomicOrdering::AcquireRelease,
"cmpxchg failure ordering cannot include release semantics", &CXI);
More information about the llvm-commits
mailing list