Remove threshold for inserting lifetime markers for named temporaries
Now that TailRecursionElimination has been fixed with r222354, the
threshold on size for lifetime marker insertion can be removed. This
only affects named temporary though, as the patch for unnamed temporaries
is still in progress.
My previous commit (r222993) was not handling debuginfo correctly, but
this could only be seen with some asan tests. Basically, lifetime markers
are just instrumentation for the compiler's usage and should not affect
debug information; however, the cleanup infrastructure was assuming it
contained only destructors, i.e. actual code to be executed, and was
setting the breakpoint for the end of the function to the closing '}', and
not the return statement, in order to show some destructors have been
called when leaving the function. This is wrong when the cleanups are only
lifetime markers, and this is now fixed.
llvm-svn: 234581
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index aa4df0a..a4cfac7 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -30,6 +30,7 @@
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/InlineAsm.h"
#include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/IntrinsicInst.h"
#include "llvm/Transforms/Utils/Local.h"
#include <sstream>
using namespace clang;
@@ -2216,7 +2217,29 @@
if (!CGF.ReturnValue->hasOneUse()) {
llvm::BasicBlock *IP = CGF.Builder.GetInsertBlock();
if (IP->empty()) return nullptr;
- llvm::StoreInst *store = dyn_cast<llvm::StoreInst>(&IP->back());
+ llvm::Instruction *I = &IP->back();
+
+ // Skip lifetime markers
+ for (llvm::BasicBlock::reverse_iterator II = IP->rbegin(),
+ IE = IP->rend();
+ II != IE; ++II) {
+ if (llvm::IntrinsicInst *Intrinsic =
+ dyn_cast<llvm::IntrinsicInst>(&*II)) {
+ if (Intrinsic->getIntrinsicID() == llvm::Intrinsic::lifetime_end) {
+ const llvm::Value *CastAddr = Intrinsic->getArgOperand(1);
+ ++II;
+ if (isa<llvm::BitCastInst>(&*II)) {
+ if (CastAddr == &*II) {
+ continue;
+ }
+ }
+ }
+ }
+ I = &*II;
+ break;
+ }
+
+ llvm::StoreInst *store = dyn_cast<llvm::StoreInst>(I);
if (!store) return nullptr;
if (store->getPointerOperand() != CGF.ReturnValue) return nullptr;
assert(!store->isAtomic() && !store->isVolatile()); // see below
@@ -2314,7 +2337,8 @@
// If there is a dominating store to ReturnValue, we can elide
// the load, zap the store, and usually zap the alloca.
- if (llvm::StoreInst *SI = findDominatingStoreToReturnValue(*this)) {
+ if (llvm::StoreInst *SI =
+ findDominatingStoreToReturnValue(*this)) {
// Reuse the debug location from the store unless there is
// cleanup code to be emitted between the store and return
// instruction.
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index 299969a..d97e405 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -125,6 +125,17 @@
return StartOfData;
}
+bool EHScopeStack::containsOnlyLifetimeMarkers(
+ EHScopeStack::stable_iterator Old) const {
+ for (EHScopeStack::iterator it = begin(); stabilize(it) != Old; it++) {
+ EHCleanupScope *cleanup = dyn_cast<EHCleanupScope>(&*it);
+ if (!cleanup || !cleanup->isLifetimeMarker())
+ return false;
+ }
+
+ return true;
+}
+
EHScopeStack::stable_iterator
EHScopeStack::getInnermostActiveNormalCleanup() const {
for (stable_iterator si = getInnermostNormalCleanup(), se = stable_end();
@@ -748,7 +759,15 @@
Scope.getNumBranchAfters() == 1) {
assert(!BranchThroughDest || !IsActive);
- // TODO: clean up the possibly dead stores to the cleanup dest slot.
+ // Clean up the possibly dead store to the cleanup dest slot.
+ llvm::Instruction *NormalCleanupDestSlot =
+ cast<llvm::Instruction>(getNormalCleanupDestSlot());
+ if (NormalCleanupDestSlot->hasOneUse()) {
+ NormalCleanupDestSlot->user_back()->eraseFromParent();
+ NormalCleanupDestSlot->eraseFromParent();
+ NormalCleanupDest = nullptr;
+ }
+
llvm::BasicBlock *BranchAfter = Scope.getBranchAfterBlock(0);
InstsToAppend.push_back(llvm::BranchInst::Create(BranchAfter));
diff --git a/clang/lib/CodeGen/CGCleanup.h b/clang/lib/CodeGen/CGCleanup.h
index 5f94aec..81c6412 100644
--- a/clang/lib/CodeGen/CGCleanup.h
+++ b/clang/lib/CodeGen/CGCleanup.h
@@ -62,6 +62,9 @@
/// Whether this cleanup is currently active.
unsigned IsActive : 1;
+ /// Whether this cleanup is a lifetime marker
+ unsigned IsLifetimeMarker : 1;
+
/// Whether the normal cleanup should test the activation flag.
unsigned TestFlagInNormalCleanup : 1;
@@ -75,7 +78,7 @@
/// The number of fixups required by enclosing scopes (not including
/// this one). If this is the top cleanup scope, all the fixups
/// from this index onwards belong to this scope.
- unsigned FixupDepth : 32 - 17 - NumCommonBits; // currently 13
+ unsigned FixupDepth : 32 - 18 - NumCommonBits; // currently 13
};
class FilterBitFields {
@@ -272,6 +275,7 @@
CleanupBits.IsNormalCleanup = isNormal;
CleanupBits.IsEHCleanup = isEH;
CleanupBits.IsActive = isActive;
+ CleanupBits.IsLifetimeMarker = false;
CleanupBits.TestFlagInNormalCleanup = false;
CleanupBits.TestFlagInEHCleanup = false;
CleanupBits.CleanupSize = cleanupSize;
@@ -295,6 +299,9 @@
bool isActive() const { return CleanupBits.IsActive; }
void setActive(bool A) { CleanupBits.IsActive = A; }
+ bool isLifetimeMarker() const { return CleanupBits.IsLifetimeMarker; }
+ void setLifetimeMarker() { CleanupBits.IsLifetimeMarker = true; }
+
llvm::AllocaInst *getActiveFlag() const { return ActiveFlag; }
void setActiveFlag(llvm::AllocaInst *Var) { ActiveFlag = Var; }
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index f1ccb09..29c6315 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "CodeGenFunction.h"
+#include "CGCleanup.h"
#include "CGDebugInfo.h"
#include "CGOpenCLRuntime.h"
#include "CodeGenModule.h"
@@ -516,10 +517,7 @@
: Addr(addr), Size(size) {}
void Emit(CodeGenFunction &CGF, Flags flags) override {
- llvm::Value *castAddr = CGF.Builder.CreateBitCast(Addr, CGF.Int8PtrTy);
- CGF.Builder.CreateCall2(CGF.CGM.getLLVMLifetimeEndFn(),
- Size, castAddr)
- ->setDoesNotThrow();
+ CGF.EmitLifetimeEnd(Size, Addr);
}
};
}
@@ -840,21 +838,6 @@
canEmitInitWithFewStoresAfterMemset(Init, StoreBudget);
}
-/// Should we use the LLVM lifetime intrinsics for the given local variable?
-static bool shouldUseLifetimeMarkers(CodeGenFunction &CGF, const VarDecl &D,
- unsigned Size) {
- // For now, only in optimized builds.
- if (CGF.CGM.getCodeGenOpts().OptimizationLevel == 0)
- return false;
-
- // Limit the size of marked objects to 32 bytes. We don't want to increase
- // compile time by marking tiny objects.
- unsigned SizeThreshold = 32;
-
- return Size > SizeThreshold;
-}
-
-
/// EmitAutoVarDecl - Emit code and set up an entry in LocalDeclMap for a
/// variable declaration with auto, register, or no storage class specifier.
/// These turn into simple stack objects, or GlobalValues depending on target.
@@ -864,6 +847,33 @@
EmitAutoVarCleanups(emission);
}
+/// Emit a lifetime.begin marker if some criteria are satisfied.
+/// \return a pointer to the temporary size Value if a marker was emitted, null
+/// otherwise
+llvm::Value *CodeGenFunction::EmitLifetimeStart(uint64_t Size,
+ llvm::Value *Addr) {
+ // For now, only in optimized builds.
+ if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+ return nullptr;
+
+ llvm::Value *SizeV = llvm::ConstantInt::get(Int64Ty, Size);
+ llvm::Value *Args[] = {
+ SizeV,
+ new llvm::BitCastInst(Addr, Int8PtrTy, "", Builder.GetInsertBlock())};
+ llvm::CallInst *C = llvm::CallInst::Create(CGM.getLLVMLifetimeStartFn(), Args,
+ "", Builder.GetInsertBlock());
+ C->setDoesNotThrow();
+ return SizeV;
+}
+
+void CodeGenFunction::EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr) {
+ llvm::Value *Args[] = {Size, new llvm::BitCastInst(Addr, Int8PtrTy, "",
+ Builder.GetInsertBlock())};
+ llvm::CallInst *C = llvm::CallInst::Create(CGM.getLLVMLifetimeEndFn(), Args,
+ "", Builder.GetInsertBlock());
+ C->setDoesNotThrow();
+}
+
/// EmitAutoVarAlloca - Emit the alloca and debug information for a
/// local variable. Does not emit initialization or destruction.
CodeGenFunction::AutoVarEmission
@@ -959,13 +969,8 @@
// Emit a lifetime intrinsic if meaningful. There's no point
// in doing this if we don't have a valid insertion point (?).
uint64_t size = CGM.getDataLayout().getTypeAllocSize(LTy);
- if (HaveInsertPoint() && shouldUseLifetimeMarkers(*this, D, size)) {
- llvm::Value *sizeV = llvm::ConstantInt::get(Int64Ty, size);
-
- emission.SizeForLifetimeMarkers = sizeV;
- llvm::Value *castAddr = Builder.CreateBitCast(Alloc, Int8PtrTy);
- Builder.CreateCall2(CGM.getLLVMLifetimeStartFn(), sizeV, castAddr)
- ->setDoesNotThrow();
+ if (HaveInsertPoint()) {
+ emission.SizeForLifetimeMarkers = EmitLifetimeStart(size, Alloc);
} else {
assert(!emission.useLifetimeMarkers());
}
@@ -1311,6 +1316,8 @@
EHStack.pushCleanup<CallLifetimeEnd>(NormalCleanup,
emission.getAllocatedAddress(),
emission.getSizeForLifetimeMarkers());
+ EHCleanupScope &cleanup = cast<EHCleanupScope>(*EHStack.begin());
+ cleanup.setLifetimeMarker();
}
// Check the type for a cleanup.
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 8401d72..fc4e8c2 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "CodeGenFunction.h"
+#include "CGCleanup.h"
#include "CGCUDARuntime.h"
#include "CGCXXABI.h"
#include "CGDebugInfo.h"
@@ -243,12 +244,13 @@
// parameters. Do this in whatever block we're currently in; it's
// important to do this before we enter the return block or return
// edges will be *really* confused.
- bool EmitRetDbgLoc = true;
- if (EHStack.stable_begin() != PrologueCleanupDepth) {
+ bool HasCleanups = EHStack.stable_begin() != PrologueCleanupDepth;
+ bool HasOnlyLifetimeMarkers =
+ HasCleanups && EHStack.containsOnlyLifetimeMarkers(PrologueCleanupDepth);
+ bool EmitRetDbgLoc = !HasCleanups || HasOnlyLifetimeMarkers;
+ if (HasCleanups) {
// Make sure the line table doesn't jump back into the body for
// the ret after it's been at EndLoc.
- EmitRetDbgLoc = false;
-
if (CGDebugInfo *DI = getDebugInfo())
if (OnlySimpleReturnStmts)
DI->EmitLocation(Builder, EndLoc);
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 7a8f9b2..0ab1e5e 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1741,6 +1741,9 @@
void EmitCXXTemporary(const CXXTemporary *Temporary, QualType TempType,
llvm::Value *Ptr);
+ llvm::Value *EmitLifetimeStart(uint64_t Size, llvm::Value *Addr);
+ void EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr);
+
llvm::Value *EmitCXXNewExpr(const CXXNewExpr *E);
void EmitCXXDeleteExpr(const CXXDeleteExpr *E);
diff --git a/clang/lib/CodeGen/EHScopeStack.h b/clang/lib/CodeGen/EHScopeStack.h
index 363d8b8..a795188 100644
--- a/clang/lib/CodeGen/EHScopeStack.h
+++ b/clang/lib/CodeGen/EHScopeStack.h
@@ -319,6 +319,10 @@
/// Pops a terminate handler off the stack.
void popTerminate();
+ // Returns true iff the current scope is either empty or contains only
+ // lifetime markers, i.e. no real cleanup code
+ bool containsOnlyLifetimeMarkers(stable_iterator Old) const;
+
/// Determines whether the exception-scopes stack is empty.
bool empty() const { return StartOfData == EndOfBuffer; }