[PATCH] D29989: [BasicAA] Take attributes into account when requesting modref info for a call site

[Igor Laevsky via Phabricator via llvm-commits]

igor-laevsky added inline comments.


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:886
   // The AAResultBase base class has some smarts, lets use them.
-  return AAResultBase::getModRefInfo(CS, Loc);
+  return static_cast<ModRefInfo>(Result & AAResultBase::getModRefInfo(CS, Loc));
 }
----------------
dberlin wrote:
> Uh?
Idea here is to intersect smarts from AAResultBase with the result we've received in this function. I'm actually not certain if this is needed since "AAResults" does intersect results from all alias analyses. Plus AAResultBase currently doesn't have any smarts. So, yes, I'm not sure if we need this operation.


================
Comment at: test/Transforms/NewGVN/readattrs.ll:1-3
-; XFAIL: *
 ; RUN: opt -newgvn -S -o - < %s | FileCheck %s
 
----------------
davide wrote:
> The fact this test passes now is actually good, but I'd like to understand why it was failing before and now it's not. Can you please elaborate?
This test passes local non-escaping object as a readonly nocapture function argument. Before my change alias analysis would have ignored readonly argument and concluded that call modifies "%a". I suspect that old GVN might use some internal smarts to determine that "%a" is unmodified by the call, while NewGVN relies on the alias analysis.


https://reviews.llvm.org/D29989