[PATCH] LLVM CFL Alias Analysis -- Algorithm

[hfinkel at anl.gov]

I agree with Nick; once current comments are addressed, I think this can go in and we'll work on it from there.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:11
@@ +10,3 @@
+// This file implements a CFL-based context-insensitive alias analysis
+// algorithm. It does not depend on types The algorithm is a mixture of the one
+// described in "Demand-driven alias analysis for C" by Xin Zheng and Radu
----------------
Missing '.' after types.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:63
@@ +62,3 @@
+template <typename Inst>
+static bool getPossibleTargets(Inst *, SmallVectorImpl<Function *> &);
+
----------------
I think this is fine, but FYI, there is another way of doing it. Instead of taking an Inst* you could take a const CallSite & (which can automatically be constructed by either). If someone else has a preference here, feel free to say so.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:95
@@ +94,3 @@
+// weight (Assign has Assign; Reference has Dereference).
+enum class EdgeWeight {
+  // The weight assigned when assigning from or to a value. For example, in:
----------------
I don't like the term "weight" here -- the different weights here are really unordered edge types. Can we call this EdgeType instead?

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:220
@@ +219,3 @@
+
+    if (isa<Constant>(LocA.Ptr) && isa<Constant>(LocB.Ptr)) {
+      return MayAlias;
----------------
I think this deserves a comment. This will catch all GlobalVariable comparisons. Maybe something like:
 // Comparisons between global variables and other constants should be handled by BasicAA.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:415
@@ +414,3 @@
+      // extermely similar, and equally correct, results either way)
+      for (unsigned I = 0; I < Arguments.size(); ++I) {
+        auto &MainInfo = Parameters[I];
----------------
It feels like there should be an early-backedge in this loop for parameters that are noalias/byval somewhere.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:678
@@ +677,3 @@
+      // We don't want the edges of most "return" instructions, but we *do* want
+      // to know what all can be returned.
+      if (auto *Ret = dyn_cast<ReturnInst>(&Inst))
----------------
Remove "all"

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:793
@@ +792,3 @@
+//===----------------------------------------------------------------------===//
+// Lengthy CFL AA methods
+//===----------------------------------------------------------------------===//
----------------
Nick Lewycky wrote:
> "Lengthy"?
Also, what is lengthy about them? This is really the query interface.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:798
@@ +797,3 @@
+  (void)DummyPair;
+  assert(DummyPair.second);
+
----------------
It is not obvious what this assert means. Please make this:
  assert(DummyPair.second && "Here is what this means");

http://reviews.llvm.org/D4551