diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp --- a/clang/lib/CodeGen/CGAtomic.cpp +++ b/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); } diff --git a/clang/test/CodeGen/atomic-ops.c b/clang/test/CodeGen/atomic-ops.c --- a/clang/test/CodeGen/atomic-ops.c +++ b/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 diff --git a/clang/test/CodeGenOpenCL/atomic-ops.cl b/clang/test/CodeGenOpenCL/atomic-ops.cl --- a/clang/test/CodeGenOpenCL/atomic-ops.cl +++ b/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 diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -9768,8 +9768,7 @@ The success and failure :ref:`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