[PATCH] D93039: Introduce llvm.noalias.decl intrinsic

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/docs/LangRef.rst:19506
+care must be taken to duplicate and uniquify the associated scope when the loop
+is unrolled. Otherwise the restrict scope could spill across iterations.
 
----------------
nikic wrote:
> I think adding an example for this intrinsic would be helpful. I'm thinking something along these lines:
> 
> ```
> ; This examples shows two possible positions for noalias.decl:
> ; If it is outside the loop (version 1), then %a and %b are noalias across *all* iterations.
> ; If it is inside the loop (version 2), then %a and %b are noalias only within *one* iteration.
> declare void @decl_in_loop(i8* %a.base, i8* %b.base) {
> entry:
>   ; call void @llvm.noalias.decl(metadata !2) ; Version 1: noalias.decl outside loop
>   br label %loop
> 
> loop:
>   %a = phi i8* [ %a.base, %entry ], [ %a.inc, %loop ]
>   %b = phi i8* [ %b.base, %entry ], [ %b.inc, %loop ]
>   ; call void @llvm.noalias.decl(metadata !2) ; Version 2: noalias.decl inside loop
>   %val = load i8, i8* %a, !alias.scope !2
>   store i8 %val, i8* %b, !noalias !2
>   %a.inc = getelementptr inbounds i8, i8* %a, i64 1
>   %b.inc = getelementptr inbounds i8, i8* %a, i64 1
>   %cond = call i1 @cond()
>   br i1 %cond, label %loop, label %exit
> 
> exit:
>   ret void
> }
> 
> !0 = !{!0} ; domain
> !1 = !{!1, !0} ; scope
> !2 = !{!1} ; scope list
> ```
Maybe:
_ exact location
+ scope

----

I don't like the loop sentence. Isn't the real problem that we need to duplicate + uniquify whenever we "duplicate" a noalias decl? So for inlining it is the same as for loop unrolling, isn't it? Loop unrolling can be mentioned afterwards as an example.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:556
+                [llvm_anyptr_ty, llvm_anyint_ty, llvm_metadata_ty],
+                [IntrArgMemOnly]>; // ArgMemOnly: blocks LICM and some more
+
----------------
jeroen.dobbelaere wrote:
> nikic wrote:
> > The ArgMemOnly here seems a bit dubious. This means it can read/write the p.alloca argument, which I assume is not intended (even if it's unused now).
> > 
> > I guess we can't make this `NoMem`, because then it could simply be DCEd right? Maybe it should be InaccessibleMemOnly?
> That might also work.
I don't think it should be DCEd if used. `NoMem` seems logical to me.


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

https://reviews.llvm.org/D93039