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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits