Allow calls with known writes when trying to remove allocas [part 2]

[reames]

Summary

This is a slight generalization of D115829. I noticed this while restructuring code for a follow up patch to perform the same optimizations in DSE.

If we have a call whose only visible effect is writing to an alloca, and we're removing the alloca anyways, we don't care if the call also reads from the same alloca. That read will be unobservable and thus doesn't block removal of the call.

Worth noting is that this observation generalizes for non-argument reads. It just happens that case reduces to a readonly call, and is already handled separately.

[anna]

Good catch!

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

static bool isNeverEqualToUnescapedAlloc(Value *V, const TargetLibraryInfo &TLI,
   // We rely on LookThroughBitCast in isAllocLikeFn being false, since looking
   // through bitcasts of V can cause
   // the result statement below to be true, even when AI and V (ex:
   // i8* ->i32* ->i8* of AI) are the same allocations.
   return isAllocLikeFn(V, &TLI) && V != AI;
 }
 
 /// Given a call CB which uses an address UsedV, return true if we can prove the
-/// calls only effect is storing to V.
+/// call's only possible effect is storing to V.
 static bool isRemovableWrite(CallBase &CB, Value *UsedV) {
   if (!CB.use_empty())
     // TODO: add recursion if returned attribute is present
     return false;
 
   if (!CB.willReturn() || !CB.doesNotThrow() || !CB.onlyAccessesArgMemory() ||
       CB.isTerminator())
     return false;
 
   if (CB.hasOperandBundles())
     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)) {
-      if (!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;
-      continue;
-    }
-    if (!CB.paramHasAttr(i, Attribute::WriteOnly))
-      // a read would hold the address live
-      return false;
+    if (UsedV != CB.getArgOperand(i) && !CB.onlyReadsMemory(i))
+    // 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,
                                  SmallVectorImpl<WeakTrackingVH> &Users,
                                  const TargetLibraryInfo &TLI) {
   SmallVector<Instruction*, 4> Worklist;

llvm/test/Transforms/InstCombine/trivial-dse-calls.ll

;
   call void @llvm.lifetime.start.p0i8(i64 4, i8* %bitcast)
   call void @unknown()
   call void @f(i8* writeonly nocapture %bitcast) argmemonly nounwind willreturn
   call void @unknown()
   call void @llvm.lifetime.end.p0i8(i64 4, i8* %bitcast)
   ret void
 }
 
+; As long as the result is unused, we can even remove reads of the alloca
+; itself since the write will be dropped.
+define void @test_dead_readwrite() {
+; CHECK-LABEL: @test_dead_readwrite(
+; CHECK-NEXT:    ret void
+;
+  %a = alloca i32, align 4
+  %bitcast = bitcast i32* %a to i8*
+  call void @f(i8* nocapture %bitcast) argmemonly nounwind willreturn
+  ret void
+}
+
 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 nonnull writeonly [[BITCAST]]) #[[ATTR1:[0-9]+]]
 ; CHECK-NEXT:    [[RES:%.*]] = load i32, i32* [[A]], align 4
 ; CHECK-NEXT:    ret i32 [[RES]]
 ;
;
   %a = alloca i32, align 4
   %a2 = alloca i32, align 4
   %bitcast = bitcast i32* %a to i8*
   %bitcast2 = bitcast i32* %a2 to i8*
   call void @f2(i8* nocapture writeonly %bitcast, i8* readonly %bitcast2) argmemonly nounwind willreturn
   ret void
 }
 
+; As long as the result is unused, we can even remove reads of the alloca
+; itself since the write will be dropped.
+define void @test_self_read() {
+; CHECK-LABEL: @test_self_read(
+; CHECK-NEXT:    ret void
+;
+  %a = alloca i32, align 4
+  %bitcast = bitcast i32* %a to i8*
+  call void @f2(i8* nocapture writeonly %bitcast, i8* nocapture readonly %bitcast) argmemonly nounwind willreturn
+  ret void
+}
+