[PATCH] D28394: [AliasAnalysis] Limit `MemoryLocation` retrieval to valid intrinsic arguments.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: test/Analysis/BasicAA/getforargument-crash.ll:13
+  ret void
+}
----------------
bryant wrote:
> hfinkel wrote:
> > Please also add a test case with a regular external function with a readnone argument, showing that it does not alias with some other load/store to that same address.
> Attempting to construct this second test makes me doubt whether short-circuiting readnone arguments is the right fix. I've tried this IR:
> 
> ```
> declare void @f(i8* readnone) argmemonly
> 
> define i8 @leave_readnone_untouched() {
>   %a = alloca i8
>   store i8 undef, i8* %a
>   call void @f(i8* %a)
>   %rv = load i8, i8* %a
>   ret i8 %rv
> }
> ```
> 
> With the patch, AA won't bother to examine f's argument, and conclude that `NoModRef:  Ptr: i8* %a        <->  call void @f(i8* %a)`.
> 
> Without the patch, AA would examine the argument and call `BasicAAResult::getModRefInfo(CS = f, ArgIdx = 0)`, which already possesses the savvy to return NoModRef on account of the readnone, and ultimately conclude the same result as before.
> 
> In light of this, what should the proper fix now be? It seems to me that the crash in my first test case purely results from the idiosyncrasies of the invariant intrinsics. In fact, they're the only ones in LangRef which use {}*, so it shouldn't be reproducible with any other intrinsic or function call.
Okay; maybe I misunderstood the problem. Why would this crash previously upon trying to get the MRI on the intrinsic's argument? Is it because of the type of the pointer?



Repository:
  rL LLVM

https://reviews.llvm.org/D28394