[DSE] Remove calls with known writes to dead memory

This is a reapply of a8a51fe, 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

Author: preames

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,

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

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