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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

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

> In D151644#4384923 <https://reviews.llvm.org/D151644#4384923>, @goldstein.w.n wrote:
>
>> 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'm not entirely confident on this, but I believe it can also happen with noalias returns, aka allocators. If you write the pointer into a malloc that gets leaked, I believe that would be non-capturing, as nobody has provenance to access the captured pointer anymore.

Ah that's an interesting case. I'll cover that as well (just disable if any allocator function).

>> 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'd say this doesn't violate nocapture on `@test`, for the reason you state.
>
>> 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.
>
> Right, I don't think we can do the speculation optimization. In fact, it might be that the address has already been captured prior to the call and `return_ptr_as_i64` returns it (neither nocapture nor noalias on the argument prevent this).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151644