[llvm] 1ba99ea - Revert "[DSE] Remove calls with known writes to dead memory"
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 18 00:25:20 PST 2021
Author: Nikita Popov
Date: 2021-12-18T09:23:41+01:00
New Revision: 1ba99eaf70951a4aa062688577047d8a23bfe54c
URL: https://github.com/llvm/llvm-project/commit/1ba99eaf70951a4aa062688577047d8a23bfe54c
DIFF: https://github.com/llvm/llvm-project/commit/1ba99eaf70951a4aa062688577047d8a23bfe54c.diff
LOG: Revert "[DSE] Remove calls with known writes to dead memory"
This reverts commit a8a51fe55649f5e07f9f2973507dc20bc4e40765.
This breaks the strncpy-overflow.cpp test case.
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 636582827297c..c74b6258f780e 100644
--- a/llvm/lib/Analysis/MemoryLocation.cpp
+++ b/llvm/lib/Analysis/MemoryLocation.cpp
@@ -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,
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index eb5eadba194d5..0657d291caca9 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -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;
}
diff --git a/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll b/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
index 13fd0e2ded0c1..caac984e7ef6b 100644
--- a/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
@@ -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
More information about the llvm-commits
mailing list