maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Philip Reames 2021-12-20 18:10:23 -08:00
parent 9b955f77a1
commit 44d23d5345
3 changed files with 50 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
llvm/lib/Analysis/MemoryLocation.cpp
View File

@ -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,

33
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
View File

@ -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;
}

18
llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
View File

@ -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