[PATCH] D73428: [Attributor] Improve `noalias` deduction based on memory information

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert marked 3 inline comments as done.
jdoerfert added a subscriber: hfinkel.
jdoerfert added a comment.

In D73428#1878746 <https://reviews.llvm.org/D73428#1878746>, @uenoku wrote:

> And I have a question.
>
>   define i32 @f1(i32* %p, i32* %q){
>   entry:
>     %0 = load i32, i32* %q
>     %1 = load i32, i32* %p
>     %add = add nsw i32 %1, %0
>     ret i32 %add
>   }
>  
>   define i32 @f2(i32* nocapture readonly %p) {
>   entry:
>     %call = tail call i32 @f1(i32* %p, i32* %p)
>     ret i32 %call
>   }
>
>
> In this case, `%call = tail call i32 @f1(i32* noalias %p, i32* noalias %p)` is deduced because they have only "read-read" dependencies.
>  I agree that this is a sound deduction since there is no write. (I know Rust compiler is doing similar things)
>
> However, looking back to the definition of `noalias` in LangRef, I think this annotation violates the definition. Is it allowed conventionally?


If this would violate the LangRef we would have a problem. I don't think it does but I think we need to make that explicit. I'll talk to @hfinkel and come back with lang ref patch or a revised version of this ;)



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2772-2774
+    // If the definition is not `noalias` we need to check globals in addition
+    // to arguments. If the definition is `noalias` we cannot alias globals to
+    // begin with.
----------------
baziotis wrote:
> jdoerfert wrote:
> > baziotis wrote:
> > > Why do we know that?
> > If we have already `noalias` (or an alloca) we know we do not alias any access in the scope not derived from this pointer. Since globals cannot be derived from a `noalias` pointer (in a way that would be problematic here) or in any way from an alloca, we know the global does not alias the pointer.
> Thanks for the explanation! The parts "a scope is derived from a pointer" (e.g. in the scope not derived from this pointer) are still unclear to me. What do I miss? Is there a specific part of code / doc I can read?
I think the actually relevant documentation is https://en.cppreference.com/w/c/language/restrict


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2769
+  /// potential aliases to the underlying call site argument. That is, if the
+  /// call site argument does not alias any argument or global which is accessed
+  /// by the callee it can be marked noalias.
----------------
uenoku wrote:
> baziotis wrote:
> > uenoku wrote:
> > > Don't we need to look at constant memories?
> > So, no malloced memory?
> I think malloced memory cannot be seen from the caller so we don't have to care about here.
Correct. Constant memory does not change, so it's the read only argument again. Malloced memory, local memory, inaccessible memory, cannot "alias" with the arguments we pass at the call site by design.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2924
+        AAMemoryLocation::ALL_LOCATIONS | AAMemoryLocation::NO_LOCAL_MEM |
+        AAMemoryLocation::NO_INACCESSIBLE_MEM;
+
----------------
uenoku wrote:
> baziotis wrote:
> > uenoku wrote:
> > > How about `NO_MALLOCED_MEM`?
> > > 
> > Why not check for malloced memory? Although I think this goes along with your comment about constant memories.
> I think malloced mem cannot be accessed from the caller but constant memory can be accessed.
Constant memory is read only, read-read argument. Malloced memory is memory that was returned by a `noalias` function. It does not alias anything not derived from that pointer. If the call was made in this function (the callee wrt AANoAliasCallSiteArgument) then it cannot alias an argument of this function since they were around before the call was made :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73428