This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG431e3138a1f3: [CGAtomic] Lift stronger requirements on
cmpxch and support acquire failure mode (authored by bruno).
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98995/new/
https://reviews.llvm.org/D98995
Files:
clang/lib/CodeGen/CGAtomic.cpp
clang/test/CodeGen/atomic-ops.c
clang/test/CodeGenOpenCL/atomic-ops.cl
llvm/docs/LangRef.rst
Index: llvm/docs/LangRef.rst
===================================================================
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -9768,8 +9768,7 @@
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
Index: clang/test/CodeGenOpenCL/atomic-ops.cl
===================================================================
--- clang/test/CodeGenOpenCL/atomic-ops.cl
+++ clang/test/CodeGenOpenCL/atomic-ops.cl
@@ -227,6 +227,7 @@
// 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 @@
// 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
Index: clang/test/CodeGen/atomic-ops.c
===================================================================
--- clang/test/CodeGen/atomic-ops.c
+++ clang/test/CodeGen/atomic-ops.c
@@ -500,6 +500,7 @@
// 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 @@
// 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 @@
// 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 @@
// 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
Index: clang/lib/CodeGen/CGAtomic.cpp
===================================================================
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -427,6 +427,8 @@
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 @@
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 @@
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 @@
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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
