[llvm] [GlobalOpt] Handle operators separately when removing GV users (PR #84694)
Anshil Gandhi via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 24 20:53:54 PDT 2024
https://github.com/gandhi56 updated https://github.com/llvm/llvm-project/pull/84694
>From 09c23889c8310b7fdc3b1db7320754d354f48296 Mon Sep 17 00:00:00 2001
From: Anshil Gandhi <gandhi21299 at gmail.com>
Date: Tue, 2 Apr 2024 20:02:21 -0600
Subject: [PATCH] [GlobalOpt] Handle operators separately when removing GV
users
This patch fixes the functionality of CleanupPointerRootUsers
by separately handling cases where the global variable is
wrapped within an operator.
Fixes #64680.
Change-Id: I205922e4203fb032beaf2c290c0374758ea9176c
---
llvm/lib/Transforms/IPO/GlobalOpt.cpp | 85 +++++++++----------
...leanup-pointer-root-users-gep-constexpr.ll | 2 -
.../Transforms/GlobalOpt/dead-store-status.ll | 2 -
llvm/test/Transforms/GlobalOpt/pr54572.ll | 8 --
4 files changed, 40 insertions(+), 57 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index da714c9a75701b..fcaf7e8c9ab2f7 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -201,61 +201,56 @@ 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;
-
+ // Collect all stores to GV.
+ SmallVector<Instruction *, 8> Writes;
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 (auto *SI = dyn_cast<StoreInst>(U)) {
+ Writes.push_back(SI);
+ } else if (auto *CE = dyn_cast<ConstantExpr>(U)) {
if (isa<GEPOperator>(CE))
append_range(Worklist, CE->users());
+ } else if (auto *MI = dyn_cast<MemIntrinsic>(U)) {
+ if (MI->getRawDest() == GV)
+ Writes.push_back(MI);
}
}
- 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);
+ auto RemoveComputationChain = [&GetTLI](Instruction *I) {
+ 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();
+ };
+
+ // Remove the store if it's value operand is a constant or a computation
+ // chain. If the value operand is a computation chain, determine if it is safe
+ // to remove it before removing the store. Remove both the store and the
+ // computation chain if it is safe to do so.
+ for (auto *Write : Writes) {
+ Value *V = Write;
+ if (auto *SI = dyn_cast<StoreInst>(V))
+ V = SI->getValueOperand();
+
+ if (isa<Constant>(V)) {
Changed = true;
+ Write->eraseFromParent();
+ } else if (isa<Instruction>(V)) {
+ if (IsSafeComputationToRemove(V, GetTLI)) {
+ Changed = true;
+ Write->eraseFromParent();
+ RemoveComputationChain(cast<Instruction>(V));
+ } else {
+ Changed = true;
+ Write->eraseFromParent();
+ }
}
}
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..db0c549ad03f4c 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,14 +99,12 @@ 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
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[TMP1]], 2
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY]], label [[FOR_END]]
; CHECK: for.end:
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr [[P:%.*]], i64 8, i1 false)
; CHECK-NEXT: ret i32 0
;
entry:
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..bdab4b0981d04a 100644
--- a/llvm/test/Transforms/GlobalOpt/pr54572.ll
+++ b/llvm/test/Transforms/GlobalOpt/pr54572.ll
@@ -6,18 +6,10 @@
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
-;.
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
}
-;.
-; CHECK: attributes #[[ATTR0:[0-9]+]] = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
-;.
More information about the llvm-commits
mailing list