[PATCH] D67272: [Attributor] Add NoCaptureCallSiteArgument in default

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D67272#1661250 <https://reviews.llvm.org/D67272#1661250>, @uenoku wrote:

> In D67272#1661149 <https://reviews.llvm.org/D67272#1661149>, @jdoerfert wrote:
>
> > Do we really need this? I mean, you can query the information and the attribute is created on-demand, right?
> >
> > On second thought, it makes sense to have it so I'm fine with this but still would like to know the answer to the question above.
>
>
> I had the same opinion but I found that testing with on-demand creation might be confusing in some cases.
>  In D67286 <https://reviews.llvm.org/D67286>, I added this test.
>
>   declare void @use_nocapture(i8* nocapture)
>   declare void @use(i8*)
>   define void @test12_2(){
>     %A = tail call noalias i8* @malloc(i64 4)
>     tail call void @use_nocapture(i8* %A)
>     tail call void @use_nocapture(i8* %A)
>     tail call void @use(i8* %A)
>     tail call void @use_nocapture(i8* %A)
>     ret void
>   }
>
>
> Without this patch, the result is
>
>   define void @test12_2(){
>     %A = tail call noalias i8* @malloc(i64 4)
>     tail call void @use_nocapture(i8* nocapture %A) ; (i)
>     tail call void @use_nocapture(i8* nocapture %A) ; (ii)
>     tail call void @use(i8* %A) ; (iii)
>     tail call void @use_nocapture(i8* %A) ; (iv)
>     ret void
>   }
>
>
> I couldn't understand easily why `nocapture` in not in (iv). It is because that CaptureTracker queries for (i), (ii)  and then AANoCapture are created at that time. But CaptureTracker stops to query at (iii). Therefore, AANoCapture is not created in (iv).
>
> I know it is useful only in debugging or testing. Maybe it is better to add a flag for creating this in default. What do you think?


There is a problem, not with this example but once there is control flow involved. I have to revisit the capture tracker solution anyway for other reasons and then I'll fix this as well.


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

https://reviews.llvm.org/D67272