[cfe-commits] r114145 - in /cfe/trunk: lib/CodeGen/CGExprCXX.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenFunction.h lib/Sema/SemaTemplateInstantiateDecl.cpp test/CodeGenCXX/exceptions.cpp
John McCall
rjmccall at apple.com
Thu Sep 16 17:50:28 PDT 2010
Author: rjmccall
Date: Thu Sep 16 19:50:28 2010
New Revision: 114145
URL: http://llvm.org/viewvc/llvm-project?rev=114145&view=rev
Log:
When emitting a new-expression inside a conditional expression,
the cleanup might not be dominated by the allocation code.
In this case, we have to store aside all the delete arguments
in case we need them later. There's room for optimization here
in cases where we end up not actually needing the cleanup in
different branches (or being able to pop it after the
initialization code).
Also make sure we only call this operator delete along the path
where we actually allocated something.
Fixes rdar://problem/8439196.
Modified:
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/CodeGenCXX/exceptions.cpp
Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=114145&r1=114144&r2=114145&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Thu Sep 16 19:50:28 2010
@@ -675,6 +675,105 @@
StoreAnyExprIntoOneUnit(CGF, E, NewPtr);
}
+/// A utility class for saving an rvalue.
+class SavedRValue {
+public:
+ enum Kind { ScalarLiteral, ScalarAddress,
+ AggregateLiteral, AggregateAddress,
+ Complex };
+
+private:
+ llvm::Value *Value;
+ Kind K;
+
+ SavedRValue(llvm::Value *V, Kind K) : Value(V), K(K) {}
+
+public:
+ SavedRValue() {}
+
+ static SavedRValue forScalarLiteral(llvm::Value *V) {
+ return SavedRValue(V, ScalarLiteral);
+ }
+
+ static SavedRValue forScalarAddress(llvm::Value *Addr) {
+ return SavedRValue(Addr, ScalarAddress);
+ }
+
+ static SavedRValue forAggregateLiteral(llvm::Value *V) {
+ return SavedRValue(V, AggregateLiteral);
+ }
+
+ static SavedRValue forAggregateAddress(llvm::Value *Addr) {
+ return SavedRValue(Addr, AggregateAddress);
+ }
+
+ static SavedRValue forComplexAddress(llvm::Value *Addr) {
+ return SavedRValue(Addr, Complex);
+ }
+
+ Kind getKind() const { return K; }
+ llvm::Value *getValue() const { return Value; }
+};
+
+/// Given an r-value, perform the code necessary to make sure that a
+/// future RestoreRValue will be able to load the value without
+/// domination concerns.
+static SavedRValue SaveRValue(CodeGenFunction &CGF, RValue RV) {
+ if (RV.isScalar()) {
+ llvm::Value *V = RV.getScalarVal();
+
+ // These automatically dominate and don't need to be saved.
+ if (isa<llvm::Constant>(V) || isa<llvm::AllocaInst>(V))
+ return SavedRValue::forScalarLiteral(V);
+
+ // Everything else needs an alloca.
+ llvm::Value *Addr = CGF.CreateTempAlloca(V->getType(), "saved-rvalue");
+ CGF.Builder.CreateStore(V, Addr);
+ return SavedRValue::forScalarAddress(Addr);
+ }
+
+ if (RV.isComplex()) {
+ CodeGenFunction::ComplexPairTy V = RV.getComplexVal();
+ const llvm::Type *ComplexTy =
+ llvm::StructType::get(CGF.getLLVMContext(),
+ V.first->getType(), V.second->getType(),
+ (void*) 0);
+ llvm::Value *Addr = CGF.CreateTempAlloca(ComplexTy, "saved-complex");
+ CGF.StoreComplexToAddr(V, Addr, /*volatile*/ false);
+ return SavedRValue::forComplexAddress(Addr);
+ }
+
+ assert(RV.isAggregate());
+ llvm::Value *V = RV.getAggregateAddr(); // TODO: volatile?
+ if (isa<llvm::Constant>(V) || isa<llvm::AllocaInst>(V))
+ return SavedRValue::forAggregateLiteral(V);
+
+ llvm::Value *Addr = CGF.CreateTempAlloca(V->getType(), "saved-rvalue");
+ CGF.Builder.CreateStore(V, Addr);
+ return SavedRValue::forAggregateAddress(Addr);
+}
+
+/// Given a saved r-value produced by SaveRValue, perform the code
+/// necessary to restore it to usability at the current insertion
+/// point.
+static RValue RestoreRValue(CodeGenFunction &CGF, SavedRValue RV) {
+ switch (RV.getKind()) {
+ case SavedRValue::ScalarLiteral:
+ return RValue::get(RV.getValue());
+ case SavedRValue::ScalarAddress:
+ return RValue::get(CGF.Builder.CreateLoad(RV.getValue()));
+ case SavedRValue::AggregateLiteral:
+ return RValue::getAggregate(RV.getValue());
+ case SavedRValue::AggregateAddress:
+ return RValue::getAggregate(CGF.Builder.CreateLoad(RV.getValue()));
+ case SavedRValue::Complex:
+ return RValue::getComplex(CGF.LoadComplexFromAddr(RV.getValue(), false));
+ }
+
+ llvm_unreachable("bad saved r-value kind");
+ return RValue();
+}
+
namespace {
/// A cleanup to call the given 'operator delete' function upon
/// abnormal exit from a new expression.
@@ -729,6 +828,104 @@
ReturnValueSlot(), DeleteArgs, OperatorDelete);
}
};
+
+ /// A cleanup to call the given 'operator delete' function upon
+ /// abnormal exit from a new expression when the new expression is
+ /// conditional.
+ class CallDeleteDuringConditionalNew : public EHScopeStack::Cleanup {
+ size_t NumPlacementArgs;
+ const FunctionDecl *OperatorDelete;
+ SavedRValue Ptr;
+ SavedRValue AllocSize;
+
+ SavedRValue *getPlacementArgs() {
+ return reinterpret_cast<SavedRValue*>(this+1);
+ }
+
+ public:
+ static size_t getExtraSize(size_t NumPlacementArgs) {
+ return NumPlacementArgs * sizeof(SavedRValue);
+ }
+
+ CallDeleteDuringConditionalNew(size_t NumPlacementArgs,
+ const FunctionDecl *OperatorDelete,
+ SavedRValue Ptr,
+ SavedRValue AllocSize)
+ : NumPlacementArgs(NumPlacementArgs), OperatorDelete(OperatorDelete),
+ Ptr(Ptr), AllocSize(AllocSize) {}
+
+ void setPlacementArg(unsigned I, SavedRValue Arg) {
+ assert(I < NumPlacementArgs && "index out of range");
+ getPlacementArgs()[I] = Arg;
+ }
+
+ void Emit(CodeGenFunction &CGF, bool IsForEH) {
+ const FunctionProtoType *FPT
+ = OperatorDelete->getType()->getAs<FunctionProtoType>();
+ assert(FPT->getNumArgs() == NumPlacementArgs + 1 ||
+ (FPT->getNumArgs() == 2 && NumPlacementArgs == 0));
+
+ CallArgList DeleteArgs;
+
+ // The first argument is always a void*.
+ FunctionProtoType::arg_type_iterator AI = FPT->arg_type_begin();
+ DeleteArgs.push_back(std::make_pair(RestoreRValue(CGF, Ptr), *AI++));
+
+ // A member 'operator delete' can take an extra 'size_t' argument.
+ if (FPT->getNumArgs() == NumPlacementArgs + 2) {
+ RValue RV = RestoreRValue(CGF, AllocSize);
+ DeleteArgs.push_back(std::make_pair(RV, *AI++));
+ }
+
+ // Pass the rest of the arguments, which must match exactly.
+ for (unsigned I = 0; I != NumPlacementArgs; ++I) {
+ RValue RV = RestoreRValue(CGF, getPlacementArgs()[I]);
+ DeleteArgs.push_back(std::make_pair(RV, *AI++));
+ }
+
+ // Call 'operator delete'.
+ CGF.EmitCall(CGF.CGM.getTypes().getFunctionInfo(DeleteArgs, FPT),
+ CGF.CGM.GetAddrOfFunction(OperatorDelete),
+ ReturnValueSlot(), DeleteArgs, OperatorDelete);
+ }
+ };
+}
+
+/// Enter a cleanup to call 'operator delete' if the initializer in a
+/// new-expression throws.
+static void EnterNewDeleteCleanup(CodeGenFunction &CGF,
+ const CXXNewExpr *E,
+ llvm::Value *NewPtr,
+ llvm::Value *AllocSize,
+ const CallArgList &NewArgs) {
+ // If we're not inside a conditional branch, then the cleanup will
+ // dominate and we can do the easier (and more efficient) thing.
+ if (!CGF.isInConditionalBranch()) {
+ CallDeleteDuringNew *Cleanup = CGF.EHStack
+ .pushCleanupWithExtra<CallDeleteDuringNew>(EHCleanup,
+ E->getNumPlacementArgs(),
+ E->getOperatorDelete(),
+ NewPtr, AllocSize);
+ for (unsigned I = 0, N = E->getNumPlacementArgs(); I != N; ++I)
+ Cleanup->setPlacementArg(I, NewArgs[I+1].first);
+
+ return;
+ }
+
+ // Otherwise, we need to save all this stuff.
+ SavedRValue SavedNewPtr = SaveRValue(CGF, RValue::get(NewPtr));
+ SavedRValue SavedAllocSize = SaveRValue(CGF, RValue::get(AllocSize));
+
+ CallDeleteDuringConditionalNew *Cleanup = CGF.EHStack
+ .pushCleanupWithExtra<CallDeleteDuringConditionalNew>(InactiveEHCleanup,
+ E->getNumPlacementArgs(),
+ E->getOperatorDelete(),
+ SavedNewPtr,
+ SavedAllocSize);
+ for (unsigned I = 0, N = E->getNumPlacementArgs(); I != N; ++I)
+ Cleanup->setPlacementArg(I, SaveRValue(CGF, NewArgs[I+1].first));
+
+ CGF.ActivateCleanupBlock(CGF.EHStack.stable_begin());
}
llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
@@ -827,13 +1024,7 @@
// exception is thrown.
EHScopeStack::stable_iterator CallOperatorDelete;
if (E->getOperatorDelete()) {
- CallDeleteDuringNew *Cleanup = CGF.EHStack
- .pushCleanupWithExtra<CallDeleteDuringNew>(EHCleanup,
- E->getNumPlacementArgs(),
- E->getOperatorDelete(),
- NewPtr, AllocSize);
- for (unsigned I = 0, N = E->getNumPlacementArgs(); I != N; ++I)
- Cleanup->setPlacementArg(I, NewArgs[I+1].first);
+ EnterNewDeleteCleanup(*this, E, NewPtr, AllocSize, NewArgs);
CallOperatorDelete = EHStack.stable_begin();
}
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=114145&r1=114144&r2=114145&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu Sep 16 19:50:28 2010
@@ -1431,9 +1431,11 @@
EHScopeStack::stable_iterator C,
ForActivation_t Kind) {
EHCleanupScope &Scope = cast<EHCleanupScope>(*CGF.EHStack.find(C));
- assert(!Scope.getActiveFlag() && "scope already has activation flag");
- bool NeedFlag = false;
+ // We always need the flag if we're activating the cleanup, because
+ // we have to assume that the current location doesn't necessarily
+ // dominate all future uses of the cleanup.
+ bool NeedFlag = (Kind == ForActivation);
// Calculate whether the cleanup was used:
@@ -1452,16 +1454,17 @@
// If it hasn't yet been used as either, we're done.
if (!NeedFlag) return;
- llvm::AllocaInst *Var = CGF.CreateTempAlloca(CGF.Builder.getInt1Ty());
- Scope.setActiveFlag(Var);
-
- if (Kind == ForActivation) {
- CGF.InitTempAlloca(Var, CGF.Builder.getFalse());
- CGF.Builder.CreateStore(CGF.Builder.getTrue(), Var);
- } else {
- CGF.InitTempAlloca(Var, CGF.Builder.getTrue());
- CGF.Builder.CreateStore(CGF.Builder.getFalse(), Var);
+ llvm::AllocaInst *Var = Scope.getActiveFlag();
+ if (!Var) {
+ Var = CGF.CreateTempAlloca(CGF.Builder.getInt1Ty(), "cleanup.isactive");
+ Scope.setActiveFlag(Var);
+
+ // Initialize to true or false depending on whether it was
+ // active up to this point.
+ CGF.InitTempAlloca(Var, CGF.Builder.getInt1(Kind == ForDeactivation));
}
+
+ CGF.Builder.CreateStore(CGF.Builder.getInt1(Kind == ForActivation), Var);
}
/// Activate a cleanup that was created in an inactivated state.
@@ -1479,7 +1482,7 @@
void CodeGenFunction::DeactivateCleanupBlock(EHScopeStack::stable_iterator C) {
assert(C != EHStack.stable_end() && "deactivating bottom of stack?");
EHCleanupScope &Scope = cast<EHCleanupScope>(*EHStack.find(C));
- assert(Scope.isActive() && "double activation");
+ assert(Scope.isActive() && "double deactivation");
// If it's the top of the stack, just pop it.
if (C == EHStack.stable_begin()) {
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=114145&r1=114144&r2=114145&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Thu Sep 16 19:50:28 2010
@@ -674,6 +674,10 @@
--ConditionalBranchLevel;
}
+ /// isInConditionalBranch - Return true if we're currently emitting
+ /// one branch or the other of a conditional expression.
+ bool isInConditionalBranch() const { return ConditionalBranchLevel != 0; }
+
private:
CGDebugInfo *DebugInfo;
Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=114145&r1=114144&r2=114145&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Thu Sep 16 19:50:28 2010
@@ -1035,7 +1035,8 @@
// Attach the parameters
for (unsigned P = 0; P < Params.size(); ++P)
- Params[P]->setOwningFunction(Function);
+ if (Params[P])
+ Params[P]->setOwningFunction(Function);
Function->setParams(Params.data(), Params.size());
SourceLocation InstantiateAtPOI;
Modified: cfe/trunk/test/CodeGenCXX/exceptions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/exceptions.cpp?rev=114145&r1=114144&r2=114145&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/exceptions.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/exceptions.cpp Thu Sep 16 19:50:28 2010
@@ -164,26 +164,78 @@
namespace test3 {
struct A {
- A(int); A(int, int); ~A();
+ A(int); A(int, int); A(const A&); ~A();
void *p;
- void *operator new(size_t, void*, void*);
- void operator delete(void*, void*, void*);
+ void *operator new(size_t, void*, double);
+ void operator delete(void*, void*, double);
};
+ void *foo();
+ double bar();
+ A makeA(), *makeAPtr();
+
A *a() {
// CHECK: define [[A:%.*]]* @_ZN5test31aEv()
// CHECK: [[FOO:%.*]] = call i8* @_ZN5test33fooEv()
- // CHECK: [[BAR:%.*]] = call i8* @_ZN5test33barEv()
- // CHECK: [[NEW:%.*]] = call i8* @_ZN5test31AnwEmPvS1_(i64 8, i8* [[FOO]], i8* [[BAR]])
+ // CHECK: [[BAR:%.*]] = call double @_ZN5test33barEv()
+ // CHECK: [[NEW:%.*]] = call i8* @_ZN5test31AnwEmPvd(i64 8, i8* [[FOO]], double [[BAR]])
// CHECK-NEXT: [[CAST:%.*]] = bitcast i8* [[NEW]] to [[A]]*
// CHECK-NEXT: invoke void @_ZN5test31AC1Ei([[A]]* [[CAST]], i32 5)
// CHECK: ret [[A]]* [[CAST]]
- // CHECK: invoke void @_ZN5test31AdlEPvS1_S1_(i8* [[NEW]], i8* [[FOO]], i8* [[BAR]])
+ // CHECK: invoke void @_ZN5test31AdlEPvS1_d(i8* [[NEW]], i8* [[FOO]], double [[BAR]])
// CHECK: call void @_ZSt9terminatev()
- extern void *foo(), *bar();
-
return new(foo(),bar()) A(5);
}
+
+ // rdar://problem/8439196
+ A *b(bool cond) {
+
+ // CHECK: define [[A:%.*]]* @_ZN5test31bEb(i1 zeroext
+ // CHECK: [[SAVED0:%.*]] = alloca i8*
+ // CHECK-NEXT: [[SAVED1:%.*]] = alloca i8*
+ // CHECK-NEXT: [[CLEANUPACTIVE:%.*]] = alloca i1
+ // CHECK-NEXT: [[TMP:%.*]] = alloca [[A]], align 8
+ // CHECK: [[TMPACTIVE:%.*]] = alloca i1
+ // CHECK-NEXT: store i1 false, i1* [[TMPACTIVE]]
+ // CHECK-NEXT: store i1 false, i1* [[CLEANUPACTIVE]]
+
+ // CHECK: [[COND:%.*]] = trunc i8 {{.*}} to i1
+ // CHECK-NEXT: br i1 [[COND]]
+ return (cond ?
+
+ // CHECK: [[FOO:%.*]] = call i8* @_ZN5test33fooEv()
+ // CHECK-NEXT: [[NEW:%.*]] = call i8* @_ZN5test31AnwEmPvd(i64 8, i8* [[FOO]], double [[CONST:.*]])
+ // CHECK-NEXT: store i8* [[NEW]], i8** [[SAVED0]]
+ // CHECK-NEXT: store i8* [[FOO]], i8** [[SAVED1]]
+ // CHECK-NEXT: store i1 true, i1* [[CLEANUPACTIVE]]
+ // CHECK-NEXT: [[CAST:%.*]] = bitcast i8* [[NEW]] to [[A]]*
+ // CHECK-NEXT: invoke void @_ZN5test35makeAEv([[A]]* sret [[TMP]])
+ // CHECK: store i1 true, i1* [[TMPACTIVE]]
+ // CHECK-NEXT: invoke void @_ZN5test31AC1ERKS0_([[A]]* [[CAST]], [[A]]* [[TMP]])
+ // CHECK: store i1 false, i1* [[CLEANUPACTIVE]]
+ // CHECK-NEXT: br label
+ // -> cond.end
+ new(foo(),10.0) A(makeA()) :
+
+ // CHECK: [[MAKE:%.*]] = invoke [[A]]* @_ZN5test38makeAPtrEv()
+ // CHECK: br label
+ // -> cond.end
+ makeAPtr());
+
+ // cond.end:
+ // CHECK: [[RESULT:%.*]] = phi [[A]]* {{.*}}[[CAST]]{{.*}}[[MAKE]]
+ // CHECK-NEXT: [[ISACTIVE:%.*]] = load i1* [[TMPACTIVE]]
+ // CHECK-NEXT: br i1 [[ISACTIVE]]
+ // CHECK: invoke void @_ZN5test31AD1Ev
+ // CHECK: ret [[A]]* [[RESULT]]
+
+ // in the EH path:
+ // CHECK: [[ISACTIVE:%.*]] = load i1* [[CLEANUPACTIVE]]
+ // CHECK-NEXT: br i1 [[ISACTIVE]]
+ // CHECK: [[V0:%.*]] = load i8** [[SAVED0]]
+ // CHECK-NEXT: [[V1:%.*]] = load i8** [[SAVED1]]
+ // CHECK-NEXT: invoke void @_ZN5test31AdlEPvS1_d(i8* [[V0]], i8* [[V1]], double [[CONST]])
+ }
}
namespace test4 {
@@ -206,5 +258,4 @@
return new(foo(),bar()) A(5);
}
-
}
More information about the cfe-commits
mailing list