[PATCH] D136512: [AA] Allow for flow-sensitive analyses.

[David Goldblatt via Phabricator via llvm-commits]

davidtgoldblatt added inline comments.


================
Comment at: llvm/include/llvm/Analysis/CFLSteensAliasAnalysis.h:80
     if (isa<Constant>(LocA.Ptr) && isa<Constant>(LocB.Ptr))
-      return AAResultBase::alias(LocA, LocB, AAQI);
+      return AAResultBase::alias(LocA, LocB, AAQI, nullptr);
 
----------------
nikic wrote:
> Just a note: You pass through CtxI to AAResultBase::alias() in some places, but not in others. Of course, this doesn't make any practical difference, because AAResultBase::alias() is a no-op that ignores all arguments. We should probably still either do one or the other for consistency.
> 
> (I'm kind of wondering if we shouldn't drop all these `AAResultBase` calls and just replace them with the appropriate values. I get the impression that these are causing a lot more confusion than they clarify, because people think they perform recursive AA, which they don't.)
Will fix; and agreed that it's better to just have all these return MayAlias instead of returning the base implementation and doing it indirectly (I think a vestige of when this stuff was CRTP-ified?).

Happy to put up a cleanup diff once it won't trigger any merge conflicts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136512