Skip to content

Commit

Permalink
CodeGen: Fix invalid bitcast for lifetime.start/end
Browse files Browse the repository at this point in the history 
lifetime.start/end expects pointer argument in alloca address space.
However in C++ a temporary variable is in default address space.

This patch changes API CreateMemTemp and CreateTempAlloca to
get the original alloca instruction and pass it lifetime.start/end.

It only affects targets with non-zero alloca address space.

Differential Revision: https://reviews.llvm.org/D45900

llvm-svn: 332593
  • Loading branch information
@yxsamliu
yxsamliu committed May 17, 2018
1 parent 0e69e2d commit a2a9cfa
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 34 deletions.
24 changes: 15 additions & 9 deletions clang/lib/CodeGen/CGCall.cpp
View file
Expand Up @@ -3812,16 +3812,17 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
// If the call returns a temporary with struct return, create a temporary
// alloca to hold the result, unless one is given to us.
Address SRetPtr = Address::invalid();
Address SRetAlloca = Address::invalid();
llvm::Value *UnusedReturnSizePtr = nullptr;
if (RetAI.isIndirect() || RetAI.isInAlloca() || RetAI.isCoerceAndExpand()) {
if (!ReturnValue.isNull()) {
SRetPtr = ReturnValue.getValue();
} else {
SRetPtr = CreateMemTemp(RetTy);
SRetPtr = CreateMemTemp(RetTy, "tmp", &SRetAlloca);
if (HaveInsertPoint() && ReturnValue.isUnused()) {
uint64_t size =
CGM.getDataLayout().getTypeAllocSize(ConvertTypeForMem(RetTy));
UnusedReturnSizePtr = EmitLifetimeStart(size, SRetPtr.getPointer());
UnusedReturnSizePtr = EmitLifetimeStart(size, SRetAlloca.getPointer());
}
}
if (IRFunctionArgs.hasSRetArg()) {
Expand Down Expand Up @@ -3888,7 +3889,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
if (!I->isAggregate()) {
// Make a temporary alloca to pass the argument.
Address Addr = CreateMemTemp(I->Ty, ArgInfo.getIndirectAlign(),
"indirect-arg-temp", false);
"indirect-arg-temp", /*Alloca=*/nullptr,
/*Cast=*/false);
IRCallArgs[FirstIRArg] = Addr.getPointer();

I->copyInto(*this, Addr);
Expand Down Expand Up @@ -3934,7 +3936,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
if (NeedCopy) {
// Create an aligned temporary, and copy to it.
Address AI = CreateMemTemp(I->Ty, ArgInfo.getIndirectAlign(),
"byval-temp", false);
"byval-temp", /*Alloca=*/nullptr,
/*Cast=*/false);
IRCallArgs[FirstIRArg] = AI.getPointer();
I->copyInto(*this, AI);
} else {
Expand Down Expand Up @@ -4062,6 +4065,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,

llvm::Value *tempSize = nullptr;
Address addr = Address::invalid();
Address AllocaAddr = Address::invalid();
if (I->isAggregate()) {
addr = I->hasLValue() ? I->getKnownLValue().getAddress()
: I->getKnownRValue().getAggregateAddress();
Expand All @@ -4076,9 +4080,11 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,

// Materialize to a temporary.
addr = CreateTempAlloca(RV.getScalarVal()->getType(),
CharUnits::fromQuantity(std::max(layout->getAlignment(),
scalarAlign)));
tempSize = EmitLifetimeStart(scalarSize, addr.getPointer());
CharUnits::fromQuantity(std::max(
layout->getAlignment(), scalarAlign)),
"tmp",
/*ArraySize=*/nullptr, &AllocaAddr);
tempSize = EmitLifetimeStart(scalarSize, AllocaAddr.getPointer());

Builder.CreateStore(RV.getScalarVal(), addr);
}
Expand All @@ -4096,7 +4102,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
assert(IRArgPos == FirstIRArg + NumIRArgs);

if (tempSize) {
EmitLifetimeEnd(tempSize, addr.getPointer());
EmitLifetimeEnd(tempSize, AllocaAddr.getPointer());
}

break;
Expand Down Expand Up @@ -4258,7 +4264,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
// pop this cleanup later on. Being eager about this is OK, since this
// temporary is 'invisible' outside of the callee.
if (UnusedReturnSizePtr)
pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, SRetPtr,
pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, SRetAlloca,
UnusedReturnSizePtr);

llvm::BasicBlock *InvokeDest = CannotThrow ? nullptr : getInvokeDest();
Expand Down
18 changes: 14 additions & 4 deletions clang/lib/CodeGen/CGDecl.cpp
View file
Expand Up @@ -965,6 +965,9 @@ llvm::Value *CodeGenFunction::EmitLifetimeStart(uint64_t Size,
if (!ShouldEmitLifetimeMarkers)
return nullptr;

assert(Addr->getType()->getPointerAddressSpace() ==
CGM.getDataLayout().getAllocaAddrSpace() &&
"Pointer should be in alloca address space");
llvm::Value *SizeV = llvm::ConstantInt::get(Int64Ty, Size);
Addr = Builder.CreateBitCast(Addr, AllocaInt8PtrTy);
llvm::CallInst *C =
Expand All @@ -974,6 +977,9 @@ llvm::Value *CodeGenFunction::EmitLifetimeStart(uint64_t Size,
}

void CodeGenFunction::EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr) {
assert(Addr->getType()->getPointerAddressSpace() ==
CGM.getDataLayout().getAllocaAddrSpace() &&
"Pointer should be in alloca address space");
Addr = Builder.CreateBitCast(Addr, AllocaInt8PtrTy);
llvm::CallInst *C =
Builder.CreateCall(CGM.getLLVMLifetimeEndFn(), {Size, Addr});
Expand Down Expand Up @@ -1058,6 +1064,7 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
codegenoptions::LimitedDebugInfo;

Address address = Address::invalid();
Address AllocaAddr = Address::invalid();
if (Ty->isConstantSizeType()) {
bool NRVO = getLangOpts().ElideConstructors &&
D.isNRVOVariable();
Expand Down Expand Up @@ -1148,7 +1155,8 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
// Create the alloca. Note that we set the name separately from
// building the instruction so that it's there even in no-asserts
// builds.
address = CreateTempAlloca(allocaTy, allocaAlignment, D.getName());
address = CreateTempAlloca(allocaTy, allocaAlignment, D.getName(),
/*ArraySize=*/nullptr, &AllocaAddr);

// Don't emit lifetime markers for MSVC catch parameters. The lifetime of
// the catch parameter starts in the catchpad instruction, and we can't
Expand Down Expand Up @@ -1176,7 +1184,7 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
!(!getLangOpts().CPlusPlus && hasLabelBeenSeenInCurrentScope())) {
uint64_t size = CGM.getDataLayout().getTypeAllocSize(allocaTy);
emission.SizeForLifetimeMarkers =
EmitLifetimeStart(size, address.getPointer());
EmitLifetimeStart(size, AllocaAddr.getPointer());
}
} else {
assert(!emission.useLifetimeMarkers());
Expand Down Expand Up @@ -1205,7 +1213,8 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
llvm::Type *llvmTy = ConvertTypeForMem(VlaSize.Type);

// Allocate memory for the array.
address = CreateTempAlloca(llvmTy, alignment, "vla", VlaSize.NumElts);
address = CreateTempAlloca(llvmTy, alignment, "vla", VlaSize.NumElts,
&AllocaAddr);

// If we have debug info enabled, properly describe the VLA dimensions for
// this type by registering the vla size expression for each of the
Expand All @@ -1215,6 +1224,7 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {

setAddrOfLocalVar(&D, address);
emission.Addr = address;
emission.AllocaAddr = AllocaAddr;

// Emit debug info for local var declaration.
if (EmitDebugInfo && HaveInsertPoint()) {
Expand All @@ -1228,7 +1238,7 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
// Make sure we call @llvm.lifetime.end.
if (emission.useLifetimeMarkers())
EHStack.pushCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker,
emission.getAllocatedAddress(),
emission.getOriginalAllocatedAddress(),
emission.getSizeForLifetimeMarkers());

return emission;
Expand Down
27 changes: 17 additions & 10 deletions clang/lib/CodeGen/CGExpr.cpp
View file
Expand Up @@ -64,9 +64,12 @@ llvm::Value *CodeGenFunction::EmitCastToVoidPtr(llvm::Value *value) {
Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
const Twine &Name,
llvm::Value *ArraySize,
Address *AllocaAddr,
bool CastToDefaultAddrSpace) {
auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
Alloca->setAlignment(Align.getQuantity());
if (AllocaAddr)
*AllocaAddr = Address(Alloca, Align);
llvm::Value *V = Alloca;
// Alloca always returns a pointer in alloca address space, which may
// be different from the type defined by the language. For example,
Expand Down Expand Up @@ -125,16 +128,18 @@ Address CodeGenFunction::CreateIRTemp(QualType Ty, const Twine &Name) {
}

Address CodeGenFunction::CreateMemTemp(QualType Ty, const Twine &Name,
Address *Alloca,
bool CastToDefaultAddrSpace) {
// FIXME: Should we prefer the preferred type alignment here?
return CreateMemTemp(Ty, getContext().getTypeAlignInChars(Ty), Name,
return CreateMemTemp(Ty, getContext().getTypeAlignInChars(Ty), Name, Alloca,
CastToDefaultAddrSpace);
}

Address CodeGenFunction::CreateMemTemp(QualType Ty, CharUnits Align,
const Twine &Name,
const Twine &Name, Address *Alloca,
bool CastToDefaultAddrSpace) {
return CreateTempAlloca(ConvertTypeForMem(Ty), Align, Name, nullptr,
return CreateTempAlloca(ConvertTypeForMem(Ty), Align, Name,
/*ArraySize=*/nullptr, Alloca,
CastToDefaultAddrSpace);
}

Expand Down Expand Up @@ -348,7 +353,8 @@ pushTemporaryCleanup(CodeGenFunction &CGF, const MaterializeTemporaryExpr *M,

static Address createReferenceTemporary(CodeGenFunction &CGF,
const MaterializeTemporaryExpr *M,
const Expr *Inner) {
const Expr *Inner,
Address *Alloca = nullptr) {
auto &TCG = CGF.getTargetHooks();
switch (M->getStorageDuration()) {
case SD_FullExpression:
Expand Down Expand Up @@ -381,7 +387,7 @@ static Address createReferenceTemporary(CodeGenFunction &CGF,
return Address(C, alignment);
}
}
return CGF.CreateMemTemp(Ty, "ref.tmp");
return CGF.CreateMemTemp(Ty, "ref.tmp", Alloca);
}
case SD_Thread:
case SD_Static:
Expand Down Expand Up @@ -458,7 +464,8 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
}

// Create and initialize the reference temporary.
Address Object = createReferenceTemporary(*this, M, E);
Address Alloca = Address::invalid();
Address Object = createReferenceTemporary(*this, M, E, &Alloca);
if (auto *Var = dyn_cast<llvm::GlobalVariable>(
Object.getPointer()->stripPointerCasts())) {
Object = Address(llvm::ConstantExpr::getBitCast(
Expand All @@ -477,13 +484,13 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
case SD_Automatic:
case SD_FullExpression:
if (auto *Size = EmitLifetimeStart(
CGM.getDataLayout().getTypeAllocSize(Object.getElementType()),
Object.getPointer())) {
CGM.getDataLayout().getTypeAllocSize(Alloca.getElementType()),
Alloca.getPointer())) {
if (M->getStorageDuration() == SD_Automatic)
pushCleanupAfterFullExpr<CallLifetimeEnd>(NormalEHLifetimeMarker,
Object, Size);
Alloca, Size);
else
pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, Object,
pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, Alloca,
Size);
}
break;
Expand Down
9 changes: 5 additions & 4 deletions clang/lib/CodeGen/CGExprAgg.cpp
View file
Expand Up @@ -253,17 +253,18 @@ void AggExprEmitter::withReturnValueSlot(
(RequiresDestruction && !Dest.getAddress().isValid());

Address RetAddr = Address::invalid();
Address RetAllocaAddr = Address::invalid();

EHScopeStack::stable_iterator LifetimeEndBlock;
llvm::Value *LifetimeSizePtr = nullptr;
llvm::IntrinsicInst *LifetimeStartInst = nullptr;
if (!UseTemp) {
RetAddr = Dest.getAddress();
} else {
RetAddr = CGF.CreateMemTemp(RetTy);
RetAddr = CGF.CreateMemTemp(RetTy, "tmp", &RetAllocaAddr);
uint64_t Size =
CGF.CGM.getDataLayout().getTypeAllocSize(CGF.ConvertTypeForMem(RetTy));
LifetimeSizePtr = CGF.EmitLifetimeStart(Size, RetAddr.getPointer());
LifetimeSizePtr = CGF.EmitLifetimeStart(Size, RetAllocaAddr.getPointer());
if (LifetimeSizePtr) {
LifetimeStartInst =
cast<llvm::IntrinsicInst>(std::prev(Builder.GetInsertPoint()));
Expand All @@ -272,7 +273,7 @@ void AggExprEmitter::withReturnValueSlot(
"Last insertion wasn't a lifetime.start?");

CGF.pushFullExprCleanup<CodeGenFunction::CallLifetimeEnd>(
NormalEHLifetimeMarker, RetAddr, LifetimeSizePtr);
NormalEHLifetimeMarker, RetAllocaAddr, LifetimeSizePtr);
LifetimeEndBlock = CGF.EHStack.stable_begin();
}
}
Expand All @@ -294,7 +295,7 @@ void AggExprEmitter::withReturnValueSlot(
// Since we're not guaranteed to be in an ExprWithCleanups, clean up
// eagerly.
CGF.DeactivateCleanupBlock(LifetimeEndBlock, LifetimeStartInst);
CGF.EmitLifetimeEnd(LifetimeSizePtr, RetAddr.getPointer());
CGF.EmitLifetimeEnd(LifetimeSizePtr, RetAllocaAddr.getPointer());
}
}

Expand Down
32 changes: 25 additions & 7 deletions clang/lib/CodeGen/CodeGenFunction.h
View file
Expand Up @@ -2023,11 +2023,14 @@ class CodeGenFunction : public CodeGenTypeCache {
/// various ways, this function will perform the cast by default. The cast
/// may be avoided by passing false as \p CastToDefaultAddrSpace; this is
/// more efficient if the caller knows that the address will not be exposed.
/// The original alloca instruction is returned through \p Alloca if it is
/// not nullptr.
llvm::AllocaInst *CreateTempAlloca(llvm::Type *Ty, const Twine &Name = "tmp",
llvm::Value *ArraySize = nullptr);
Address CreateTempAlloca(llvm::Type *Ty, CharUnits align,
const Twine &Name = "tmp",
llvm::Value *ArraySize = nullptr,
Address *Alloca = nullptr,
bool CastToDefaultAddrSpace = true);

/// CreateDefaultAlignedTempAlloca - This creates an alloca with the
Expand Down Expand Up @@ -2064,10 +2067,13 @@ class CodeGenFunction : public CodeGenTypeCache {

/// CreateMemTemp - Create a temporary memory object of the given type, with
/// appropriate alignment. Cast it to the default address space if
/// \p CastToDefaultAddrSpace is true.
/// \p CastToDefaultAddrSpace is true. Returns the original alloca
/// instruction by \p Alloca if it is not nullptr.
Address CreateMemTemp(QualType T, const Twine &Name = "tmp",
Address *Alloca = nullptr,
bool CastToDefaultAddrSpace = true);
Address CreateMemTemp(QualType T, CharUnits Align, const Twine &Name = "tmp",
Address *Alloca = nullptr,
bool CastToDefaultAddrSpace = true);

/// CreateAggTemp - Create a temporary memory object for the given
Expand Down Expand Up @@ -2515,7 +2521,9 @@ class CodeGenFunction : public CodeGenTypeCache {

const VarDecl *Variable;

/// The address of the alloca. Invalid if the variable was emitted
/// The address of the alloca for languages with explicit address space
/// (e.g. OpenCL) or alloca casted to generic pointer for address space
/// agnostic languages (e.g. C++). Invalid if the variable was emitted
/// as a global constant.
Address Addr;

Expand All @@ -2531,13 +2539,19 @@ class CodeGenFunction : public CodeGenTypeCache {
/// Non-null if we should use lifetime annotations.
llvm::Value *SizeForLifetimeMarkers;

/// Address with original alloca instruction. Invalid if the variable was
/// emitted as a global constant.
Address AllocaAddr;

struct Invalid {};
AutoVarEmission(Invalid) : Variable(nullptr), Addr(Address::invalid()) {}
AutoVarEmission(Invalid)
: Variable(nullptr), Addr(Address::invalid()),
AllocaAddr(Address::invalid()) {}

AutoVarEmission(const VarDecl &variable)
: Variable(&variable), Addr(Address::invalid()), NRVOFlag(nullptr),
IsByRef(false), IsConstantAggregate(false),
SizeForLifetimeMarkers(nullptr) {}
: Variable(&variable), Addr(Address::invalid()), NRVOFlag(nullptr),
IsByRef(false), IsConstantAggregate(false),
SizeForLifetimeMarkers(nullptr), AllocaAddr(Address::invalid()) {}

bool wasEmittedAsGlobal() const { return !Addr.isValid(); }

Expand All @@ -2553,11 +2567,15 @@ class CodeGenFunction : public CodeGenTypeCache {
}

/// Returns the raw, allocated address, which is not necessarily
/// the address of the object itself.
/// the address of the object itself. It is casted to default
/// address space for address space agnostic languages.
Address getAllocatedAddress() const {
return Addr;
}

/// Returns the address for the original alloca instruction.
Address getOriginalAllocatedAddress() const { return AllocaAddr; }

/// Returns the address of the object within this declaration.
/// Note that this does not chase the forwarding pointer for
/// __block decls.
Expand Down
27 changes: 27 additions & 0 deletions clang/test/CodeGenCXX/amdgcn_declspec_get.cpp
View file
@@ -0,0 +1,27 @@
// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -O3 -fdeclspec \
// RUN: -disable-llvm-passes -o - %s | FileCheck %s

int get_x();

struct A {
__declspec(property(get = _get_x)) int x;
static int _get_x(void) {
return get_x();
};
};

extern const A a;

// CHECK-LABEL: define void @_Z4testv()
// CHECK: %i = alloca i32, align 4, addrspace(5)
// CHECK: %[[ii:.*]] = addrspacecast i32 addrspace(5)* %i to i32*
// CHECK: %[[cast:.*]] = bitcast i32 addrspace(5)* %i to i8 addrspace(5)*
// CHECK: call void @llvm.lifetime.start.p5i8(i64 4, i8 addrspace(5)* %[[cast]])
// CHECK: %call = call i32 @_ZN1A6_get_xEv()
// CHECK: store i32 %call, i32* %[[ii]]
// CHECK: %[[cast2:.*]] = bitcast i32 addrspace(5)* %i to i8 addrspace(5)*
// CHECK: call void @llvm.lifetime.end.p5i8(i64 4, i8 addrspace(5)* %[[cast2]])
void test()
{
int i = a.x;
}

0 comments on commit a2a9cfa

Please sign in to comment.