From 195337d2e5d4625ae9dc1328c7cdbc7115b0261b Mon Sep 17 00:00:00 2001 From: Daniel Dunbar Date: Tue, 9 Feb 2010 02:48:28 +0000 Subject: [PATCH] IRgen: Add CreateMemTemp, for creating an temporary memory object for a particular type, and flood fill. - CreateMemTemp sets the alignment on the alloca correctly, which fixes a great many places in IRgen where we were doing the wrong thing. - This fixes many many more places than the test case, but my feeling is we need to audit alignment systematically so I'm not inclined to try hard to test the individual fixes in this patch. If this bothers you, patches welcome! PR6240. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@95648 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGBuiltin.cpp | 3 +-- lib/CodeGen/CGCall.cpp | 21 ++++++++++----------- lib/CodeGen/CGDecl.cpp | 3 +-- lib/CodeGen/CGExpr.cpp | 26 +++++++++++++++----------- lib/CodeGen/CGExprAgg.cpp | 21 ++++++++++----------- lib/CodeGen/CGExprCXX.cpp | 3 +-- lib/CodeGen/CGObjC.cpp | 16 +++++++--------- lib/CodeGen/CodeGenFunction.h | 7 ++++++- test/CodeGen/address-space-field1.c | 4 ++-- test/CodeGenCXX/alloca-align.cpp | 12 ++++++++++++ 10 files changed, 65 insertions(+), 51 deletions(-) create mode 100644 test/CodeGenCXX/alloca-align.cpp diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp index 4ba6a2484b..beaf7b89c0 100644 --- a/lib/CodeGen/CGBuiltin.cpp +++ b/lib/CodeGen/CGBuiltin.cpp @@ -657,8 +657,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD, // Unknown builtin, for now just dump it out and return undef. if (hasAggregateLLVMType(E->getType())) - return RValue::getAggregate(CreateTempAlloca(ConvertTypeForMem( - E->getType()))); + return RValue::getAggregate(CreateMemTemp(E->getType())); return RValue::get(llvm::UndefValue::get(ConvertType(E->getType()))); } diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp index 707513a23d..b064c125ad 100644 --- a/lib/CodeGen/CGCall.cpp +++ b/lib/CodeGen/CGCall.cpp @@ -716,7 +716,7 @@ void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI, // Create a temporary alloca to hold the argument; the rest of // codegen expects to access aggregates & complex values by // reference. - V = CreateTempAlloca(ConvertTypeForMem(Ty)); + V = CreateMemTemp(Ty); Builder.CreateStore(AI, V); } else { if (!getContext().typesAreCompatible(Ty, Arg->getType())) { @@ -733,8 +733,7 @@ void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI, // If this structure was expanded into multiple arguments then // we need to create a temporary and reconstruct it from the // arguments. - llvm::Value *Temp = CreateTempAlloca(ConvertTypeForMem(Ty), - Arg->getName() + ".addr"); + llvm::Value *Temp = CreateMemTemp(Ty, Arg->getName() + ".addr"); // FIXME: What are the right qualifiers here? llvm::Function::arg_iterator End = ExpandTypeFromArgs(Ty, LValue::MakeAddr(Temp, Qualifiers()), AI); @@ -750,7 +749,7 @@ void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI, case ABIArgInfo::Ignore: // Initialize the local variable appropriately. if (hasAggregateLLVMType(Ty)) { - EmitParmDecl(*Arg, CreateTempAlloca(ConvertTypeForMem(Ty))); + EmitParmDecl(*Arg, CreateMemTemp(Ty)); } else { EmitParmDecl(*Arg, llvm::UndefValue::get(ConvertType(Arg->getType()))); } @@ -763,7 +762,7 @@ void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI, // FIXME: This is very wasteful; EmitParmDecl is just going to drop the // result in a new alloca anyway, so we could just store into that // directly if we broke the abstraction down more. - llvm::Value *V = CreateTempAlloca(ConvertTypeForMem(Ty), "coerce"); + llvm::Value *V = CreateMemTemp(Ty, "coerce"); CreateCoercedStore(AI, V, /*DestIsVolatile=*/false, *this); // Match to what EmitParmDecl is expecting for this type. if (!CodeGenFunction::hasAggregateLLVMType(Ty)) { @@ -858,7 +857,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, if (CGM.ReturnTypeUsesSret(CallInfo)) { llvm::Value *Value = ReturnValue.getValue(); if (!Value) - Value = CreateTempAlloca(ConvertTypeForMem(RetTy)); + Value = CreateMemTemp(RetTy); Args.push_back(Value); } @@ -874,7 +873,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, case ABIArgInfo::Indirect: if (RV.isScalar() || RV.isComplex()) { // Make a temporary alloca to pass the argument. - Args.push_back(CreateTempAlloca(ConvertTypeForMem(I->second))); + Args.push_back(CreateMemTemp(I->second)); if (RV.isScalar()) EmitStoreOfScalar(RV.getScalarVal(), Args.back(), false, I->second); else @@ -905,10 +904,10 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, // FIXME: Avoid the conversion through memory if possible. llvm::Value *SrcPtr; if (RV.isScalar()) { - SrcPtr = CreateTempAlloca(ConvertTypeForMem(I->second), "coerce"); + SrcPtr = CreateMemTemp(I->second, "coerce"); EmitStoreOfScalar(RV.getScalarVal(), SrcPtr, false, I->second); } else if (RV.isComplex()) { - SrcPtr = CreateTempAlloca(ConvertTypeForMem(I->second), "coerce"); + SrcPtr = CreateMemTemp(I->second, "coerce"); StoreComplexToAddr(RV.getComplexVal(), SrcPtr, false); } else SrcPtr = RV.getAggregateAddr(); @@ -1013,7 +1012,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, bool DestIsVolatile = ReturnValue.isVolatile(); if (!DestPtr) { - DestPtr = CreateTempAlloca(ConvertTypeForMem(RetTy), "agg.tmp"); + DestPtr = CreateMemTemp(RetTy, "agg.tmp"); DestIsVolatile = false; } Builder.CreateStore(CI, DestPtr, DestIsVolatile); @@ -1031,7 +1030,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, bool DestIsVolatile = ReturnValue.isVolatile(); if (!DestPtr) { - DestPtr = CreateTempAlloca(ConvertTypeForMem(RetTy), "coerce"); + DestPtr = CreateMemTemp(RetTy, "coerce"); DestIsVolatile = false; } diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp index 632dcc1953..793a220506 100644 --- a/lib/CodeGen/CGDecl.cpp +++ b/lib/CodeGen/CGDecl.cpp @@ -709,8 +709,7 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, llvm::Value *Arg) { DeclPtr = Arg; } else { // Otherwise, create a temporary to hold the value. - DeclPtr = CreateTempAlloca(ConvertTypeForMem(Ty)); - DeclPtr->setName(D.getName() + ".addr"); + DeclPtr = CreateMemTemp(Ty, D.getName() + ".addr"); // Store the initial value into the alloca. EmitStoreOfScalar(Arg, DeclPtr, CTy.isVolatileQualified(), Ty); diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index 943e890074..bb74f9b2f5 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -36,6 +36,14 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(const llvm::Type *Ty, return new llvm::AllocaInst(Ty, 0, Name, AllocaInsertPt); } +llvm::Value *CodeGenFunction::CreateMemTemp(QualType Ty, const llvm::Twine &Name) { + llvm::AllocaInst *Alloc = CreateTempAlloca(ConvertTypeForMem(Ty), Name); + // FIXME: Should we prefer the preferred type alignment here? + CharUnits Align = getContext().getTypeAlignInChars(Ty); + Alloc->setAlignment(Align.getQuantity()); + return Alloc; +} + /// EvaluateExprAsBool - Perform the usual unary conversions on the specified /// expression and compare the result against zero, returning an Int1Ty value. llvm::Value *CodeGenFunction::EvaluateExprAsBool(const Expr *E) { @@ -195,8 +203,7 @@ RValue CodeGenFunction::EmitReferenceBindingToExpr(const Expr* E, Val = RValue::get(Val.getAggregateAddr()); } else { // Create a temporary variable that we can bind the reference to. - llvm::Value *Temp = CreateTempAlloca(ConvertTypeForMem(E->getType()), - "reftmp"); + llvm::Value *Temp = CreateMemTemp(E->getType(), "reftmp"); if (Val.isScalar()) EmitStoreOfScalar(Val.getScalarVal(), Temp, false, E->getType()); else @@ -1362,8 +1369,7 @@ EmitExtVectorElementExpr(const ExtVectorElementExpr *E) { llvm::Value *Vec = EmitScalarExpr(E->getBase()); // Store the vector to memory (because LValue wants an address). - llvm::Value *VecMem = CreateTempAlloca(ConvertTypeForMem( - E->getBase()->getType())); + llvm::Value *VecMem = CreateMemTemp(E->getBase()->getType()); Builder.CreateStore(Vec, VecMem); Base = LValue::MakeAddr(VecMem, Qualifiers()); } @@ -1558,6 +1564,7 @@ CodeGenFunction::EmitConditionalOperatorLValue(const ConditionalOperator* E) { if (!LHS.isSimple()) return EmitUnsupportedLValue(E, "conditional operator"); + // FIXME: We shouldn't need an alloca for this. llvm::Value *Temp = CreateTempAlloca(LHS.getAddress()->getType(),"condtmp"); Builder.CreateStore(LHS.getAddress(), Temp); EmitBranch(ContBlock); @@ -1667,12 +1674,9 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) { LValue CodeGenFunction::EmitNullInitializationLValue( const CXXZeroInitValueExpr *E) { QualType Ty = E->getType(); - llvm::AllocaInst *Alloc = CreateTempAlloca(ConvertTypeForMem(Ty)); - CharUnits Align = getContext().getTypeAlignInChars(Ty); - Alloc->setAlignment(Align.getQuantity()); - LValue lvalue = LValue::MakeAddr(Alloc, Qualifiers()); - EmitMemSetToZero(lvalue.getAddress(), Ty); - return lvalue; + LValue LV = LValue::MakeAddr(CreateMemTemp(Ty), MakeQualifiers(Ty)); + EmitMemSetToZero(LV.getAddress(), Ty); + return LV; } //===--------------------------------------------------------------------===// @@ -1767,7 +1771,7 @@ LValue CodeGenFunction::EmitVAArgExprLValue(const VAArgExpr *E) { } LValue CodeGenFunction::EmitCXXConstructLValue(const CXXConstructExpr *E) { - llvm::Value *Temp = CreateTempAlloca(ConvertTypeForMem(E->getType()), "tmp"); + llvm::Value *Temp = CreateMemTemp(E->getType(), "tmp"); EmitCXXConstructExpr(Temp, E); return LValue::MakeAddr(Temp, MakeQualifiers(E->getType())); } diff --git a/lib/CodeGen/CGExprAgg.cpp b/lib/CodeGen/CGExprAgg.cpp index b6340dcbb9..97455c7b13 100644 --- a/lib/CodeGen/CGExprAgg.cpp +++ b/lib/CodeGen/CGExprAgg.cpp @@ -147,7 +147,7 @@ void AggExprEmitter::EmitFinalDestCopy(const Expr *E, RValue Src, bool Ignore) { return; // If the source is volatile, we must read from it; to do that, we need // some place to put it. - DestPtr = CGF.CreateTempAlloca(CGF.ConvertType(E->getType()), "agg.tmp"); + DestPtr = CGF.CreateMemTemp(E->getType(), "agg.tmp"); } if (RequiresGCollection) { @@ -228,8 +228,7 @@ void AggExprEmitter::VisitCastExpr(CastExpr *E) { case CastExpr::CK_BaseToDerivedMemberPointer: { QualType SrcType = E->getSubExpr()->getType(); - llvm::Value *Src = CGF.CreateTempAlloca(CGF.ConvertTypeForMem(SrcType), - "tmp"); + llvm::Value *Src = CGF.CreateMemTemp(SrcType, "tmp"); CGF.EmitAggExpr(E->getSubExpr(), Src, SrcType.isVolatileQualified()); llvm::Value *SrcPtr = Builder.CreateStructGEP(Src, 0, "src.ptr"); @@ -380,14 +379,14 @@ void AggExprEmitter::VisitBinAssign(const BinaryOperator *E) { if (LHS.isPropertyRef()) { llvm::Value *AggLoc = DestPtr; if (!AggLoc) - AggLoc = CGF.CreateTempAlloca(CGF.ConvertType(E->getRHS()->getType())); + AggLoc = CGF.CreateMemTemp(E->getRHS()->getType()); CGF.EmitAggExpr(E->getRHS(), AggLoc, VolatileDest); CGF.EmitObjCPropertySet(LHS.getPropertyRefExpr(), RValue::getAggregate(AggLoc, VolatileDest)); } else if (LHS.isKVCRef()) { llvm::Value *AggLoc = DestPtr; if (!AggLoc) - AggLoc = CGF.CreateTempAlloca(CGF.ConvertType(E->getRHS()->getType())); + AggLoc = CGF.CreateMemTemp(E->getRHS()->getType()); CGF.EmitAggExpr(E->getRHS(), AggLoc, VolatileDest); CGF.EmitObjCPropertySet(LHS.getKVCRefExpr(), RValue::getAggregate(AggLoc, VolatileDest)); @@ -458,7 +457,7 @@ void AggExprEmitter::VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) { if (!Val) { // Create a temporary variable. - Val = CGF.CreateTempAlloca(CGF.ConvertTypeForMem(E->getType()), "tmp"); + Val = CGF.CreateMemTemp(E->getType(), "tmp"); // FIXME: volatile CGF.EmitAggExpr(E->getSubExpr(), Val, false); @@ -476,7 +475,7 @@ AggExprEmitter::VisitCXXConstructExpr(const CXXConstructExpr *E) { if (!Val) { // Create a temporary variable. - Val = CGF.CreateTempAlloca(CGF.ConvertTypeForMem(E->getType()), "tmp"); + Val = CGF.CreateMemTemp(E->getType(), "tmp"); } if (E->requiresZeroInitialization()) @@ -493,7 +492,7 @@ void AggExprEmitter::VisitCXXExprWithTemporaries(CXXExprWithTemporaries *E) { if (!Val) { // Create a temporary variable. - Val = CGF.CreateTempAlloca(CGF.ConvertTypeForMem(E->getType()), "tmp"); + Val = CGF.CreateMemTemp(E->getType(), "tmp"); } CGF.EmitCXXExprWithTemporaries(E, Val, VolatileDest, IsInitializer); } @@ -503,7 +502,7 @@ void AggExprEmitter::VisitCXXZeroInitValueExpr(CXXZeroInitValueExpr *E) { if (!Val) { // Create a temporary variable. - Val = CGF.CreateTempAlloca(CGF.ConvertTypeForMem(E->getType()), "tmp"); + Val = CGF.CreateMemTemp(E->getType(), "tmp"); } LValue LV = LValue::MakeAddr(Val, Qualifiers()); EmitNullInitializationToLValue(LV, E->getType()); @@ -514,7 +513,7 @@ void AggExprEmitter::VisitImplicitValueInitExpr(ImplicitValueInitExpr *E) { if (!Val) { // Create a temporary variable. - Val = CGF.CreateTempAlloca(CGF.ConvertTypeForMem(E->getType()), "tmp"); + Val = CGF.CreateMemTemp(E->getType(), "tmp"); } LValue LV = LValue::MakeAddr(Val, Qualifiers()); EmitNullInitializationToLValue(LV, E->getType()); @@ -708,7 +707,7 @@ void CodeGenFunction::EmitAggExpr(const Expr *E, llvm::Value *DestPtr, LValue CodeGenFunction::EmitAggExprToLValue(const Expr *E) { assert(hasAggregateLLVMType(E->getType()) && "Invalid argument!"); Qualifiers Q = MakeQualifiers(E->getType()); - llvm::Value *Temp = CreateTempAlloca(ConvertTypeForMem(E->getType())); + llvm::Value *Temp = CreateMemTemp(E->getType()); EmitAggExpr(E, Temp, Q.hasVolatile()); return LValue::MakeAddr(Temp, Q); } diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp index 7af90b7a46..0328621604 100644 --- a/lib/CodeGen/CGExprCXX.cpp +++ b/lib/CodeGen/CGExprCXX.cpp @@ -164,8 +164,7 @@ CodeGenFunction::EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E, const llvm::Type *Int8PtrTy = llvm::Type::getInt8PtrTy(VMContext); // Get the member function pointer. - llvm::Value *MemFnPtr = - CreateTempAlloca(ConvertTypeForMem(MemFnExpr->getType()), "mem.fn"); + llvm::Value *MemFnPtr = CreateMemTemp(MemFnExpr->getType(), "mem.fn"); EmitAggExpr(MemFnExpr, MemFnPtr, /*VolatileDest=*/false); // Emit the 'this' pointer. diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp index fedf16cbd9..595f8f6037 100644 --- a/lib/CodeGen/CGObjC.cpp +++ b/lib/CodeGen/CGObjC.cpp @@ -452,9 +452,7 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){ // Fast enumeration state. QualType StateTy = getContext().getObjCFastEnumerationStateType(); - llvm::AllocaInst *StatePtr = CreateTempAlloca(ConvertTypeForMem( - StateTy), "state.ptr"); - StatePtr->setAlignment(getContext().getTypeAlign(StateTy) >> 3); + llvm::Value *StatePtr = CreateMemTemp(StateTy, "state.ptr"); EmitMemSetToZero(StatePtr, StateTy); // Number of elements in the items array. @@ -472,8 +470,7 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){ getContext().getConstantArrayType(getContext().getObjCIdType(), llvm::APInt(32, NumItems), ArrayType::Normal, 0); - llvm::Value *ItemsPtr = CreateTempAlloca(ConvertTypeForMem( - ItemsTy), "items.ptr"); + llvm::Value *ItemsPtr = CreateMemTemp(ItemsTy, "items.ptr"); llvm::Value *Collection = EmitScalarExpr(S.getCollection()); @@ -495,7 +492,8 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){ FastEnumSel, Collection, false, Args); - llvm::Value *LimitPtr = CreateTempAlloca(UnsignedLongLTy, "limit.ptr"); + llvm::Value *LimitPtr = CreateMemTemp(getContext().UnsignedLongTy, + "limit.ptr"); Builder.CreateStore(CountRV.getScalarVal(), LimitPtr); llvm::BasicBlock *NoElements = createBasicBlock("noelements"); @@ -509,8 +507,7 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){ EmitBlock(SetStartMutations); - llvm::Value *StartMutationsPtr = - CreateTempAlloca(UnsignedLongLTy); + llvm::Value *StartMutationsPtr = CreateMemTemp(getContext().UnsignedLongTy); llvm::Value *StateMutationsPtrPtr = Builder.CreateStructGEP(StatePtr, 2, "mutationsptr.ptr"); @@ -525,7 +522,8 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){ llvm::BasicBlock *LoopStart = createBasicBlock("loopstart"); EmitBlock(LoopStart); - llvm::Value *CounterPtr = CreateTempAlloca(UnsignedLongLTy, "counter.ptr"); + llvm::Value *CounterPtr = CreateMemTemp(getContext().UnsignedLongTy, + "counter.ptr"); Builder.CreateStore(Zero, CounterPtr); llvm::BasicBlock *LoopBody = createBasicBlock("loopbody"); diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index 64274d16d8..0e601e5d28 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -655,10 +655,15 @@ public: } /// CreateTempAlloca - This creates a alloca and inserts it into the entry - /// block. + /// block. The caller is responsible for setting an appropriate alignment on + /// the alloca. llvm::AllocaInst *CreateTempAlloca(const llvm::Type *Ty, const llvm::Twine &Name = "tmp"); + /// CreateMemTemp - Create a temporary memory object of the given type, with + /// appropriate alignment. + llvm::Value *CreateMemTemp(QualType T, const llvm::Twine &Name = "tmp"); + /// EvaluateExprAsBool - Perform the usual unary conversions on the specified /// expression and compare the result against zero, returning an Int1Ty value. llvm::Value *EvaluateExprAsBool(const Expr *E); diff --git a/test/CodeGen/address-space-field1.c b/test/CodeGen/address-space-field1.c index 61d88f9e75..a81e08ebca 100644 --- a/test/CodeGen/address-space-field1.c +++ b/test/CodeGen/address-space-field1.c @@ -1,8 +1,8 @@ // RUN: %clang_cc1 -emit-llvm < %s -o - | FileCheck %s // CHECK:%struct.S = type { i32, i32 } // CHECK:define void @test_addrspace(%struct.S addrspace(1)* %p1, %struct.S addrspace(2)* %p2) nounwind -// CHECK: [[p1addr:%.*]] = alloca %struct.S addrspace(1)* ; <%struct.S addrspace(1)**> [#uses=3] -// CHECK: [[p2addr:%.*]] = alloca %struct.S addrspace(2)* ; <%struct.S addrspace(2)**> [#uses=3] +// CHECK: [[p1addr:%.*]] = alloca %struct.S addrspace(1)* +// CHECK: [[p2addr:%.*]] = alloca %struct.S addrspace(2)* // CHECK: store %struct.S addrspace(1)* %p1, %struct.S addrspace(1)** [[p1addr]] // CHECK: store %struct.S addrspace(2)* %p2, %struct.S addrspace(2)** [[p2addr]] // CHECK: [[t0:%.*]] = load %struct.S addrspace(2)** [[p2addr]] ; <%struct.S addrspace(2)*> [#uses=1] diff --git a/test/CodeGenCXX/alloca-align.cpp b/test/CodeGenCXX/alloca-align.cpp new file mode 100644 index 0000000000..de6b34d060 --- /dev/null +++ b/test/CodeGenCXX/alloca-align.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s +// +// CHECK: alloca %struct.MemsetRange, align 16 + +struct MemsetRange { + int Start, End; + unsigned Alignment; + int TheStores __attribute__((aligned(16))); +}; +void foobar() { + (void) MemsetRange(); +} -- 2.40.0