[llvm] 8a0e35f - [MemoryLocation] Don't require nocapture in getForDest()

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 22 03:20:29 PST 2021


Author: Nikita Popov
Date: 2021-12-22T12:20:13+01:00
New Revision: 8a0e35f3a78d314d4661ff740a35e590f07b3297

URL: https://github.com/llvm/llvm-project/commit/8a0e35f3a78d314d4661ff740a35e590f07b3297
DIFF: https://github.com/llvm/llvm-project/commit/8a0e35f3a78d314d4661ff740a35e590f07b3297.diff

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

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.

Differential Revision: https://reviews.llvm.org/D116148

Added: 
    

Modified: 
    llvm/lib/Analysis/MemoryLocation.cpp
    llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
    llvm/test/Transforms/InstCombine/trivial-dse-calls.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/MemoryLocation.cpp b/llvm/lib/Analysis/MemoryLocation.cpp
index 636582827297c..120f4cda1e351 100644
--- a/llvm/lib/Analysis/MemoryLocation.cpp
+++ b/llvm/lib/Analysis/MemoryLocation.cpp
@@ -134,19 +134,6 @@ MemoryLocation::getForDest(const CallBase *CB, const TargetLibraryInfo &TLI) {
     }
   }
 
-  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 @@ MemoryLocation::getForDest(const CallBase *CB, const TargetLibraryInfo &TLI) {
   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) {

diff  --git a/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll b/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
index 13fd0e2ded0c1..2b3838b00b3ee 100644
--- a/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll
@@ -7,6 +7,7 @@ declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
 declare void @unknown()
 declare void @f(i8*)
 declare void @f2(i8*, i8*)
+declare i8* @f3(i8*, i8*)
 
 ; Basic case for DSEing a trivially dead writing call
 define void @test_dead() {
@@ -205,22 +206,38 @@ define void @test_unreleated_read() {
   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(
+; 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
+  %a2 = alloca i32, align 4
+  %bitcast = bitcast i32* %a to i8*
+  %bitcast2 = bitcast i32* %a2 to i8*
+  call i8* @f3(i8* nocapture writeonly %bitcast, i8* readonly %bitcast2) argmemonly nounwind willreturn
+  ret void
+}
+
+; Cannot remove call, as %bitcast2 is captured via the return value.
+define i8 @test_neg_unrelated_capture_used_via_return() {
+; CHECK-LABEL: @test_neg_unrelated_capture_used_via_return(
 ; 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]]
-; CHECK-NEXT:    ret void
+; CHECK-NEXT:    [[CAPTURE:%.*]] = call i8* @f3(i8* nocapture writeonly [[BITCAST]], i8* readonly [[BITCAST2]]) #[[ATTR1]]
+; CHECK-NEXT:    [[V:%.*]] = load i8, i8* [[CAPTURE]], align 1
+; CHECK-NEXT:    ret i8 [[V]]
 ;
   %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
+  %capture = call i8* @f3(i8* nocapture writeonly %bitcast, i8* readonly %bitcast2) argmemonly nounwind willreturn
+  %v = load i8, i8* %capture
+  ret i8 %v
 }
 
 ; As long as the result is unused, we can even remove reads of the alloca

diff  --git a/llvm/test/Transforms/InstCombine/trivial-dse-calls.ll b/llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
index f2c009ac37966..6e2433336d1a7 100644
--- a/llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
+++ b/llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
@@ -7,6 +7,7 @@ declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
 declare void @unknown()
 declare void @f(i8*)
 declare void @f2(i8*, i8*)
+declare i8* @f3(i8*, i8*)
 
 ; Basic case for DSEing a trivially dead writing call
 define void @test_dead() {
@@ -192,22 +193,38 @@ define void @test_unreleated_read() {
   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(
+; 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
+  %a2 = alloca i32, align 4
+  %bitcast = bitcast i32* %a to i8*
+  %bitcast2 = bitcast i32* %a2 to i8*
+  call i8* @f3(i8* nocapture writeonly %bitcast, i8* readonly %bitcast2) argmemonly nounwind willreturn
+  ret void
+}
+
+; Cannot remove call, as %bitcast2 is captured via the return value.
+define i8 @test_neg_unrelated_capture_used_via_return() {
+; CHECK-LABEL: @test_neg_unrelated_capture_used_via_return(
 ; 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]]
-; CHECK-NEXT:    ret void
+; CHECK-NEXT:    [[CAPTURE:%.*]] = call i8* @f3(i8* nocapture nonnull writeonly [[BITCAST]], i8* nonnull readonly [[BITCAST2]]) #[[ATTR1]]
+; CHECK-NEXT:    [[V:%.*]] = load i8, i8* [[CAPTURE]], align 1
+; CHECK-NEXT:    ret i8 [[V]]
 ;
   %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
+  %capture = call i8* @f3(i8* nocapture writeonly %bitcast, i8* readonly %bitcast2) argmemonly nounwind willreturn
+  %v = load i8, i8* %capture
+  ret i8 %v
 }
 
 ; As long as the result is unused, we can even remove reads of the alloca


        


More information about the llvm-commits mailing list