[llvm-commits] [llvm] r160529 - in /llvm/trunk: lib/Transforms/IPO/GlobalOpt.cpp test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll
Nick Lewycky
nicholas at mxc.ca
Thu Jul 19 15:35:28 PDT 2012
Author: nicholas
Date: Thu Jul 19 17:35:28 2012
New Revision: 160529
URL: http://llvm.org/viewvc/llvm-project?rev=160529&view=rev
Log:
Don't wipe out global variables that are probably storing pointers to heap
memory. This makes clang play nice with leak checkers.
Added:
llvm/trunk/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll
Modified:
llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
llvm/trunk/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll
Modified: llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=160529&r1=160528&r2=160529&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp Thu Jul 19 17:35:28 2012
@@ -296,6 +296,159 @@
return false;
}
+/// isLeakCheckerRoot - Is this global variable possibly used by a leak checker
+/// as a root? If so, we might not really want to eliminate the stores to it.
+static bool isLeakCheckerRoot(GlobalVariable *GV) {
+ // A global variable is a root if it is a pointer, or could plausibly contain
+ // a pointer. There are two challenges; one is that we could have a struct
+ // the has an inner member which is a pointer. We recurse through the type to
+ // detect these (up to a point). The other is that we may actually be a union
+ // of a pointer and another type, and so our LLVM type is an integer which
+ // gets converted into a pointer, or our type is an [i8 x #] with a pointer
+ // potentially contained here.
+
+ if (GV->hasPrivateLinkage())
+ return false;
+
+ SmallVector<Type *, 4> Types;
+ Types.push_back(cast<PointerType>(GV->getType())->getElementType());
+
+ unsigned Limit = 20;
+ do {
+ Type *Ty = Types.pop_back_val();
+ switch (Ty->getTypeID()) {
+ default: break;
+ case Type::PointerTyID: return true;
+ case Type::ArrayTyID:
+ case Type::VectorTyID: {
+ SequentialType *STy = cast<SequentialType>(Ty);
+ Types.push_back(STy->getElementType());
+ break;
+ }
+ case Type::StructTyID: {
+ StructType *STy = cast<StructType>(Ty);
+ if (STy->isOpaque()) return true;
+ for (StructType::element_iterator I = STy->element_begin(),
+ E = STy->element_end(); I != E; ++I) {
+ Type *InnerTy = *I;
+ if (isa<PointerType>(InnerTy)) return true;
+ if (isa<CompositeType>(InnerTy))
+ Types.push_back(InnerTy);
+ }
+ break;
+ }
+ }
+ if (--Limit == 0) return true;
+ } while (!Types.empty());
+ return false;
+}
+
+/// Given a value that is stored to a global but never read, determine whether
+/// it's safe to remove the store and the chain of computation that feeds the
+/// store.
+static bool IsSafeComputationToRemove(Value *V) {
+ do {
+ if (!V->hasOneUse())
+ return false;
+ if (isa<LoadInst>(V) || isa<Argument>(V) || isa<GlobalValue>(V))
+ return false;
+ if (isAllocationFn(V) || isa<Constant>(V))
+ return true;
+
+ Instruction *I = cast<Instruction>(V);
+ if (I->mayHaveSideEffects())
+ return false;
+ if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I)) {
+ if (!GEP->hasAllConstantIndices())
+ return false;
+ } else if (I->getNumOperands() != 1) {
+ return false;
+ }
+
+ V = I->getOperand(0);
+ } while (1);
+}
+
+/// CleanupPointerRootUsers - This GV is a pointer root. Loop over all users
+/// of the global and clean up any that obviously don't assign the global a
+/// value that isn't dynamically allocated.
+static bool CleanupPointerRootUsers(GlobalVariable *GV) {
+ bool Changed = false;
+
+ // If Dead[n].first is the only use of a malloc result, we can delete its
+ // chain of computation and the store to the global in Dead[n].second.
+ SmallVector<std::pair<Instruction *, Instruction *>, 32> Dead;
+
+ // Constants can't be pointers to dynamically allocated memory.
+ for (Value::use_iterator UI = GV->use_begin(), E = GV->use_end();
+ UI != E;) {
+ User *U = *UI++;
+ if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
+ Value *V = SI->getValueOperand();
+ if (isa<Constant>(V)) {
+ Changed = true;
+ SI->eraseFromParent();
+ } else if (Instruction *I = dyn_cast<Instruction>(V)) {
+ if (I->hasOneUse())
+ Dead.push_back(std::make_pair(I, SI));
+ }
+ } else if (MemSetInst *MSI = dyn_cast<MemSetInst>(U)) {
+ if (isa<Constant>(MSI->getValue())) {
+ Changed = true;
+ MSI->eraseFromParent();
+ } else if (Instruction *I = dyn_cast<Instruction>(MSI->getValue())) {
+ if (I->hasOneUse())
+ Dead.push_back(std::make_pair(I, MSI));
+ }
+ } else if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(U)) {
+ GlobalVariable *MemSrc = dyn_cast<GlobalVariable>(MTI->getSource());
+ if (MemSrc && MemSrc->isConstant()) {
+ Changed = true;
+ MTI->eraseFromParent();
+ } else if (Instruction *I = dyn_cast<Instruction>(MemSrc)) {
+ if (I->hasOneUse())
+ Dead.push_back(std::make_pair(I, MTI));
+ }
+ } else if (ConstantExpr *CE = dyn_cast<ConstantExpr>(U)) {
+ if (CE->use_empty()) {
+ CE->destroyConstant();
+ Changed = true;
+ }
+ } else if (Constant *C = dyn_cast<Constant>(U)) {
+ if (SafeToDestroyConstant(C)) {
+ C->destroyConstant();
+ // This could have invalidated UI, start over from scratch.
+ Dead.clear();
+ CleanupPointerRootUsers(GV);
+ return true;
+ }
+ }
+ }
+
+ for (int i = 0, e = Dead.size(); i != e; ++i) {
+ if (IsSafeComputationToRemove(Dead[i].first)) {
+ Dead[i].second->eraseFromParent();
+ Instruction *I = Dead[i].first;
+ do {
+ Instruction *J = dyn_cast<Instruction>(I->getOperand(0));
+ if (!J)
+ break;
+ I->eraseFromParent();
+ I = J;
+ } while (!isAllocationFn(I));
+ I->eraseFromParent();
+ }
+ }
+
+ if (GV->use_empty()) {
+ GV->eraseFromParent();
+ ++NumDeleted;
+ Changed = true;
+ }
+
+ return Changed;
+}
+
/// CleanupConstantGlobalUsers - We just marked GV constant. Loop over all
/// users of the global, cleaning up the obvious ones. This is largely just a
/// quick scan over the use list to clean up the easy and obvious cruft. This
@@ -812,13 +965,18 @@
// If we nuked all of the loads, then none of the stores are needed either,
// nor is the global.
if (AllNonStoreUsesGone) {
- DEBUG(dbgs() << " *** GLOBAL NOW DEAD!\n");
- CleanupConstantGlobalUsers(GV, 0, TD, TLI);
+ if (isLeakCheckerRoot(GV)) {
+ Changed |= CleanupPointerRootUsers(GV);
+ } else {
+ Changed = true;
+ CleanupConstantGlobalUsers(GV, 0, TD, TLI);
+ }
if (GV->use_empty()) {
+ DEBUG(dbgs() << " *** GLOBAL NOW DEAD!\n");
+ Changed = true;
GV->eraseFromParent();
++NumDeleted;
}
- Changed = true;
}
return Changed;
}
@@ -1794,10 +1952,15 @@
if (!GS.isLoaded) {
DEBUG(dbgs() << "GLOBAL NEVER LOADED: " << *GV);
- // Delete any stores we can find to the global. We may not be able to
- // make it completely dead though.
- bool Changed = CleanupConstantGlobalUsers(GV, GV->getInitializer(),
- TD, TLI);
+ bool Changed;
+ if (isLeakCheckerRoot(GV)) {
+ // Delete any constant stores to the global.
+ Changed = CleanupPointerRootUsers(GV);
+ } else {
+ // Delete any stores we can find to the global. We may not be able to
+ // make it completely dead though.
+ Changed = CleanupConstantGlobalUsers(GV, GV->getInitializer(), TD, TLI);
+ }
// If the global is dead now, delete it.
if (GV->use_empty()) {
@@ -1845,7 +2008,7 @@
if (GV->use_empty()) {
DEBUG(dbgs() << " *** Substituting initializer allowed us to "
- << "simplify all users and delete global!\n");
+ << "simplify all users and delete global!\n");
GV->eraseFromParent();
++NumDeleted;
} else {
Modified: llvm/trunk/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll?rev=160529&r1=160528&r2=160529&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll (original)
+++ llvm/trunk/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll Thu Jul 19 17:35:28 2012
@@ -17,7 +17,7 @@
%2 = sext i32 %1 to i64 ; <i64> [#uses=1]
%3 = mul i64 %2, ptrtoint (%struct.strchartype* getelementptr (%struct.strchartype* null, i64 1) to i64) ; <i64> [#uses=1]
%4 = tail call i8* @malloc(i64 %3) ; <i8*> [#uses=1]
-; CHECK: call i8* @malloc(i64
+; CHECK-NOT: call i8* @malloc(i64
%5 = bitcast i8* %4 to %struct.strchartype* ; <%struct.strchartype*> [#uses=1]
store %struct.strchartype* %5, %struct.strchartype** @chartypes, align 8
ret void
Added: llvm/trunk/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll?rev=160529&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll (added)
+++ llvm/trunk/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll Thu Jul 19 17:35:28 2012
@@ -0,0 +1,20 @@
+; RUN: opt -globalopt -S -o - < %s | FileCheck %s
+
+ at test1 = internal global i8* null
+
+define void @test1a() {
+; CHECK: @test1a
+; CHECK-NOT: store
+; CHECK-NEXT: ret void
+ store i8* null, i8** @test1
+ ret void
+}
+
+define void @test1b(i8* %p) {
+; CHECK: @test1b
+; CHECK-NEXT: store
+; CHECK-NEXT: ret void
+ store i8* %p, i8** @test1
+ ret void
+}
+
More information about the llvm-commits
mailing list