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); } _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
