[PATCH] D116148: [MemoryLocation] Don't require nocapture in getForDest()

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: reames, anna, fhahn.
Herald added a subscriber: hiraditya.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

As @reames mentioned on related reviews, we don't need the nocapture requirement here. First of all, from an API perspective, this is not something that MemoryLocation::getForDest() should be checking in the first place, because it does not affect which memory this particular call can access; it's an orthogonal concern that should be handled by the caller if necessary.

However, for both of the motivating users in DSE and InstCombine, we don't need the nocapture requirement, because the capture can either be purely local to the call (a pointer identity check that is irrelevant to us), be part of the return value (which we check is unused), or be written in the dest location, which we have determined to be dead.

This allows us to remove the special handling for libcalls as well.


https://reviews.llvm.org/D116148

Files:
  llvm/lib/Analysis/MemoryLocation.cpp
  llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
  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
@@ -192,14 +192,10 @@
   ret void
 }
 
-; But that removing a capture of an unrelated pointer isn't okay.
-define void @test_neg_unreleated_capture() {
-; CHECK-LABEL: @test_neg_unreleated_capture(
-; 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 nonnull writeonly [[BITCAST]], i8* nonnull readonly [[BITCAST2]]) #[[ATTR1]]
+; Removing a capture is also okay. The capture can only be in the return value
+; (which is unused) or written into the dead out parameter.
+define void @test_unrelated_capture() {
+; CHECK-LABEL: @test_unrelated_capture(
 ; CHECK-NEXT:    ret void
 ;
   %a = alloca i32, align 4
Index: llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
===================================================================
--- llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
+++ llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
@@ -205,14 +205,10 @@
   ret void
 }
 
-; But that removing a capture of an unrelated pointer isn't okay.
-define void @test_neg_unreleated_capture() {
-; CHECK-LABEL: @test_neg_unreleated_capture(
-; 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* readonly [[BITCAST2]]) #[[ATTR1]]
+; Removing a capture is also okay. The capture can only be in the return value
+; (which is unused) or written into the dead out parameter.
+define void @test_unrelated_capture() {
+; CHECK-LABEL: @test_unrelated_capture(
 ; CHECK-NEXT:    ret void
 ;
   %a = alloca i32, align 4
Index: llvm/lib/Analysis/MemoryLocation.cpp
===================================================================
--- llvm/lib/Analysis/MemoryLocation.cpp
+++ llvm/lib/Analysis/MemoryLocation.cpp
@@ -134,19 +134,6 @@
     }
   }
 
-  LibFunc LF;
-  if (TLI.getLibFunc(*CB, LF) && TLI.has(LF)) {
-    switch (LF) {
-    case LibFunc_strncpy:
-    case LibFunc_strcpy:
-    case LibFunc_strcat:
-    case LibFunc_strncat:
-      return getForArgument(CB, 0, &TLI);
-    default:
-      break;
-    }
-  }
-
   if (!CB->onlyAccessesArgMemory())
     return None;
 
@@ -159,9 +146,6 @@
   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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116148.395816.patch
Type: text/x-patch
Size: 3070 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211222/e486d9bb/attachment.bin>