From 6bde954f4702b4075a97a59e148bb22ef845e342 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 26 Oct 2010 22:09:15 +0000 Subject: [PATCH] Extract procedures to do scalar-to-memory and memory-to-scalar conversions in IR gen, and use those to fix a correctness issue with bool atomic intrinsics. rdar://problem/8461234 llvm-svn: 117403 --- clang/lib/CodeGen/CGBuiltin.cpp | 167 ++++++++++++++++------------ clang/lib/CodeGen/CGExpr.cpp | 36 +++--- clang/lib/CodeGen/CodeGenFunction.h | 8 ++ clang/test/CodeGen/atomic.c | 3 +- 4 files changed, 128 insertions(+), 86 deletions(-) diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index e1510011c772..e6e29929fd74 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -41,29 +41,28 @@ static void EmitMemoryBarrier(CodeGenFunction &CGF, C, C + 5); } -static Value *EmitCastToInt(CodeGenFunction &CGF, - const llvm::Type *ToType, Value *Val) { - if (Val->getType()->isPointerTy()) - return CGF.Builder.CreatePtrToInt(Val, ToType); +/// Emit the conversions required to turn the given value into an +/// integer of the given size. +static Value *EmitToInt(CodeGenFunction &CGF, llvm::Value *V, + QualType T, const llvm::IntegerType *IntType) { + V = CGF.EmitToMemory(V, T); - assert(Val->getType()->isIntegerTy() && - "Used a non-integer and non-pointer type with atomic builtin"); - assert(Val->getType()->getScalarSizeInBits() <= - ToType->getScalarSizeInBits() && "Integer type too small"); - return CGF.Builder.CreateSExtOrBitCast(Val, ToType); + if (V->getType()->isPointerTy()) + return CGF.Builder.CreatePtrToInt(V, IntType); + + assert(V->getType() == IntType); + return V; } -static Value *EmitCastFromInt(CodeGenFunction &CGF, QualType ToQualType, - Value *Val) { - const llvm::Type *ToType = CGF.ConvertType(ToQualType); - if (ToType->isPointerTy()) { - return CGF.Builder.CreateIntToPtr(Val, ToType); - } - assert(Val->getType()->isIntegerTy() && - "Used a non-integer and non-pointer type with atomic builtin"); - assert(Val->getType()->getScalarSizeInBits() >= - ToType->getScalarSizeInBits() && "Integer type too small"); - return CGF.Builder.CreateTruncOrBitCast(Val, ToType); +static Value *EmitFromInt(CodeGenFunction &CGF, llvm::Value *V, + QualType T, const llvm::Type *ResultType) { + V = CGF.EmitFromMemory(V, T); + + if (ResultType->isPointerTy()) + return CGF.Builder.CreateIntToPtr(V, ResultType); + + assert(V->getType() == ResultType); + return V; } // The atomic builtins are also full memory barriers. This is a utility for @@ -85,48 +84,69 @@ static Value *EmitCallWithBarrier(CodeGenFunction &CGF, Value *Fn, /// and the expression node. static RValue EmitBinaryAtomic(CodeGenFunction &CGF, Intrinsic::ID Id, const CallExpr *E) { + QualType T = E->getType(); + assert(E->getArg(0)->getType()->isPointerType()); + assert(CGF.getContext().hasSameUnqualifiedType(T, + E->getArg(0)->getType()->getPointeeType())); + assert(CGF.getContext().hasSameUnqualifiedType(T, E->getArg(1)->getType())); + llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0)); unsigned AddrSpace = cast(DestPtr->getType())->getAddressSpace(); - const llvm::Type *ValueType = - llvm::IntegerType::get(CGF.getLLVMContext(), - CGF.getContext().getTypeSize(E->getType())); - const llvm::Type *PtrType = ValueType->getPointerTo(AddrSpace); - const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType }; - Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2); - Value *Args[2] = { CGF.Builder.CreateBitCast(DestPtr, PtrType), - EmitCastToInt(CGF, ValueType, - CGF.EmitScalarExpr(E->getArg(1))) }; - return RValue::get(EmitCastFromInt(CGF, E->getType(), - EmitCallWithBarrier(CGF, AtomF, Args, - Args + 2))); + const llvm::IntegerType *IntType = + llvm::IntegerType::get(CGF.getLLVMContext(), + CGF.getContext().getTypeSize(T)); + const llvm::Type *IntPtrType = IntType->getPointerTo(AddrSpace); + + const llvm::Type *IntrinsicTypes[2] = { IntType, IntPtrType }; + llvm::Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2); + + llvm::Value *Args[2]; + Args[0] = CGF.Builder.CreateBitCast(DestPtr, IntPtrType); + Args[1] = CGF.EmitScalarExpr(E->getArg(1)); + const llvm::Type *ValueType = Args[1]->getType(); + Args[1] = EmitToInt(CGF, Args[1], T, IntType); + + llvm::Value *Result = EmitCallWithBarrier(CGF, AtomF, Args, Args + 2); + Result = EmitFromInt(CGF, Result, T, ValueType); + return RValue::get(Result); } /// Utility to insert an atomic instruction based Instrinsic::ID and -// the expression node, where the return value is the result of the -// operation. +/// the expression node, where the return value is the result of the +/// operation. static RValue EmitBinaryAtomicPost(CodeGenFunction &CGF, Intrinsic::ID Id, const CallExpr *E, Instruction::BinaryOps Op) { + QualType T = E->getType(); + assert(E->getArg(0)->getType()->isPointerType()); + assert(CGF.getContext().hasSameUnqualifiedType(T, + E->getArg(0)->getType()->getPointeeType())); + assert(CGF.getContext().hasSameUnqualifiedType(T, E->getArg(1)->getType())); + llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0)); unsigned AddrSpace = cast(DestPtr->getType())->getAddressSpace(); - - const llvm::Type *ValueType = - llvm::IntegerType::get(CGF.getLLVMContext(), - CGF.getContext().getTypeSize(E->getType())); - const llvm::Type *PtrType = ValueType->getPointerTo(AddrSpace); - const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType }; - Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2); - Value *Args[2] = { CGF.Builder.CreateBitCast(DestPtr, PtrType), - EmitCastToInt(CGF, ValueType, - CGF.EmitScalarExpr(E->getArg(1))) }; - Value *Result = EmitCallWithBarrier(CGF, AtomF, Args, Args + 2); - return RValue::get(EmitCastFromInt(CGF, E->getType(), - CGF.Builder.CreateBinOp(Op, Result, - Args[1]))); + const llvm::IntegerType *IntType = + llvm::IntegerType::get(CGF.getLLVMContext(), + CGF.getContext().getTypeSize(T)); + const llvm::Type *IntPtrType = IntType->getPointerTo(AddrSpace); + + const llvm::Type *IntrinsicTypes[2] = { IntType, IntPtrType }; + llvm::Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2); + + llvm::Value *Args[2]; + Args[1] = CGF.EmitScalarExpr(E->getArg(1)); + const llvm::Type *ValueType = Args[1]->getType(); + Args[1] = EmitToInt(CGF, Args[1], T, IntType); + Args[0] = CGF.Builder.CreateBitCast(DestPtr, IntPtrType); + + llvm::Value *Result = EmitCallWithBarrier(CGF, AtomF, Args, Args + 2); + Result = CGF.Builder.CreateBinOp(Op, Result, Args[1]); + Result = EmitFromInt(CGF, Result, T, ValueType); + return RValue::get(Result); } /// EmitFAbs - Emit a call to fabs/fabsf/fabsl, depending on the type of ValTy, @@ -793,25 +813,29 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD, case Builtin::BI__sync_val_compare_and_swap_4: case Builtin::BI__sync_val_compare_and_swap_8: case Builtin::BI__sync_val_compare_and_swap_16: { + QualType T = E->getType(); llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0)); unsigned AddrSpace = cast(DestPtr->getType())->getAddressSpace(); - const llvm::Type *ValueType = + + const llvm::IntegerType *IntType = llvm::IntegerType::get(CGF.getLLVMContext(), - CGF.getContext().getTypeSize(E->getType())); - const llvm::Type *PtrType = ValueType->getPointerTo(AddrSpace); - const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType }; + CGF.getContext().getTypeSize(T)); + const llvm::Type *IntPtrType = IntType->getPointerTo(AddrSpace); + const llvm::Type *IntrinsicTypes[2] = { IntType, IntPtrType }; Value *AtomF = CGM.getIntrinsic(Intrinsic::atomic_cmp_swap, IntrinsicTypes, 2); - Value *Args[3] = { Builder.CreateBitCast(DestPtr, PtrType), - EmitCastToInt(CGF, ValueType, - CGF.EmitScalarExpr(E->getArg(1))), - EmitCastToInt(CGF, ValueType, - CGF.EmitScalarExpr(E->getArg(2))) }; - return RValue::get(EmitCastFromInt(CGF, E->getType(), - EmitCallWithBarrier(CGF, AtomF, Args, - Args + 3))); + Value *Args[3]; + Args[0] = Builder.CreateBitCast(DestPtr, IntPtrType); + Args[1] = CGF.EmitScalarExpr(E->getArg(1)); + const llvm::Type *ValueType = Args[1]->getType(); + Args[1] = EmitToInt(CGF, Args[1], T, IntType); + Args[2] = EmitToInt(CGF, CGF.EmitScalarExpr(E->getArg(2)), T, IntType); + + Value *Result = EmitCallWithBarrier(CGF, AtomF, Args, Args + 3); + Result = EmitFromInt(CGF, Result, T, ValueType); + return RValue::get(Result); } case Builtin::BI__sync_bool_compare_and_swap_1: @@ -819,27 +843,30 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD, case Builtin::BI__sync_bool_compare_and_swap_4: case Builtin::BI__sync_bool_compare_and_swap_8: case Builtin::BI__sync_bool_compare_and_swap_16: { + QualType T = E->getArg(1)->getType(); llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0)); unsigned AddrSpace = cast(DestPtr->getType())->getAddressSpace(); - const llvm::Type *ValueType = + + const llvm::IntegerType *IntType = llvm::IntegerType::get(CGF.getLLVMContext(), - CGF.getContext().getTypeSize(E->getArg(1)->getType())); - const llvm::Type *PtrType = ValueType->getPointerTo(AddrSpace); - const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType }; + CGF.getContext().getTypeSize(T)); + const llvm::Type *IntPtrType = IntType->getPointerTo(AddrSpace); + const llvm::Type *IntrinsicTypes[2] = { IntType, IntPtrType }; Value *AtomF = CGM.getIntrinsic(Intrinsic::atomic_cmp_swap, IntrinsicTypes, 2); - Value *Args[3] = { Builder.CreateBitCast(DestPtr, PtrType), - EmitCastToInt(CGF, ValueType, - CGF.EmitScalarExpr(E->getArg(1))), - EmitCastToInt(CGF, ValueType, - CGF.EmitScalarExpr(E->getArg(2))) }; + Value *Args[3]; + Args[0] = Builder.CreateBitCast(DestPtr, IntPtrType); + Args[1] = EmitToInt(CGF, CGF.EmitScalarExpr(E->getArg(1)), T, IntType); + Args[2] = EmitToInt(CGF, CGF.EmitScalarExpr(E->getArg(2)), T, IntType); + Value *OldVal = Args[1]; Value *PrevVal = EmitCallWithBarrier(*this, AtomF, Args, Args + 3); Value *Result = Builder.CreateICmpEQ(PrevVal, OldVal); // zext bool to int. - return RValue::get(Builder.CreateZExt(Result, ConvertType(E->getType()))); + Result = Builder.CreateZExt(Result, ConvertType(E->getType())); + return RValue::get(Result); } case Builtin::BI__sync_lock_test_and_set_1: diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 7bc8159a3d39..85cd0c0c30b7 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -590,13 +590,7 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(llvm::Value *Addr, bool Volatile, if (TBAAInfo) CGM.DecorateInstruction(Load, TBAAInfo); - // Bool can have different representation in memory than in registers. - llvm::Value *V = Load; - if (Ty->isBooleanType()) - if (V->getType() != llvm::Type::getInt1Ty(VMContext)) - V = Builder.CreateTrunc(V, llvm::Type::getInt1Ty(VMContext), "tobool"); - - return V; + return EmitFromMemory(Load, Ty); } static bool isBooleanUnderlyingType(QualType Ty) { @@ -605,17 +599,31 @@ static bool isBooleanUnderlyingType(QualType Ty) { return false; } +llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value *Value, QualType Ty) { + // Bool has a different representation in memory than in registers. + if (Ty->isBooleanType() || isBooleanUnderlyingType(Ty)) { + assert(Value->getType()->isIntegerTy(1) && "value rep of bool not i1"); + return Builder.CreateZExt(Value, Builder.getInt8Ty(), "frombool"); + } + + return Value; +} + +llvm::Value *CodeGenFunction::EmitFromMemory(llvm::Value *Value, QualType Ty) { + // Bool has a different representation in memory than in registers. + if (Ty->isBooleanType() || isBooleanUnderlyingType(Ty)) { + assert(Value->getType()->isIntegerTy(8) && "memory rep of bool not i8"); + return Builder.CreateTrunc(Value, Builder.getInt1Ty(), "tobool"); + } + + return Value; +} + void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, llvm::Value *Addr, bool Volatile, unsigned Alignment, QualType Ty, llvm::MDNode *TBAAInfo) { - - if (Ty->isBooleanType() || isBooleanUnderlyingType(Ty)) { - // Bool can have different representation in memory than in registers. - const llvm::PointerType *DstPtr = cast(Addr->getType()); - Value = Builder.CreateIntCast(Value, DstPtr->getElementType(), false); - } - + Value = EmitToMemory(Value, Ty); llvm::StoreInst *Store = Builder.CreateStore(Value, Addr, Volatile); if (Alignment) Store->setAlignment(Alignment); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 53056bc60980..09b7168a9ea5 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1349,6 +1349,14 @@ public: /// object. LValue EmitCheckedLValue(const Expr *E); + /// EmitToMemory - Change a scalar value from its value + /// representation to its in-memory representation. + llvm::Value *EmitToMemory(llvm::Value *Value, QualType Ty); + + /// EmitFromMemory - Change a scalar value from its memory + /// representation to its value representation. + llvm::Value *EmitFromMemory(llvm::Value *Value, QualType Ty); + /// EmitLoadOfScalar - Load a scalar value from an address, taking /// care to appropriately convert from the memory representation to /// the LLVM value representation. diff --git a/clang/test/CodeGen/atomic.c b/clang/test/CodeGen/atomic.c index e3bc2185b750..4a7c13f03c41 100644 --- a/clang/test/CodeGen/atomic.c +++ b/clang/test/CodeGen/atomic.c @@ -102,8 +102,7 @@ int atomic(void) { if ( __sync_val_compare_and_swap(&valb, 0, 1)) { // CHECK: call void @llvm.memory.barrier(i1 true, i1 true, i1 true, i1 true, i1 true) -// FIXME: Doesn't seem right! - // CHECK: call i8 @llvm.atomic.cmp.swap.i8.p0i8(i8* %valb, i8 0, i8 -1) + // CHECK: call i8 @llvm.atomic.cmp.swap.i8.p0i8(i8* %valb, i8 0, i8 1) // CHECK: call void @llvm.memory.barrier(i1 true, i1 true, i1 true, i1 true, i1 true) old = 42; }