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

Revert "[DSE] Remove calls with known writes to dead memory" 

This reverts commit a8a51fe556.

This breaks the strncpy-overflow.cpp test case.
This commit is contained in:
Nikita Popov 2021-12-18 09:11:33 +01:00
parent 3aae04c744
commit 1ba99eaf70
3 changed files with 40 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -147,44 +147,7 @@ MemoryLocation::getForDest(const CallBase *CB, const TargetLibraryInfo &TLI) {
}
}
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());
return None;
}
MemoryLocation MemoryLocation::getForArgument(const CallBase *Call,

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

@ -2562,24 +2562,35 @@ 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,
const TargetLibraryInfo &TLI) {
static bool isRemovableWrite(CallBase &CB, Value *UsedV) {
if (!CB.use_empty())
// TODO: add recursion if returned attribute is present
return false;
if (CB.isTerminator())
// TODO: remove implementation restriction
if (!CB.willReturn() || !CB.doesNotThrow() || !CB.onlyAccessesArgMemory() ||
CB.isTerminator())
return false;
if (!CB.willReturn() || !CB.doesNotThrow())
if (CB.hasOperandBundles())
return false;
// 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;
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;
}
static bool isAllocSiteRemovable(Instruction *AI,
@ -2649,7 +2660,7 @@ static bool isAllocSiteRemovable(Instruction *AI,
}
}
if (isRemovableWrite(*cast<CallBase>(I), PI, TLI)) {
if (isRemovableWrite(*cast<CallBase>(I), PI)) {
Users.emplace_back(I);
continue;
}

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

@ -11,6 +11,9 @@ 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
@ -25,6 +28,7 @@ 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
;
@ -44,6 +48,7 @@ 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
@ -62,6 +67,9 @@ 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
@ -74,7 +82,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:[0-9]+]]
; CHECK-NEXT: call void @f(i8* nocapture writeonly [[BITCAST]]) #[[ATTR1]]
; CHECK-NEXT: [[RES:%.*]] = load i32, i32* [[A]], align 4
; CHECK-NEXT: ret i32 [[RES]]
;
@ -195,6 +203,11 @@ 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
@ -227,6 +240,9 @@ 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