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

[bryant via Phabricator via llvm-commits]

bryant added inline comments.


================
Comment at: test/Analysis/BasicAA/getforargument-crash.ll:13
+  ret void
+}
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D28394