[InstCombine] Read-only call without return can capture

[nikic]

The copied from constant memory analysis had a special case where nocapture was not required for read-only calls without (or unused) return. This is not correct, as the address can still be captured though means other than memory and the return value, for example using divergence. This code should not be trying to do its own nocapture inference.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

The copied from constant memory analysis had a special case where nocapture was not required for read-only calls without (or unused) return. This is not correct, as the address can still be captured though means other than memory and the return value, for example using divergence. This code should not be trying to do its own nocapture inference.

---

Full diff: https://github.com/llvm/llvm-project/pull/157878.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (+2-2)
• (modified) llvm/test/Transforms/InstCombine/memcpy-from-global.ll (+21-6)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 4b10586616c29..53e77e6cc5c31 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -107,8 +107,8 @@ isOnlyCopiedFromConstantMemory(AAResults *AA, AllocaInst *V,
         // a load (but one that potentially returns the value itself), so we can
         // ignore it if we know that the value isn't captured.
         bool NoCapture = Call->doesNotCapture(DataOpNo);
-        if ((Call->onlyReadsMemory() && (Call->use_empty() || NoCapture)) ||
-            (Call->onlyReadsMemory(DataOpNo) && NoCapture))
+        if (NoCapture &&
+            (Call->onlyReadsMemory() || Call->onlyReadsMemory(DataOpNo)))
           continue;
       }
 
diff --git a/llvm/test/Transforms/InstCombine/memcpy-from-global.ll b/llvm/test/Transforms/InstCombine/memcpy-from-global.ll
index ff85d827bdcb4..f10ba1e3d27e6 100644
--- a/llvm/test/Transforms/InstCombine/memcpy-from-global.ll
+++ b/llvm/test/Transforms/InstCombine/memcpy-from-global.ll
@@ -139,13 +139,14 @@ define void @test2_addrspacecast() {
   ret void
 }
 
-declare void @bar(ptr)
-declare void @bar_as1(ptr addrspace(1))
+declare void @bar(ptr nocapture)
+declare void @bar_may_capture(ptr)
+declare void @bar_as1(ptr addrspace(1) nocapture)
 
 
 ;; Should be able to eliminate the alloca.
-define void @test3() {
-; CHECK-LABEL: @test3(
+define void @test3_nocapture() {
+; CHECK-LABEL: @test3_nocapture(
 ; CHECK-NEXT:    call void @bar(ptr nonnull @G) #[[ATTR3:[0-9]+]]
 ; CHECK-NEXT:    ret void
 ;
@@ -155,6 +156,20 @@ define void @test3() {
   ret void
 }
 
+; Can not eliminate the alloca, as the function may capture its address.
+define void @test3_may_capture() {
+; CHECK-LABEL: @test3_may_capture(
+; CHECK-NEXT:    [[A:%.*]] = alloca [[T:%.*]], align 8
+; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 8 dereferenceable(124) [[A]], ptr noundef nonnull align 16 dereferenceable(124) @G, i64 124, i1 false)
+; CHECK-NEXT:    call void @bar_may_capture(ptr nonnull [[A]]) #[[ATTR3]]
+; CHECK-NEXT:    ret void
+;
+  %A = alloca %T
+  call void @llvm.memcpy.p0.p0.i64(ptr align 4 %A, ptr align 4 @G, i64 124, i1 false)
+  call void @bar_may_capture(ptr %A) readonly
+  ret void
+}
+
 define void @test3_addrspacecast() {
 ; CHECK-LABEL: @test3_addrspacecast(
 ; CHECK-NEXT:    call void @bar(ptr nonnull @G) #[[ATTR3]]
@@ -395,12 +410,12 @@ define void @memcpy_to_capturing_readonly() {
 ; CHECK-LABEL: @memcpy_to_capturing_readonly(
 ; CHECK-NEXT:    [[A:%.*]] = alloca [[U:%.*]], align 16
 ; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 16 dereferenceable(20) [[A]], ptr noundef nonnull align 16 dereferenceable(20) @H, i64 20, i1 false)
-; CHECK-NEXT:    call void @bar(ptr nonnull readonly [[A]])
+; CHECK-NEXT:    call void @bar_may_capture(ptr nonnull readonly [[A]])
 ; CHECK-NEXT:    ret void
 ;
   %A = alloca %U, align 16
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %A, ptr align 4 @H, i64 20, i1 false)
-  call void @bar(ptr readonly %A)
+  call void @bar_may_capture(ptr readonly %A)
   ret void
 }
 

[dtcxzyw]

Although we can still infer nocapture if the callee has willreturn + nounwind, it seems not profitable in practice...

[tmiasko]

we can still infer nocapture if the callee has willreturn + nounwind

That would be insufficient still, since a callee can exhibit undefined behavior conditionally based on captured address. A counter example from a similar issue #129090 (comment).

[dianqk]

Should this patch be backported?

[nikic]

I don't think so. It's unlikely to affect non-artificial code.