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

[David Majnemer]

Author: majnemer
Date: Mon Mar 10 16:35:33 2014
New Revision: 203493

URL: http://llvm.org/viewvc/llvm-project?rev=203493&view=rev
Log:
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

Modified:
    cfe/trunk/lib/CodeGen/CGAtomic.cpp
    cfe/trunk/test/CodeGen/atomic-ops.c

Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=203493&r1=203492&r2=203493&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Mon Mar 10 16:35:33 2014
@@ -201,16 +201,42 @@ EmitAtomicOp(CodeGenFunction &CGF, Atomi
   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;
   }

Modified: cfe/trunk/test/CodeGen/atomic-ops.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/atomic-ops.c?rev=203493&r1=203492&r2=203493&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/atomic-ops.c (original)
+++ cfe/trunk/test/CodeGen/atomic-ops.c Mon Mar 10 16:35:33 2014
@@ -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);
 }