[PATCH] D151644: [InstCombine] Propegating `nocapture` flag to callsites

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D151644#4384102 <https://reviews.llvm.org/D151644#4384102>, @nikic wrote:

> propegate -> propagate everywhere
>
> You're really playing with fire here when it comes to "it's captured, but it doesn't really matter" reasoning. Consider something like this:
>
>   define i32 @test(ptr noalias nocapture %p) {
>     %a = alloca ptr
>     store i32 0, ptr %p 
>     call void @store_into_alloca(ptr %a, ptr %p)
>     %p2 = load ptr, ptr %a
>     %v = load i32, ptr %p2
>     ret i32 %v
>   }
>   
>   declare void @store_into_alloca(ptr, ptr)
>
> This call is clearly capturing, in a way that will affect alias analysis results (`load i32, ptr %p2` cannot ref `%p` with that `nocapture` attribute in place, which will be materialized with invalid alias scope metadata during inlining).

Alright fair enough. I think this can only occur for allocas as any other type of store with no more may-write instructions would actually capture.
I guess we also need to handle returns in a similiar way. You could do:

  > define i32 @test(ptr noalias nocapture %p) {
  >   %p2 = call ptr @return_ptr(ptr %p)
  >   %v = load i32, ptr %p2
  >   ret i32 %v
  > }

where `p2` captures and it would also be an alias analysis issue.
So the rule needs to update.
I don't think we need so go so far as "any load after makes it unsafe"
although maybe that does need to be case. How about the following case:

>   define i32 @test(ptr noalias nocapture %p, ptr noalias nocapture %q) {
>     %a = alloca ptr
>     store ptr %a, ptr %q 
>     call void @store_p_into_alloca_in_q(ptr %q, ptr %p)
>     %p3 = load ptr, ptr %q
>     %p2 = load ptr, ptr %p3
>     %v = load i32, ptr %p2
>     ret i32 %v
>   }

so `%p` is stored into `%a` through `%q` that is obviously a capture, but does that
violate the `nocapture` in `@test` or because the temporary pointer that capture
`%p` is no longer accesible after `@test` is it safe?

I was also thinking something like:

  > define void @test(ptr noalias nocapture %p) {
  >   %p_i64 = call ptr @return_ptr_as_i64(ptr %p)
  >   %p2 = ptrtoint ptr %p to i64
  >   %d = sub i64 %p_64, %p2
  >   %unused = udiv i64 %n, %d
  >   ret void
  > }

Might be an issue as was thinking if `@return_ptr_as_i64` can't capture then the return
value must != `%p` so the `udiv` is speculatable. But I think its safe to return random
from a function that has a nocapture argument so its possible to get the exact bit pattern
of the pointer, it just cant be derived from the pointer itself.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151644/new/

https://reviews.llvm.org/D151644