[PATCH] D71617: [WIP][NFC][Attributor] noalias attribute deduction fixme

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D71617#1912380 <https://reviews.llvm.org/D71617#1912380>, @pgode wrote:

> In D71617#1911979 <https://reviews.llvm.org/D71617#1911979>, @jdoerfert wrote:
>
> > @pgode Are you still looking into this? Do you need feedback from me?
>
>
> I apologize for not coming back on this as I got tied up with other tasks.


No worries.

> Thank you for patiently clarifying things so far. 
>  I agree to the changes which you suggested but I couldn't create a new test which passes due to this change.
> 
> As you had pointed that I was not clear in my logic. Major reason for that was I had incomplete understanding of fixpoint iteration and data flow analysis concepts, which I could get a better understanding now.
>  Please guide me more on examples or if you think I should work on something else if there is better solution with someone.
> 
> Bit more detail on the examples (I am referring) are:
> 
> 1. I started with wrong example. In the code below, I was questioning noalias attribute propagation to strtox. Here the log message clearly showed that the pointer 's' gets captured by 'sh_fromstring' callsite in strtox.
> 
>   ``` ; Function Attrs: nounwind optsize define dso_local float @strtof(i8* noalias %s, i8** noalias %p) #0 { entry: %call = tail call fastcc double @strtox(i8* %s, i8** %p, i32 0) #3 %conv = fptrunc double %call to float ret float %conv }
> 
>   ; Function Attrs: nounwind optsize define internal fastcc double @strtox(i8* %s, i8** %p, i32 %prec) unnamed_addr #0 { entry: %f = alloca %struct._IO_FILE, align 8 %0 = bitcast %struct._IO_FILE* %f to i8* call void @llvm.lifetime.start.p0i8(i64 144, i8* nonnull %0) #4 %call = call i32 bitcast (i32 (...)* @sh_fromstring to i32 (%struct._IO_FILE*, i8*)*)(%struct._IO_FILE* nonnull %f, i8* %s) #5 call void @__shlim(%struct._IO_FILE* nonnull %f, i64 0) #5 %call1 = call double @__floatscan(%struct._IO_FILE* nonnull %f, i32 %prec, i32 1) #5 %shcnt = getelementptr inbounds %struct._IO_FILE, %struct._IO_FILE* %f, i32 0, i32 30 %1 = load i64, i64* %shcnt, align 8, !tbaa !2 ... } ; Function Attrs: nounwind optsize define dso_local double @strtold(i8* noalias %s, i8** noalias %p) #0 { entry: %call = tail call fastcc double @strtox(i8* %s, i8** %p, i32 2) #3 ret double %call } ```

I think we should be able to propagate `noalias` to `%p` in `@strtox`. Please add this as a test.

> 2. In the other example as well I got it wrong when I was questioning that the noalias attribute should propagate callsite argument %a to test10_helper_1(). This can't happen because the test10_helper_1 is not defined thus we can't determine noalias correctly even though test10_helper_1 callsite is reachable.
> 
>   ``` TEST 10 declare void @test10_helper_1(i8* %a) define void @test10_helper_2(i8* noalias %a) { tail call void @test10_helper_1(i8* %a) ret void } define void @test10(i8* noalias %a) { tail call void @test10_helper_1(i8* %a) tail call void @test10_helper_2(i8* %a) ret void } ```

I think both call sites of `@test10_helper_1` can have `noalias`. In both cases there is no escaping use that reaches the call site (except the call site itself).



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2465
+    auto UsePred = [&](const Use &U, bool &Follow) -> bool {
+      Instruction *UserI = cast<Instruction>(U.getUser());
+      const Function *ScopeFn = IRP.getAnchorScope();
----------------
I think you need to allow `UserI` to be `getCtx()`. That is, if the user is the current instruction we ignore the use but follow if there are subsequent uses!


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

https://reviews.llvm.org/D71617