[llvm] [InstCombine] Remove Store when its Ptr is removable (PR #79565)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 29 07:25:05 PST 2024
https://github.com/ParkHanbum updated https://github.com/llvm/llvm-project/pull/79565
>From 21b364fd56f452fb96cbce5c22c903f847e4ba8a Mon Sep 17 00:00:00 2001
From: Hanbum Park <kese111 at gmail.com>
Date: Mon, 29 Jan 2024 23:01:18 +0900
Subject: [PATCH] [InstCombine] Remove Store when its Ptr is removable (#68120)
Previously, if the `Store`'s `Ptr` was `Alloca` and satisfy OneUse()
it would be removed. However, if `Alloca` was overlapped and
referenced by instructions such as `GEP` or `AddrSpaceCast`, it could
not be removed and an error occurred as a result.
This patch fixes this by having the `Store`'s `Ptr` find out
if the target is `Alloca`, and remove the `Store` if possible.
---
.../InstCombine/InstCombineInternal.h | 3 +
.../InstCombineLoadStoreAlloca.cpp | 60 +++++++++++++++++++
.../InstCombine/InstructionCombining.cpp | 6 +-
llvm/test/Transforms/InstCombine/store.ll | 51 ++++++++++++++++
4 files changed, 117 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index b75ab9ad30142d..ceae5c15deb486 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -157,6 +157,9 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
Instruction *visitGEPOfGEP(GetElementPtrInst &GEP, GEPOperator *Src);
Instruction *visitAllocaInst(AllocaInst &AI);
Instruction *visitAllocSite(Instruction &FI);
+ bool isAllocSiteRemovable(Instruction *AI,
+ SmallVectorImpl<WeakTrackingVH> &Users,
+ const TargetLibraryInfo &TLI);
Instruction *visitFree(CallInst &FI, Value *FreedOp);
Instruction *visitLoadInst(LoadInst &LI);
Instruction *visitStoreInst(StoreInst &SI);
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 1254a050027a45..c7eb8706aa804e 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -1353,6 +1353,63 @@ static bool equivalentAddressValues(Value *A, Value *B) {
return false;
}
+static bool isStoreSiteRemovable(StoreInst *SI, InstCombinerImpl &IC) {
+ if (canSimplifyNullStoreOrGEP(*SI) ||
+ !isa<Instruction>(SI->getPointerOperand()))
+ return false;
+
+ SmallVector<Instruction *, 4> Worklist;
+ Worklist.push_back(cast<Instruction>(SI->getPointerOperand()));
+ AllocaInst *AI = nullptr;
+ do {
+ Instruction *PI = Worklist.pop_back_val();
+ switch (PI->getOpcode()) {
+ default:
+ break;
+ case Instruction::AddrSpaceCast:
+ case Instruction::BitCast:
+ case Instruction::GetElementPtr:
+ if (auto TI = dyn_cast<Instruction>(PI->getOperand(0)))
+ Worklist.push_back(TI);
+ break;
+ case Instruction::Alloca: {
+ AI = cast<AllocaInst>(PI);
+ break;
+ }
+ }
+ for (User *U : PI->users()) {
+ Instruction *I = cast<Instruction>(U);
+ switch (I->getOpcode()) {
+ default:
+ return false;
+ case Instruction::AddrSpaceCast:
+ case Instruction::BitCast:
+ case Instruction::GetElementPtr:
+ continue;
+ case Instruction::Store: {
+ StoreInst *SI = cast<StoreInst>(I);
+ if (SI->isVolatile() || SI->getPointerOperand() != PI)
+ return false;
+ if (auto TI = dyn_cast<Instruction>(SI->getPointerOperand());
+ TI && SI->getPointerOperand() != PI)
+ Worklist.push_back(TI);
+ continue;
+ }
+ case Instruction::Alloca: {
+ AI = cast<AllocaInst>(I);
+ break;
+ }
+ }
+ }
+ } while (!Worklist.empty());
+
+ SmallVector<WeakTrackingVH, 64> temp;
+ if (AI && IC.isAllocSiteRemovable(AI, temp, IC.getTargetLibraryInfo()))
+ return true;
+
+ return false;
+}
+
Instruction *InstCombinerImpl::visitStoreInst(StoreInst &SI) {
Value *Val = SI.getOperand(0);
Value *Ptr = SI.getOperand(1);
@@ -1381,6 +1438,9 @@ Instruction *InstCombinerImpl::visitStoreInst(StoreInst &SI) {
// FIXME: Some bits are legal for ordered atomic stores; needs refactoring.
if (!SI.isUnordered()) return nullptr;
+ if (isStoreSiteRemovable(&SI, *this))
+ return eraseInstFromFunction(SI);
+
// If the RHS is an alloca with a single use, zapify the store, making the
// alloca dead.
if (Ptr->hasOneUse()) {
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index dd168917f4dc9c..d0d07770bce955 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2712,9 +2712,9 @@ static bool isRemovableWrite(CallBase &CB, Value *UsedV,
return Dest && Dest->Ptr == UsedV;
}
-static bool isAllocSiteRemovable(Instruction *AI,
- SmallVectorImpl<WeakTrackingVH> &Users,
- const TargetLibraryInfo &TLI) {
+bool InstCombinerImpl::isAllocSiteRemovable(
+ Instruction *AI, SmallVectorImpl<WeakTrackingVH> &Users,
+ const TargetLibraryInfo &TLI) {
SmallVector<Instruction*, 4> Worklist;
const std::optional<StringRef> Family = getAllocationFamily(AI, &TLI);
Worklist.push_back(AI);
diff --git a/llvm/test/Transforms/InstCombine/store.ll b/llvm/test/Transforms/InstCombine/store.ll
index 673395464c85aa..daf8dd03ab14ed 100644
--- a/llvm/test/Transforms/InstCombine/store.ll
+++ b/llvm/test/Transforms/InstCombine/store.ll
@@ -345,6 +345,57 @@ define void @store_to_readonly_noalias(ptr readonly noalias %0) {
ret void
}
+; if we know Ptr of store can be remove then remove it.
+define ptr @src_store_oneuse_addrspaceast_alloca_has_one_chained_users(ptr %arg) {
+; CHECK-LABEL: @src_store_oneuse_addrspaceast_alloca_has_one_chained_users(
+; CHECK-NEXT: bb:
+; CHECK-NEXT: ret ptr [[ARG:%.*]]
+;
+bb:
+ %i = alloca ptr, addrspace(5)
+ %gep = getelementptr i8, ptr addrspace(5) %i, i8 1
+ %i1 = addrspacecast ptr addrspace(5) %gep to ptr
+ store ptr %arg, ptr %i1
+ %i2 = load ptr, ptr %i1
+ ret ptr %i2
+}
+
+define ptr @src_store_oneuse_addrspaceast_alloca_has_two_chained_users(ptr %arg, i32 %t) {
+; CHECK-LABEL: @src_store_oneuse_addrspaceast_alloca_has_two_chained_users(
+; CHECK-NEXT: bb:
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[T:%.*]], 0
+; CHECK-NEXT: br i1 [[CMP]], label [[BB1:%.*]], label [[BB2:%.*]]
+; CHECK: bb1:
+; CHECK-NEXT: br label [[FIN:%.*]]
+; CHECK: bb2:
+; CHECK-NEXT: br label [[FIN]]
+; CHECK: fin:
+; CHECK-NEXT: ret ptr [[ARG:%.*]]
+;
+bb:
+ %i = alloca ptr, addrspace(5)
+ %gep = getelementptr i8, ptr addrspace(5) %i, i8 1
+ %i1 = addrspacecast ptr addrspace(5) %gep to ptr
+ %gep2 = getelementptr i8, ptr addrspace(5) %i, i8 1
+ %i2 = addrspacecast ptr addrspace(5) %gep2 to ptr
+ %cmp = icmp eq i32 %t, 0
+ br i1 %cmp, label %bb1, label %bb2
+
+bb1:
+ store ptr %arg, ptr %i1
+ %bb1_load = load ptr, ptr %i1
+ br label %fin
+
+bb2:
+ store ptr %arg, ptr %i2
+ %bb2_load = load ptr, ptr %i2
+ br label %fin
+
+fin:
+ %res = phi ptr [%bb1_load, %bb1], [%bb2_load, %bb2]
+ ret ptr %res
+}
+
!0 = !{!4, !4, i64 0}
!1 = !{!"omnipotent char", !2}
!2 = !{!"Simple C/C++ TBAA"}
More information about the llvm-commits
mailing list