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

[Hideto Ueno via Phabricator via llvm-commits]

uenoku added a comment.

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?


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

https://reviews.llvm.org/D67272