diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -106,175 +106,6 @@ "entry frequency, for a call site to be considered cold for enabling" "coldcc")); -/// 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 Types; - Types.push_back(GV->getValueType()); - - unsigned Limit = 20; - do { - Type *Ty = Types.pop_back_val(); - switch (Ty->getTypeID()) { - default: break; - case Type::PointerTyID: - return true; - case Type::FixedVectorTyID: - case Type::ScalableVectorTyID: - if (cast(Ty)->getElementType()->isPointerTy()) - return true; - break; - case Type::ArrayTyID: - Types.push_back(cast(Ty)->getElementType()); - break; - case Type::StructTyID: { - StructType *STy = cast(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(InnerTy)) return true; - if (isa(InnerTy) || isa(InnerTy) || - isa(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, function_ref GetTLI) { - do { - if (isa(V)) - return true; - if (!V->hasOneUse()) - return false; - if (isa(V) || isa(V) || isa(V) || - isa(V)) - return false; - if (isAllocationFn(V, GetTLI)) - return true; - - Instruction *I = cast(V); - if (I->mayHaveSideEffects()) - return false; - if (GetElementPtrInst *GEP = dyn_cast(I)) { - if (!GEP->hasAllConstantIndices()) - return false; - } else if (I->getNumOperands() != 1) { - return false; - } - - V = I->getOperand(0); - } while (true); -} - -/// 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, - function_ref GetTLI) { - // A brief explanation of leak checkers. The goal is to find bugs where - // pointers are forgotten, causing an accumulating growth in memory - // usage over time. The common strategy for leak checkers is to explicitly - // allow the memory pointed to by globals at exit. This is popular because it - // also solves another problem where the main thread of a C++ program may shut - // down before other threads that are still expecting to use those globals. To - // handle that case, we expect the program may create a singleton and never - // destroy it. - - 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, 32> Dead; - - // Constants can't be pointers to dynamically allocated memory. - for (Value::user_iterator UI = GV->user_begin(), E = GV->user_end(); - UI != E;) { - User *U = *UI++; - if (StoreInst *SI = dyn_cast(U)) { - Value *V = SI->getValueOperand(); - if (isa(V)) { - Changed = true; - SI->eraseFromParent(); - } else if (Instruction *I = dyn_cast(V)) { - if (I->hasOneUse()) - Dead.push_back(std::make_pair(I, SI)); - } - } else if (MemSetInst *MSI = dyn_cast(U)) { - if (isa(MSI->getValue())) { - Changed = true; - MSI->eraseFromParent(); - } else if (Instruction *I = dyn_cast(MSI->getValue())) { - if (I->hasOneUse()) - Dead.push_back(std::make_pair(I, MSI)); - } - } else if (MemTransferInst *MTI = dyn_cast(U)) { - GlobalVariable *MemSrc = dyn_cast(MTI->getSource()); - if (MemSrc && MemSrc->isConstant()) { - Changed = true; - MTI->eraseFromParent(); - } else if (Instruction *I = dyn_cast(MemSrc)) { - if (I->hasOneUse()) - Dead.push_back(std::make_pair(I, MTI)); - } - } else if (ConstantExpr *CE = dyn_cast(U)) { - if (CE->use_empty()) { - CE->destroyConstant(); - Changed = true; - } - } else if (Constant *C = dyn_cast(U)) { - if (isSafeToDestroyConstant(C)) { - C->destroyConstant(); - // This could have invalidated UI, start over from scratch. - Dead.clear(); - CleanupPointerRootUsers(GV, GetTLI); - return true; - } - } - } - - for (int i = 0, e = Dead.size(); i != e; ++i) { - if (IsSafeComputationToRemove(Dead[i].first, GetTLI)) { - Dead[i].second->eraseFromParent(); - Instruction *I = Dead[i].first; - do { - if (isAllocationFn(I, GetTLI)) - break; - Instruction *J = dyn_cast(I->getOperand(0)); - if (!J) - break; - I->eraseFromParent(); - I = J; - } while (true); - I->eraseFromParent(); - Changed = true; - } - } - - return Changed; -} - /// 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 returns true if it made a change. @@ -823,12 +654,8 @@ // If we nuked all of the loads, then none of the stores are needed either, // nor is the global. if (AllNonStoreUsesGone) { - if (isLeakCheckerRoot(GV)) { - Changed |= CleanupPointerRootUsers(GV, GetTLI); - } else { - Changed = true; - CleanupConstantGlobalUsers(GV, nullptr, DL, GetTLI); - } + Changed = true; + CleanupConstantGlobalUsers(GV, nullptr, DL, GetTLI); if (GV->use_empty()) { LLVM_DEBUG(dbgs() << " *** GLOBAL NOW DEAD!\n"); Changed = true; @@ -1997,15 +1824,10 @@ if (!GS.IsLoaded) { LLVM_DEBUG(dbgs() << "GLOBAL NEVER LOADED: " << *GV << "\n"); - if (isLeakCheckerRoot(GV)) { - // Delete any constant stores to the global. - Changed = CleanupPointerRootUsers(GV, GetTLI); - } 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(), DL, GetTLI); - } + // 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(), DL, GetTLI); // If the global is dead now, delete it. if (GV->use_empty()) { diff --git a/llvm/test/ThinLTO/X86/import-constant.ll b/llvm/test/ThinLTO/X86/import-constant.ll --- a/llvm/test/ThinLTO/X86/import-constant.ll +++ b/llvm/test/ThinLTO/X86/import-constant.ll @@ -32,11 +32,8 @@ ; IMPORT-NEXT: @_ZL3Obj.llvm.{{.*}} = available_externally hidden constant %struct.S { i32 4, i32 8, i32* @val } ; IMPORT-NEXT: @val = available_externally global i32 42 -; OPT: @outer = internal unnamed_addr global %struct.Q zeroinitializer - ; OPT: define dso_local i32 @main() ; OPT-NEXT: entry: -; OPT-NEXT: store %struct.S* null, %struct.S** getelementptr inbounds (%struct.Q, %struct.Q* @outer, i64 0, i32 0) ; OPT-NEXT: ret i32 12 ; NOREFS: @outer = internal local_unnamed_addr global %struct.Q zeroinitializer diff --git a/llvm/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll b/llvm/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll --- a/llvm/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll +++ b/llvm/test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll @@ -17,7 +17,7 @@ %2 = sext i32 %1 to i64 ; [#uses=1] %3 = mul i64 %2, ptrtoint (%struct.strchartype* getelementptr (%struct.strchartype, %struct.strchartype* null, i64 1) to i64) ; [#uses=1] %4 = tail call i8* @malloc(i64 %3) ; [#uses=1] -; CHECK-NOT: call i8* @malloc(i64 +; CHECK: 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 diff --git a/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll b/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll --- a/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll +++ b/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll @@ -1,49 +0,0 @@ -; RUN: opt -globalopt -S -o - < %s | FileCheck %s - -@glbl = internal global i8* null - -define void @test1a() { -; CHECK-LABEL: @test1a( -; CHECK-NOT: store -; CHECK-NEXT: ret void - store i8* null, i8** @glbl - ret void -} - -define void @test1b(i8* %p) { -; CHECK-LABEL: @test1b( -; CHECK-NEXT: store -; CHECK-NEXT: ret void - store i8* %p, i8** @glbl - ret void -} - -define void @test2() { -; CHECK-LABEL: @test2( -; CHECK: alloca i8 - %txt = alloca i8 - call void @foo2(i8* %txt) - %call2 = call i8* @strdup(i8* %txt) - store i8* %call2, i8** @glbl - ret void -} -declare i8* @strdup(i8*) -declare void @foo2(i8*) - -define void @test3() uwtable personality i32 (i32, i64, i8*, i8*)* @__gxx_personality_v0 { -; CHECK-LABEL: @test3( -; CHECK-NOT: bb1: -; CHECK-NOT: bb2: -; CHECK: invoke - %ptr = invoke i8* @_Znwm(i64 1) - to label %bb1 unwind label %bb2 -bb1: - store i8* %ptr, i8** @glbl - unreachable -bb2: - %tmp1 = landingpad { i8*, i32 } - cleanup - resume { i8*, i32 } %tmp1 -} -declare i32 @__gxx_personality_v0(i32, i64, i8*, i8*) -declare i8* @_Znwm(i64) diff --git a/llvm/test/Transforms/GlobalOpt/dead-store-status.ll b/llvm/test/Transforms/GlobalOpt/dead-store-status.ll --- a/llvm/test/Transforms/GlobalOpt/dead-store-status.ll +++ b/llvm/test/Transforms/GlobalOpt/dead-store-status.ll @@ -1,10 +1,10 @@ -; RUN: opt < %s -globalopt -S | FileCheck %s +; RUN: opt < %s -globalopt -instcombine -S | FileCheck %s ; When removing the store to @global in @foo, the pass would incorrectly return ; false. This was caught by the pass return status check that is hidden under ; EXPENSIVE_CHECKS. -; CHECK: @global = internal unnamed_addr global i16* null, align 1 +; CHECK-NOT: @global = internal unnamed_addr global i16* null, align 1 ; CHECK-LABEL: @foo ; CHECK-NEXT: entry: