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

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: nikic, anna.
Herald added subscribers: bollu, hiraditya, mcrosier.
reames requested review of this revision.
Herald added a project: LLVM.

This is a slight generalization of D115829 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115898

Files:
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/trivial-dse-calls.ll


Index: llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
===================================================================
--- llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
+++ llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
@@ -50,6 +50,18 @@
   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
@@ -198,3 +210,15 @@
   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
+}
+
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2561,7 +2561,7 @@
 }
 
 /// Given a call CB which uses an address UsedV, return true if we can prove the
-/// calls only effect is storing to V.
+/// calls 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
@@ -2580,18 +2580,15 @@
     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
+    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;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115898.394962.patch
Type: text/x-patch
Size: 3158 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211216/17248129/attachment.bin>