[llvm] [GlobalOpt] Handle operators separately when removing GV users (PR #84694)
Anshil Gandhi via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 7 10:41:31 PST 2025
https://github.com/gandhi56 updated https://github.com/llvm/llvm-project/pull/84694
>From 67a0a218e6a5f21a86235d957608ca4d5783e5de Mon Sep 17 00:00:00 2001
From: Anshil Gandhi <Anshil.Gandhi at amd.com>
Date: Mon, 28 Oct 2024 01:07:58 +0000
Subject: [PATCH] [GlobalOpt] Eliminate redundant cleanup functions
Remove CleanupPointerRootUsers and replace it
with an extended version of
CleanupConstantGlobalUsers.
Fixes #64680.
Change-Id: I4c3fc52890eec1a3706cba9ecb916fd344672756
---
llvm/lib/Transforms/IPO/GlobalOpt.cpp | 138 +++++-------------
...leanup-pointer-root-users-gep-constexpr.ll | 1 -
.../Transforms/GlobalOpt/dead-store-status.ll | 2 -
llvm/test/Transforms/GlobalOpt/pr54572.ll | 13 +-
4 files changed, 43 insertions(+), 111 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 16a80e9ebbeaab..dcab2049429e3d 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -160,12 +160,12 @@ static bool isLeakCheckerRoot(GlobalVariable *GV) {
/// 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(
+static bool isSafeComputationToRemove(
Value *V, function_ref<TargetLibraryInfo &(Function &)> GetTLI) {
do {
if (isa<Constant>(V))
return true;
- if (!V->hasOneUse())
+ if (V->hasNUsesOrMore(1))
return false;
if (isa<LoadInst>(V) || isa<InvokeInst>(V) || isa<Argument>(V) ||
isa<GlobalValue>(V))
@@ -187,90 +187,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<TargetLibraryInfo &(Function &)> 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<std::pair<Instruction *, Instruction *>, 32> Dead;
-
- SmallVector<User *> 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<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>(MTI->getSource())) {
- if (I->hasOneUse())
- Dead.push_back(std::make_pair(I, MTI));
- }
- } else if (ConstantExpr *CE = dyn_cast<ConstantExpr>(U)) {
- if (isa<GEPOperator>(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<Instruction>(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<TargetLibraryInfo &(Function &)> GetTLI) {
Constant *Init = GV->getInitializer();
SmallVector<User *, 8> WorkList(GV->users());
SmallPtrSet<User *, 8> Visited;
@@ -320,11 +242,30 @@ static bool CleanupConstantGlobalUsers(GlobalVariable *GV,
}
}
} else if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
- // Store must be unreachable or storing Init into the global.
- EraseFromParent(SI);
- } else if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(U)) { // memset/cpy/mv
- if (getUnderlyingObject(MI->getRawDest()) == GV)
- EraseFromParent(MI);
+ auto *V = SI->getValueOperand();
+ if (isa<Constant>(V)) {
+ EraseFromParent(SI);
+ } else if (isa<Instruction>(V)) {
+ EraseFromParent(SI);
+ if (isSafeComputationToRemove(V, GetTLI))
+ RecursivelyDeleteTriviallyDeadInstructions(V);
+ } else if (isa<Argument>(V)) {
+ if (!V->getType()->isPointerTy())
+ EraseFromParent(SI);
+ }
+ } else if (auto *MSI = dyn_cast<MemSetInst>(U)) { // memset/cpy/mv
+ if (getUnderlyingObject(MSI->getRawDest()) == GV)
+ EraseFromParent(MSI);
+ } else if (auto *MTI = dyn_cast<MemTransferInst>(U)) {
+ auto *Src = MTI->getRawSource();
+ auto *Dst = MTI->getRawDest();
+ if (getUnderlyingObject(Dst) != GV)
+ continue;
+ if (isa<Instruction, Operator>(Src)) {
+ EraseFromParent(MTI);
+ if (isSafeComputationToRemove(Src, GetTLI))
+ RecursivelyDeleteTriviallyDeadInstructions(Src);
+ }
} else if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(U)) {
if (II->getIntrinsicID() == Intrinsic::threadlocal_address)
append_range(WorkList, II->users());
@@ -872,12 +813,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 +1427,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 +1451,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 +1506,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 26728a74d032ca..66a99c15ba4d3b 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 9a8fbb8d65f0e0..597c08929af902 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 962c75bd83b415..50f3ce92e104b7 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) }
;.
More information about the llvm-commits
mailing list