[llvm] 44d23d5 - [DSE] Remove calls with known writes to dead memory
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 20 18:14:53 PST 2021
Author: Philip Reames
Date: 2021-12-20T18:10:23-08:00
New Revision: 44d23d5345a6ed377c238ebd74f0c7a628e1c968
URL: https://github.com/llvm/llvm-project/commit/44d23d5345a6ed377c238ebd74f0c7a628e1c968
DIFF: https://github.com/llvm/llvm-project/commit/44d23d5345a6ed377c238ebd74f0c7a628e1c968.diff
LOG: [DSE] Remove calls with known writes to dead memory
This is a reapply of a8a51fe5, which was reverted in 1ba99e due to a failing compiler-rt test. That test was a false positive because it was checking asan failures not accounting for the fact the call could be validly optimized out. I hopefully managed to stablize that test in 9b955f. (That's a speculative fix due to disk consumption needed to build compiler-rt tests locally being absurd.)
Original commit message follows..
The majority of this change is sinking logic from instcombine into MemoryLocation such that it can be generically reused. If we have a call with a single analyzable write to an argument, we can treat that as-if it were a store of unknown size.
Merging the code in this was unblocks DSE in the store to dead memory code paths. In theory, it should also enable classic DSE of such calls, but the code appears to not know how to use object sizes to refine unknown access bounds (yet).
In addition, this does make the isAllocRemovable path slightly stronger by reusing the libfunc and additional intrinsics bits which are already in getForDest.
Differential Revision: https://reviews.llvm.org/D115904
Added:
Modified:
llvm/lib/Analysis/MemoryLocation.cpp
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/MemoryLocation.cpp b/llvm/lib/Analysis/MemoryLocation.cpp
index c74b6258f780..636582827297 100644
--- a/llvm/lib/Analysis/MemoryLocation.cpp
+++ b/llvm/lib/Analysis/MemoryLocation.cpp
@@ -147,7 +147,44 @@ MemoryLocation::getForDest(const CallBase *CB, const TargetLibraryInfo &TLI) {
}
}
- return None;
+ if (!CB->onlyAccessesArgMemory())
+ return None;
+
+ if (CB->hasOperandBundles())
+ // TODO: remove implementation restriction
+ return None;
+
+ Value *UsedV = nullptr;
+ Optional<unsigned> UsedIdx;
+ for (unsigned i = 0; i < CB->arg_size(); i++) {
+ if (!CB->getArgOperand(i)->getType()->isPointerTy())
+ continue;
+ if (!CB->doesNotCapture(i))
+ // capture would allow the address to be read back in an untracked manner
+ return None;
+ if (CB->onlyReadsMemory(i))
+ continue;
+ if (!UsedV) {
+ // First potentially writing parameter
+ UsedV = CB->getArgOperand(i);
+ UsedIdx = i;
+ continue;
+ }
+ UsedIdx = None;
+ if (UsedV != CB->getArgOperand(i))
+ // Can't describe writing to two distinct locations.
+ // TODO: This results in an inprecision when two values derived from the
+ // same object are passed as arguments to the same function.
+ return None;
+ }
+ if (!UsedV)
+ // We don't currently have a way to represent a "does not write" result
+ // and thus have to be conservative and return unknown.
+ return None;
+
+ if (UsedIdx)
+ return getForArgument(CB, *UsedIdx, &TLI);
+ return MemoryLocation::getBeforeOrAfter(UsedV, CB->getAAMetadata());
}
MemoryLocation MemoryLocation::getForArgument(const CallBase *Call,
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 0657d291caca..eb5eadba194d 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2562,35 +2562,24 @@ static bool isNeverEqualToUnescapedAlloc(Value *V, const TargetLibraryInfo &TLI,
/// Given a call CB which uses an address UsedV, return true if we can prove the
/// call's only possible effect is storing to V.
-static bool isRemovableWrite(CallBase &CB, Value *UsedV) {
+static bool isRemovableWrite(CallBase &CB, Value *UsedV,
+ const TargetLibraryInfo &TLI) {
if (!CB.use_empty())
// TODO: add recursion if returned attribute is present
return false;
- if (!CB.willReturn() || !CB.doesNotThrow() || !CB.onlyAccessesArgMemory() ||
- CB.isTerminator())
+ if (CB.isTerminator())
+ // TODO: remove implementation restriction
return false;
- if (CB.hasOperandBundles())
+ if (!CB.willReturn() || !CB.doesNotThrow())
return false;
- for (unsigned i = 0; i < CB.arg_size(); i++) {
- if (!CB.getArgOperand(i)->getType()->isPointerTy())
- continue;
- if (!CB.doesNotCapture(i))
- // capture would allow the address to be read back in an untracked manner
- return false;
- if (UsedV != CB.getArgOperand(i) && !CB.onlyReadsMemory(i))
- // A write to another memory location keeps the call live, and thus we
- // must keep the alloca so that the call has somewhere to write to.
- // TODO: This results in an inprecision when two values derived from the
- // same alloca are passed as arguments to the same function.
- return false;
- // Note: Both reads from and writes to the alloca are fine. Since the
- // result is unused nothing can observe the values read from the alloca
- // without writing it to some other observable location (checked above).
- }
- return true;
+ // If the only possible side effect of the call is writing to the alloca,
+ // and the result isn't used, we can safely remove any reads implied by the
+ // call including those which might read the alloca itself.
+ Optional<MemoryLocation> Dest = MemoryLocation::getForDest(&CB, TLI);
+ return Dest && Dest->Ptr == UsedV;
}
static bool isAllocSiteRemovable(Instruction *AI,
@@ -2660,7 +2649,7 @@ static bool isAllocSiteRemovable(Instruction *AI,
}
}
- if (isRemovableWrite(*cast<CallBase>(I), PI)) {
+ if (isRemovableWrite(*cast<CallBase>(I), PI, TLI)) {
Users.emplace_back(I);
continue;
}
diff --git a/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll b/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
index caac984e7ef6..13fd0e2ded0c 100644
--- a/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
@@ -11,9 +11,6 @@ declare void @f2(i8*, i8*)
; Basic case for DSEing a trivially dead writing call
define void @test_dead() {
; CHECK-LABEL: @test_dead(
-; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
-; CHECK-NEXT: [[BITCAST:%.*]] = bitcast i32* [[A]] to i8*
-; CHECK-NEXT: call void @f(i8* nocapture writeonly [[BITCAST]]) #[[ATTR1:[0-9]+]]
; CHECK-NEXT: ret void
;
%a = alloca i32, align 4
@@ -28,7 +25,6 @@ define void @test_lifetime() {
; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
; CHECK-NEXT: [[BITCAST:%.*]] = bitcast i32* [[A]] to i8*
; CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 4, i8* [[BITCAST]])
-; CHECK-NEXT: call void @f(i8* nocapture writeonly [[BITCAST]]) #[[ATTR1]]
; CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 4, i8* [[BITCAST]])
; CHECK-NEXT: ret void
;
@@ -48,7 +44,6 @@ define void @test_lifetime2() {
; CHECK-NEXT: [[BITCAST:%.*]] = bitcast i32* [[A]] to i8*
; CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 4, i8* [[BITCAST]])
; CHECK-NEXT: call void @unknown()
-; CHECK-NEXT: call void @f(i8* nocapture writeonly [[BITCAST]]) #[[ATTR1]]
; CHECK-NEXT: call void @unknown()
; CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 4, i8* [[BITCAST]])
; CHECK-NEXT: ret void
@@ -67,9 +62,6 @@ define void @test_lifetime2() {
; itself since the write will be dropped.
define void @test_dead_readwrite() {
; CHECK-LABEL: @test_dead_readwrite(
-; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
-; CHECK-NEXT: [[BITCAST:%.*]] = bitcast i32* [[A]] to i8*
-; CHECK-NEXT: call void @f(i8* nocapture [[BITCAST]]) #[[ATTR1]]
; CHECK-NEXT: ret void
;
%a = alloca i32, align 4
@@ -82,7 +74,7 @@ define i32 @test_neg_read_after() {
; CHECK-LABEL: @test_neg_read_after(
; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
; CHECK-NEXT: [[BITCAST:%.*]] = bitcast i32* [[A]] to i8*
-; CHECK-NEXT: call void @f(i8* nocapture writeonly [[BITCAST]]) #[[ATTR1]]
+; CHECK-NEXT: call void @f(i8* nocapture writeonly [[BITCAST]]) #[[ATTR1:[0-9]+]]
; CHECK-NEXT: [[RES:%.*]] = load i32, i32* [[A]], align 4
; CHECK-NEXT: ret i32 [[RES]]
;
@@ -203,11 +195,6 @@ define i32 @test_neg_captured_before() {
; Show that reading from unrelated memory is okay
define void @test_unreleated_read() {
; CHECK-LABEL: @test_unreleated_read(
-; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
-; CHECK-NEXT: [[A2:%.*]] = alloca i32, align 4
-; CHECK-NEXT: [[BITCAST:%.*]] = bitcast i32* [[A]] to i8*
-; CHECK-NEXT: [[BITCAST2:%.*]] = bitcast i32* [[A2]] to i8*
-; CHECK-NEXT: call void @f2(i8* nocapture writeonly [[BITCAST]], i8* nocapture readonly [[BITCAST2]]) #[[ATTR1]]
; CHECK-NEXT: ret void
;
%a = alloca i32, align 4
@@ -240,9 +227,6 @@ define void @test_neg_unreleated_capture() {
; itself since the write will be dropped.
define void @test_self_read() {
; CHECK-LABEL: @test_self_read(
-; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
-; CHECK-NEXT: [[BITCAST:%.*]] = bitcast i32* [[A]] to i8*
-; CHECK-NEXT: call void @f2(i8* nocapture writeonly [[BITCAST]], i8* nocapture readonly [[BITCAST]]) #[[ATTR1]]
; CHECK-NEXT: ret void
;
%a = alloca i32, align 4
More information about the llvm-commits
mailing list