diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index 16a80e9ebbeaa..1812e3d4588be 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -108,55 +108,6 @@ static cl::opt ColdCCRelFreq( "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 (Type *InnerTy : STy->elements()) { - 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. @@ -165,7 +116,7 @@ static bool IsSafeComputationToRemove( do { if (isa(V)) return true; - if (!V->hasOneUse()) + if (V->hasNUsesOrMore(1)) return false; if (isa(V) || isa(V) || isa(V) || isa(V)) @@ -187,90 +138,12 @@ static bool IsSafeComputationToRemove( } 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; - - SmallVector Worklist(GV->users()); - // Constants can't be pointers to dynamically allocated memory. - while (!Worklist.empty()) { - User *U = Worklist.pop_back_val(); - 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(MTI->getSource())) { - if (I->hasOneUse()) - Dead.push_back(std::make_pair(I, MTI)); - } - } else if (ConstantExpr *CE = dyn_cast(U)) { - if (isa(CE)) - append_range(Worklist, CE->users()); - } - } - - 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; - } - } - - GV->removeDeadConstantUsers(); - 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. -static bool CleanupConstantGlobalUsers(GlobalVariable *GV, - const DataLayout &DL) { +static bool CleanupConstantGlobalUsers( + GlobalVariable *GV, const DataLayout &DL, + function_ref GetTLI) { Constant *Init = GV->getInitializer(); SmallVector WorkList(GV->users()); SmallPtrSet Visited; @@ -320,11 +193,30 @@ static bool CleanupConstantGlobalUsers(GlobalVariable *GV, } } } else if (StoreInst *SI = dyn_cast(U)) { - // Store must be unreachable or storing Init into the global. - EraseFromParent(SI); - } else if (MemIntrinsic *MI = dyn_cast(U)) { // memset/cpy/mv - if (getUnderlyingObject(MI->getRawDest()) == GV) - EraseFromParent(MI); + auto *V = SI->getValueOperand(); + if (isa(V)) { + EraseFromParent(SI); + } else if (isa(V)) { + EraseFromParent(SI); + if (IsSafeComputationToRemove(V, GetTLI)) + RecursivelyDeleteTriviallyDeadInstructions(V); + } else if (isa(V)) { + if (!V->getType()->isPointerTy()) + EraseFromParent(SI); + } + } else if (auto *MSI = dyn_cast(U)) { // memset/cpy/mv + if (getUnderlyingObject(MSI->getRawDest()) == GV) + EraseFromParent(MSI); + } else if (auto *MTI = dyn_cast(U)) { + auto *Src = MTI->getRawSource(); + auto *Dst = MTI->getRawDest(); + if (getUnderlyingObject(Dst) != GV) + continue; + if (isa(Src)) { + EraseFromParent(MTI); + if (IsSafeComputationToRemove(Src, GetTLI)) + RecursivelyDeleteTriviallyDeadInstructions(Src); + } } else if (IntrinsicInst *II = dyn_cast(U)) { if (II->getIntrinsicID() == Intrinsic::threadlocal_address) append_range(WorkList, II->users()); @@ -872,12 +764,7 @@ static bool OptimizeAwayTrappingUsesOfLoads( // 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, DL); - } + Changed |= CleanupConstantGlobalUsers(GV, DL, GetTLI); if (GV->use_empty()) { LLVM_DEBUG(dbgs() << " *** GLOBAL NOW DEAD!\n"); Changed = true; @@ -1491,15 +1378,7 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS, // Delete it now. 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, DL); - } + Changed = CleanupConstantGlobalUsers(GV, DL, GetTLI); // If the global is dead now, delete it. if (GV->use_empty()) { @@ -1523,7 +1402,7 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS, } // Clean up any obviously simplifiable users now. - Changed |= CleanupConstantGlobalUsers(GV, DL); + Changed |= CleanupConstantGlobalUsers(GV, DL, GetTLI); // If the global is dead now, just nuke it. if (GV->use_empty()) { @@ -1578,7 +1457,7 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS, } // Clean up any obviously simplifiable users now. - CleanupConstantGlobalUsers(GV, DL); + CleanupConstantGlobalUsers(GV, DL, GetTLI); if (GV->use_empty()) { LLVM_DEBUG(dbgs() << " *** Substituting initializer allowed us to " diff --git a/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-gep-constexpr.ll b/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-gep-constexpr.ll index 26728a74d032c..66a99c15ba4d3 100644 --- a/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-gep-constexpr.ll +++ b/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-gep-constexpr.ll @@ -99,7 +99,6 @@ define i32 @load_gv_from_op_remove_store(ptr %p) local_unnamed_addr { ; CHECK-NEXT: call void @fn0() ; CHECK-NEXT: br label [[IF_END]] ; CHECK: if.end: -; CHECK-NEXT: store ptr [[E]], ptr getelementptr inbounds ([3 x ptr], ptr @b, i64 0, i64 2), align 16 ; CHECK-NEXT: [[TMP1:%.*]] = load i32, ptr @a, align 4 ; CHECK-NEXT: [[INC:%.*]] = add nsw i32 [[TMP1]], 1 ; CHECK-NEXT: store i32 [[INC]], ptr @a, align 4 diff --git a/llvm/test/Transforms/GlobalOpt/dead-store-status.ll b/llvm/test/Transforms/GlobalOpt/dead-store-status.ll index 9a8fbb8d65f0e..597c08929af90 100644 --- a/llvm/test/Transforms/GlobalOpt/dead-store-status.ll +++ b/llvm/test/Transforms/GlobalOpt/dead-store-status.ll @@ -4,8 +4,6 @@ ; false. This was caught by the pass return status check that is hidden under ; EXPENSIVE_CHECKS. -; CHECK: @global = internal unnamed_addr global ptr null, align 1 - ; CHECK-LABEL: @foo ; CHECK-NEXT: entry: ; CHECK-NEXT: ret i16 undef diff --git a/llvm/test/Transforms/GlobalOpt/pr54572.ll b/llvm/test/Transforms/GlobalOpt/pr54572.ll index 962c75bd83b41..50f3ce92e104b 100644 --- a/llvm/test/Transforms/GlobalOpt/pr54572.ll +++ b/llvm/test/Transforms/GlobalOpt/pr54572.ll @@ -7,17 +7,24 @@ declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg) ;. -; CHECK: @[[B:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr global ptr null -; CHECK: @[[C:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr constant [2 x ptr] zeroinitializer +; CHECK: @b = internal unnamed_addr global ptr null ;. define void @test() { ; CHECK-LABEL: @test( -; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr getelementptr inbounds ([2 x ptr], ptr @c, i64 0, i64 1), i64 8, i1 false) ; CHECK-NEXT: ret void ; call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr getelementptr inbounds ([2 x ptr], ptr @c, i64 0, i64 1), i64 8, i1 false) ret void } + +define void @neg_test(ptr %arg) { +; CHECK-LABEL: @neg_test( +; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr [[ARG:%.*]], i64 8, i1 false) +; CHECK-NEXT: ret void +; + call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr %arg, i64 8, i1 false) + ret void +} ;. ; CHECK: attributes #[[ATTR0:[0-9]+]] = { nocallback nofree nounwind willreturn memory(argmem: readwrite) } ;.