From: David Majnemer <david.majnemer@gmail.com>
Date: Mon, 10 Mar 2014 21:35:33 +0000 (+0000)
Subject: IRGen: __c11/__atomic compare-and-exchange should respect the standard
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=76246f61fadc921dcb5bb7aec86bc740465bffc1;p=clang

IRGen: __c11/__atomic compare-and-exchange should respect the standard

Summary:
'Expected' should only be modified if the operation fails.

This fixes PR18899.

Reviewers: chandlerc, rsmith, rjmccall

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D2922

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@203493 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/CodeGen/CGAtomic.cpp b/lib/CodeGen/CGAtomic.cpp
index cb92f9ad22..8865577404 100644
--- a/lib/CodeGen/CGAtomic.cpp
+++ b/lib/CodeGen/CGAtomic.cpp
@@ -201,16 +201,42 @@ EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, llvm::Value *Dest,
   case AtomicExpr::AO__atomic_compare_exchange_n: {
     // Note that cmpxchg only supports specifying one ordering and
     // doesn't support weak cmpxchg, at least at the moment.
-    llvm::LoadInst *LoadVal1 = CGF.Builder.CreateLoad(Val1);
-    LoadVal1->setAlignment(Align);
-    llvm::LoadInst *LoadVal2 = CGF.Builder.CreateLoad(Val2);
-    LoadVal2->setAlignment(Align);
-    llvm::AtomicCmpXchgInst *CXI =
-        CGF.Builder.CreateAtomicCmpXchg(Ptr, LoadVal1, LoadVal2, Order);
-    CXI->setVolatile(E->isVolatile());
-    llvm::StoreInst *StoreVal1 = CGF.Builder.CreateStore(CXI, Val1);
-    StoreVal1->setAlignment(Align);
-    llvm::Value *Cmp = CGF.Builder.CreateICmpEQ(CXI, LoadVal1);
+
+    llvm::LoadInst *Expected = CGF.Builder.CreateLoad(Val1);
+    Expected->setAlignment(Align);
+    llvm::LoadInst *Desired = CGF.Builder.CreateLoad(Val2);
+    Desired->setAlignment(Align);
+    llvm::AtomicCmpXchgInst *Old =
+        CGF.Builder.CreateAtomicCmpXchg(Ptr, Expected, Desired, Order);
+    Old->setVolatile(E->isVolatile());
+
+    // Cmp holds the result of the compare-exchange operation: true on success,
+    // false on failure.
+    llvm::Value *Cmp = CGF.Builder.CreateICmpEQ(Old, Expected);
+
+    // This basic block is used to hold the store instruction if the operation
+    // failed.
+    llvm::BasicBlock *StoreExpectedBB =
+        CGF.createBasicBlock("cmpxchg.store_expected", CGF.CurFn);
+
+    // This basic block is the exit point of the operation, we should end up
+    // here regardless of whether or not the operation succeeded.
+    llvm::BasicBlock *ContinueBB =
+        CGF.createBasicBlock("cmpxchg.continue", CGF.CurFn);
+
+    // Update Expected if Expected isn't equal to Old, otherwise branch to the
+    // exit point.
+    CGF.Builder.CreateCondBr(Cmp, ContinueBB, StoreExpectedBB);
+
+    CGF.Builder.SetInsertPoint(StoreExpectedBB);
+    // Update the memory at Expected with Old's value.
+    llvm::StoreInst *StoreExpected = CGF.Builder.CreateStore(Old, Val1);
+    StoreExpected->setAlignment(Align);
+    // Finally, branch to the exit point.
+    CGF.Builder.CreateBr(ContinueBB);
+
+    CGF.Builder.SetInsertPoint(ContinueBB);
+    // Update the memory at Dest with Cmp's value.
     CGF.EmitStoreOfScalar(Cmp, CGF.MakeAddrLValue(Dest, E->getType()));
     return;
   }
diff --git a/test/CodeGen/atomic-ops.c b/test/CodeGen/atomic-ops.c
index 830f21a569..ec3a4b7461 100644
--- a/test/CodeGen/atomic-ops.c
+++ b/test/CodeGen/atomic-ops.c
@@ -91,14 +91,20 @@ int fi3d(int *i) {
 
 _Bool fi4(_Atomic(int) *i) {
   // CHECK: @fi4
-  // CHECK: cmpxchg i32*
+  // CHECK: [[OLD:%[.0-9A-Z_a-z]+]] = cmpxchg i32* [[PTR:%[.0-9A-Z_a-z]+]], i32 [[EXPECTED:%[.0-9A-Z_a-z]+]], i32 [[DESIRED:%[.0-9A-Z_a-z]+]]
+  // CHECK: [[CMP:%[.0-9A-Z_a-z]+]] = icmp eq i32 [[OLD]], [[EXPECTED]]
+  // CHECK: br i1 [[CMP]], label %[[STORE_EXPECTED:[.0-9A-Z_a-z]+]], label %[[CONTINUE:[.0-9A-Z_a-z]+]]
+  // CHECK: store i32 [[OLD]]
   int cmp = 0;
   return __c11_atomic_compare_exchange_strong(i, &cmp, 1, memory_order_acquire, memory_order_acquire);
 }
 
 _Bool fi4a(int *i) {
   // CHECK: @fi4
-  // CHECK: cmpxchg i32*
+  // CHECK: [[OLD:%[.0-9A-Z_a-z]+]] = cmpxchg i32* [[PTR:%[.0-9A-Z_a-z]+]], i32 [[EXPECTED:%[.0-9A-Z_a-z]+]], i32 [[DESIRED:%[.0-9A-Z_a-z]+]]
+  // CHECK: [[CMP:%[.0-9A-Z_a-z]+]] = icmp eq i32 [[OLD]], [[EXPECTED]]
+  // CHECK: br i1 [[CMP]], label %[[STORE_EXPECTED:[.0-9A-Z_a-z]+]], label %[[CONTINUE:[.0-9A-Z_a-z]+]]
+  // CHECK: store i32 [[OLD]]
   int cmp = 0;
   int desired = 1;
   return __atomic_compare_exchange(i, &cmp, &desired, 0, memory_order_acquire, memory_order_acquire);
@@ -106,7 +112,10 @@ _Bool fi4a(int *i) {
 
 _Bool fi4b(int *i) {
   // CHECK: @fi4
-  // CHECK: cmpxchg i32*
+  // CHECK: [[OLD:%[.0-9A-Z_a-z]+]] = cmpxchg i32* [[PTR:%[.0-9A-Z_a-z]+]], i32 [[EXPECTED:%[.0-9A-Z_a-z]+]], i32 [[DESIRED:%[.0-9A-Z_a-z]+]]
+  // CHECK: [[CMP:%[.0-9A-Z_a-z]+]] = icmp eq i32 [[OLD]], [[EXPECTED]]
+  // CHECK: br i1 [[CMP]], label %[[STORE_EXPECTED:[.0-9A-Z_a-z]+]], label %[[CONTINUE:[.0-9A-Z_a-z]+]]
+  // CHECK: store i32 [[OLD]]
   int cmp = 0;
   return __atomic_compare_exchange_n(i, &cmp, 1, 1, memory_order_acquire, memory_order_acquire);
 }