[funcattrs] Fix a bug in recently introduced writeonly argument inference

This fixes a bug in 740057d.  There's two ways to describe the issue:
* One caller hasn't yet proven nocapture on the argument.  Given that, the inference routine is responsible for bailing out on a potential capture.
* Even if we know the argument is nocapture, the access inference needs to traverse the exact set of users the capture tracking would (or exit conservatively).  Even if capture tracking can prove a store is non-capturing (e.g. to a local alloc which doesn't escape), we still need to track the copy of the pointer to see if it's later reloaded and accessed again.

Note that all the test changes except the newly added ones appear to be false negatives.  That is, cases where we could prove writeonly, but the current code isn't strong enough.  That's why I didn't spot this originally.

diff --git a/clang/test/CodeGen/ms-mixed-ptr-sizes.c b/clang/test/CodeGen/ms-mixed-ptr-sizes.c
index 294a891..ececa42 100644 (file)
--- a/clang/test/CodeGen/ms-mixed-ptr-sizes.c
+++ b/clang/test/CodeGen/ms-mixed-ptr-sizes.c
@@ -7,32 +7,32 @@ struct Foo {
 };
 void use_foo(struct Foo *f);
 void test_sign_ext(struct Foo *f, int * __ptr32 __sptr i) {
-// X64-LABEL: define dso_local void @test_sign_ext({{.*}}i32 addrspace(270)* writeonly %i)
-// X86-LABEL: define dso_local void @test_sign_ext(%struct.Foo* %f, i32* writeonly %i)
+// X64-LABEL: define dso_local void @test_sign_ext({{.*}}i32 addrspace(270)* %i)
+// X86-LABEL: define dso_local void @test_sign_ext(%struct.Foo* %f, i32* %i)
 // X64: %{{.+}} = addrspacecast i32 addrspace(270)* %i to i32*
 // X86: %{{.+}} = addrspacecast i32* %i to i32 addrspace(272)*
   f->p64 = i;
   use_foo(f);
 }
 void test_zero_ext(struct Foo *f, int * __ptr32 __uptr i) {
-// X64-LABEL: define dso_local void @test_zero_ext({{.*}}i32 addrspace(271)* writeonly %i)
-// X86-LABEL: define dso_local void @test_zero_ext({{.*}}i32 addrspace(271)* writeonly %i)
+// X64-LABEL: define dso_local void @test_zero_ext({{.*}}i32 addrspace(271)* %i)
+// X86-LABEL: define dso_local void @test_zero_ext({{.*}}i32 addrspace(271)* %i)
 // X64: %{{.+}} = addrspacecast i32 addrspace(271)* %i to i32*
 // X86: %{{.+}} = addrspacecast i32 addrspace(271)* %i to i32 addrspace(272)*
   f->p64 = i;
   use_foo(f);
 }
 void test_trunc(struct Foo *f, int * __ptr64 i) {
-// X64-LABEL: define dso_local void @test_trunc(%struct.Foo* %f, i32* writeonly %i)
-// X86-LABEL: define dso_local void @test_trunc({{.*}}i32 addrspace(272)* writeonly %i)
+// X64-LABEL: define dso_local void @test_trunc(%struct.Foo* %f, i32* %i)
+// X86-LABEL: define dso_local void @test_trunc({{.*}}i32 addrspace(272)* %i)
 // X64: %{{.+}} = addrspacecast i32* %i to i32 addrspace(270)*
 // X86: %{{.+}} = addrspacecast i32 addrspace(272)* %i to i32*
   f->p32 = i;
   use_foo(f);
 }
 void test_noop(struct Foo *f, int * __ptr32 i) {
-// X64-LABEL: define dso_local void @test_noop({{.*}}i32 addrspace(270)* writeonly %i)
-// X86-LABEL: define dso_local void @test_noop({{.*}}i32* writeonly %i)
+// X64-LABEL: define dso_local void @test_noop({{.*}}i32 addrspace(270)* %i)
+// X86-LABEL: define dso_local void @test_noop({{.*}}i32* %i)
 // X64-NOT: addrspacecast
 // X86-NOT: addrspacecast
   f->p32 = i;
@@ -40,8 +40,8 @@ void test_noop(struct Foo *f, int * __ptr32 i) {
 }
 
 void test_other(struct Foo *f, __attribute__((address_space(10))) int *i) {
-// X64-LABEL: define dso_local void @test_other({{.*}}i32 addrspace(10)* writeonly %i)
-// X86-LABEL: define dso_local void @test_other({{.*}}i32 addrspace(10)* writeonly %i)
+// X64-LABEL: define dso_local void @test_other({{.*}}i32 addrspace(10)* %i)
+// X86-LABEL: define dso_local void @test_other({{.*}}i32 addrspace(10)* %i)
 // X64: %{{.+}} = addrspacecast i32 addrspace(10)* %i to i32 addrspace(270)*
 // X86: %{{.+}} = addrspacecast i32 addrspace(10)* %i to i32*
   f->p32 = (int * __ptr32)i;

diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 1ad0055..2cee9c0 100644 (file)
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -769,6 +769,10 @@ determinePointerAccessAttrs(Argument *A,
       break;
 
     case Instruction::Store:
+      if (cast<StoreInst>(I)->getValueOperand() == *U)
+        // untrackable capture
+        return Attribute::None;
+
       // A volatile store has side effects beyond what writeonly can be relied
       // upon.
       if (cast<StoreInst>(I)->isVolatile())

diff --git a/llvm/test/Feature/OperandBundles/pr26510.ll b/llvm/test/Feature/OperandBundles/pr26510.ll
index 8786286..1877f35 100644 (file)
--- a/llvm/test/Feature/OperandBundles/pr26510.ll
+++ b/llvm/test/Feature/OperandBundles/pr26510.ll
@@ -10,7 +10,7 @@
 
 declare void @foo() readnone
 
-; CHECK-LABEL: define i8* @test(i8* writeonly %p)
+; CHECK-LABEL: define i8* @test(i8* %p)
 ; CHECK:   %a = alloca i8*, align 8
 ; CHECK:   store i8* %p, i8** %a, align 8
 ; CHECK:   call void @foo() [ "abc"(i8** %a) ]

diff --git a/llvm/test/Transforms/Coroutines/coro-async.ll b/llvm/test/Transforms/Coroutines/coro-async.ll
index 86d95c3..151573f 100644 (file)
--- a/llvm/test/Transforms/Coroutines/coro-async.ll
+++ b/llvm/test/Transforms/Coroutines/coro-async.ll
@@ -256,7 +256,7 @@ entry:
   unreachable
 }
 
-; CHECK-LABEL: define swiftcc void @my_async_function2(%async.task* %task, %async.actor* %actor, i8* writeonly %async.ctxt)
+; CHECK-LABEL: define swiftcc void @my_async_function2(%async.task* %task, %async.actor* %actor, i8* %async.ctxt)
 ; CHECK-SAME: #[[FRAMEPOINTER:[0-9]+]]
 ; CHECK-SAME: !dbg ![[SP3:[0-9]+]]
 ; CHECK: store i8* %async.ctxt,
@@ -269,7 +269,7 @@ entry:
 ; CHECK: tail call swiftcc void @asyncSuspend(i8* [[CALLEE_CTXT]], %async.task* %task, %async.actor* %actor)
 ; CHECK: ret void
 
-; CHECK-LABEL: define internal swiftcc void @my_async_function2.resume.0(i8* writeonly %0, i8* nocapture readnone %1, i8* nocapture readonly %2)
+; CHECK-LABEL: define internal swiftcc void @my_async_function2.resume.0(i8* %0, i8* nocapture readnone %1, i8* nocapture readonly %2)
 ; CHECK-SAME: #[[FRAMEPOINTER]]
 ; CHECK-SAME: !dbg ![[SP4:[0-9]+]]
 ; CHECK: [[CALLEE_CTXT_ADDR:%.*]] = bitcast i8* %2 to i8**

diff --git a/llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll b/llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll
index e493787..435f781 100644 (file)
--- a/llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll
+++ b/llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll
@@ -7,7 +7,7 @@ define i32* @a(i32** %p) {
        ret i32* %tmp
 }
 
-; CHECK: define i32* @b(i32* writeonly %q)
+; CHECK: define i32* @b(i32* %q)
 define i32* @b(i32 *%q) {
        %mem = alloca i32*
        store i32* %q, i32** %mem

diff --git a/llvm/test/Transforms/FunctionAttrs/nocapture.ll b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
index 5fc33e5..2042f3e 100644 (file)
--- a/llvm/test/Transforms/FunctionAttrs/nocapture.ll
+++ b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
@@ -8,7 +8,7 @@ define i32* @c1(i32* %q) {
        ret i32* %q
 }
 
-; FNATTR: define void @c2(i32* writeonly %q)
+; FNATTR: define void @c2(i32* %q)
 ; It would also be acceptable to mark %q as readnone. Update @c3 too.
 define void @c2(i32* %q) {
        store i32* %q, i32** @g
@@ -268,7 +268,7 @@ entry:
 }
 
 @g3 = global i8* null
-; FNATTR: define void @captureStrip(i8* writeonly %p)
+; FNATTR: define void @captureStrip(i8* %p)
 define void @captureStrip(i8* %p) {
   %b = call i8* @llvm.strip.invariant.group.p0i8(i8* %p)
   store i8* %b, i8** @g3

diff --git a/llvm/test/Transforms/FunctionAttrs/readattrs.ll b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
index e300b85..59f58bd 100644 (file)
--- a/llvm/test/Transforms/FunctionAttrs/readattrs.ll
+++ b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
@@ -34,7 +34,7 @@ define void @test4_2(i8* %p) {
   ret void
 }
 
-; CHECK: define void @test5(i8** nocapture writeonly %p, i8* writeonly %q)
+; CHECK: define void @test5(i8** nocapture writeonly %p, i8* %q)
 ; Missed optz'n: we could make %q readnone, but don't break test6!
 define void @test5(i8** %p, i8* %q) {
   store i8* %q, i8** %p
@@ -42,7 +42,7 @@ define void @test5(i8** %p, i8* %q) {
 }
 
 declare void @test6_1()
-; CHECK: define void @test6_2(i8** nocapture writeonly %p, i8* writeonly %q)
+; CHECK: define void @test6_2(i8** nocapture writeonly %p, i8* %q)
 ; This is not a missed optz'n.
 define void @test6_2(i8** %p, i8* %q) {
   store i8* %q, i8** %p

diff --git a/llvm/test/Transforms/FunctionAttrs/writeonly.ll b/llvm/test/Transforms/FunctionAttrs/writeonly.ll
index ac3b5ff..fb39111 100644 (file)
--- a/llvm/test/Transforms/FunctionAttrs/writeonly.ll
+++ b/llvm/test/Transforms/FunctionAttrs/writeonly.ll
@@ -31,6 +31,15 @@ define void @test_store(i8* %p) {
   ret void
 }
 
+@G = external global i8*
+; CHECK: define i8 @test_store_capture(i8* %p)
+define i8 @test_store_capture(i8* %p) {
+  store i8* %p, i8** @G
+  %p2 = load i8*, i8** @G
+  %v = load i8, i8* %p2
+  ret i8 %v
+}
+
 ; CHECK: define void @test_addressing(i8* nocapture writeonly %p)
 define void @test_addressing(i8* %p) {
   %gep = getelementptr i8, i8* %p, i64 8