[llvm] 431e313 - [CGAtomic] Lift stronger requirements on cmpxch and support acquire failure mode
Bruno Cardoso Lopes via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 23 16:45:46 PDT 2021
Author: Bruno Cardoso Lopes
Date: 2021-03-23T16:45:37-07:00
New Revision: 431e3138a1f3fd2bb7b25e1f4766d935058136f8
URL: https://github.com/llvm/llvm-project/commit/431e3138a1f3fd2bb7b25e1f4766d935058136f8
DIFF: https://github.com/llvm/llvm-project/commit/431e3138a1f3fd2bb7b25e1f4766d935058136f8.diff
LOG: [CGAtomic] Lift stronger requirements on cmpxch and support acquire failure mode
- Fix `emitAtomicCmpXchgFailureSet` to support release/acquire (succ/fail) memory order.
- Remove stronger checks for cmpxch.
Effectively, this addresses http://wg21.link/p0418
Differential Revision: https://reviews.llvm.org/D98995
Added:
Modified:
clang/lib/CodeGen/CGAtomic.cpp
clang/test/CodeGen/atomic-ops.c
clang/test/CodeGenOpenCL/atomic-ops.cl
llvm/docs/LangRef.rst
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index c7256e240a31..8ac36e4a6b64 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -427,6 +427,8 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction &CGF, AtomicExpr *E,
else
switch ((llvm::AtomicOrderingCABI)FOS) {
case llvm::AtomicOrderingCABI::relaxed:
+ // 31.7.2.18: "The failure argument shall not be memory_order_release
+ // nor memory_order_acq_rel". Fallback to monotonic.
case llvm::AtomicOrderingCABI::release:
case llvm::AtomicOrderingCABI::acq_rel:
FailureOrder = llvm::AtomicOrdering::Monotonic;
@@ -439,12 +441,10 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction &CGF, AtomicExpr *E,
FailureOrder = llvm::AtomicOrdering::SequentiallyConsistent;
break;
}
- if (isStrongerThan(FailureOrder, SuccessOrder)) {
- // Don't assert on undefined behavior "failure argument shall be no
- // stronger than the success argument".
- FailureOrder =
- llvm::AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrder);
- }
+ // Prior to c++17, "the failure argument shall be no stronger than the
+ // success argument". This condition has been lifted and the only
+ // precondition is 31.7.2.18. Effectively treat this as a DR and skip
+ // language version checks.
emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
FailureOrder, Scope);
return;
@@ -454,8 +454,7 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction &CGF, AtomicExpr *E,
llvm::BasicBlock *MonotonicBB = nullptr, *AcquireBB = nullptr,
*SeqCstBB = nullptr;
MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
- if (SuccessOrder != llvm::AtomicOrdering::Monotonic &&
- SuccessOrder != llvm::AtomicOrdering::Release)
+ if (SuccessOrder != llvm::AtomicOrdering::Monotonic)
AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
if (SuccessOrder == llvm::AtomicOrdering::SequentiallyConsistent)
SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
@@ -479,8 +478,9 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction &CGF, AtomicExpr *E,
emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
Size, SuccessOrder, llvm::AtomicOrdering::Acquire, Scope);
CGF.Builder.CreateBr(ContBB);
- SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
- AcquireBB);
+ 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);
}
diff --git a/clang/test/CodeGen/atomic-ops.c b/clang/test/CodeGen/atomic-ops.c
index 4deb1322e0ff..269406fc3c50 100644
--- a/clang/test/CodeGen/atomic-ops.c
+++ b/clang/test/CodeGen/atomic-ops.c
@@ -500,6 +500,7 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int success, int fail) {
// CHECK: [[RELEASE]]
// CHECK: switch {{.*}}, label %[[RELEASE_MONOTONIC:[0-9a-zA-Z._]+]] [
+ // CHECK-NEXT: i32 2, label %[[RELEASE_ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[ACQREL]]
@@ -527,6 +528,14 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} acquire acquire, align
// CHECK: br
+ // CHECK: [[RELEASE_MONOTONIC]]
+ // CHECK: cmpxchg {{.*}} release monotonic, align
+ // CHECK: br
+
+ // CHECK: [[RELEASE_ACQUIRE]]
+ // CHECK: cmpxchg {{.*}} release acquire, align
+ // CHECK: br
+
// CHECK: [[ACQREL_MONOTONIC]]
// CHECK: cmpxchg {{.*}} acq_rel monotonic, align
// CHECK: br
@@ -562,6 +571,20 @@ void generalWeakness(int *ptr, int *ptr2, _Bool weak) {
// CHECK-NOT: br
// CHECK: cmpxchg weak {{.*}} seq_cst seq_cst, align
// CHECK: br
+
+ __atomic_compare_exchange_n(ptr, ptr2, 42, weak, memory_order_release, memory_order_acquire);
+ // CHECK: switch i1 {{.*}}, label %[[WEAK:[0-9a-zA-Z._]+]] [
+ // CHECK-NEXT: i1 false, label %[[STRONG:[0-9a-zA-Z._]+]]
+
+ // CHECK: [[STRONG]]
+ // CHECK-NOT: br
+ // CHECK: cmpxchg {{.*}} release acquire
+ // CHECK: br
+
+ // CHECK: [[WEAK]]
+ // CHECK-NOT: br
+ // CHECK: cmpxchg weak {{.*}} release acquire
+ // CHECK: br
}
// Having checked the flow in the previous two cases, we'll trust clang to
@@ -576,7 +599,9 @@ void EMIT_ALL_THE_THINGS(int *ptr, int *ptr2, int new, _Bool weak, int success,
// CHECK: = cmpxchg weak {{.*}} acquire monotonic, align
// CHECK: = cmpxchg weak {{.*}} acquire acquire, align
// CHECK: = cmpxchg {{.*}} release monotonic, align
+ // CHECK: = cmpxchg {{.*}} release acquire, align
// CHECK: = cmpxchg weak {{.*}} release monotonic, align
+ // CHECK: = cmpxchg weak {{.*}} release acquire, align
// CHECK: = cmpxchg {{.*}} acq_rel monotonic, align
// CHECK: = cmpxchg {{.*}} acq_rel acquire, align
// CHECK: = cmpxchg weak {{.*}} acq_rel monotonic, align
diff --git a/clang/test/CodeGenOpenCL/atomic-ops.cl b/clang/test/CodeGenOpenCL/atomic-ops.cl
index bd5a01c5434a..e5da50883c65 100644
--- a/clang/test/CodeGenOpenCL/atomic-ops.cl
+++ b/clang/test/CodeGenOpenCL/atomic-ops.cl
@@ -227,6 +227,7 @@ void generalFailureOrder(atomic_int *ptr, int *ptr2, int success, int fail) {
// CHECK: [[RELEASE]]
// CHECK: switch {{.*}}, label %[[RELEASE_MONOTONIC:[0-9a-zA-Z._]+]] [
+ // CHECK-NEXT: i32 2, label %[[RELEASE_ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[ACQREL]]
@@ -254,6 +255,14 @@ void generalFailureOrder(atomic_int *ptr, int *ptr2, int success, int fail) {
// CHECK: cmpxchg {{.*}} acquire acquire, align 4
// CHECK: br
+ // CHECK: [[RELEASE_MONOTONIC]]
+ // CHECK: cmpxchg {{.*}} release monotonic, align 4
+ // CHECK: br
+
+ // CHECK: [[RELEASE_ACQUIRE]]
+ // CHECK: cmpxchg {{.*}} release acquire, align 4
+ // CHECK: br
+
// CHECK: [[ACQREL_MONOTONIC]]
// CHECK: cmpxchg {{.*}} acq_rel monotonic, align 4
// CHECK: br
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 4ddedd8cf087..09a8933c110a 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -9768,8 +9768,7 @@ this ``cmpxchg`` with other :ref:`volatile operations <volatile>`.
The success and failure :ref:`ordering <ordering>` arguments specify how this
``cmpxchg`` synchronizes with other atomic operations. Both ordering parameters
-must be at least ``monotonic``, the ordering constraint on failure must be no
-stronger than that on success, and the failure ordering cannot be either
+must be at least ``monotonic``, the failure ordering cannot be either
``release`` or ``acq_rel``.
A ``cmpxchg`` instruction can also take an optional
More information about the llvm-commits
mailing list