[PATCH] D17329: [AA] Hoist the logic to reformulate various AA queries in terms of other parts of the AA interface out of the base class of every single AA result object.

[Chandler Carruth via llvm-commits]

chandlerc added inline comments.

================
Comment at: lib/Analysis/AliasAnalysis.cpp:139
@@ +138,3 @@
+  // aggregate set of AA results.
+  auto MRB = getModRefBehavior(CS);
+  if (MRB == FMRB_DoesNotAccessMemory)
----------------
Gerolf wrote:
> This routine queries AA again but this time for behavior rather than ModRefInfo, but the result (no mod ref info) could be the same. Can this be combined in some meta routine so that no mod ref is spit out early? The AA loops can be very compile-time expensive so we should avoid/shorten them when possible.
This is interesting, but seems really orthogonal to this change. I don't want to try to fix everything in one go, I'm trying to fix a specific problem with the existing implementation.

================
Comment at: lib/Analysis/AliasAnalysis.cpp:149
@@ +148,3 @@
+    ModRefInfo AllArgsMask = MRI_NoModRef;
+    if (doesAccessArgPointees(MRB)) {
+      for (auto AI = CS.arg_begin(), AE = CS.arg_end(); AI != AE; ++AI) {
----------------
Gerolf wrote:
> Could you refactor into a function?
This is directly moving code from one place to another here. I don't really want to refactor all of it to look different at the same time. This applies to most of the remaining comments as well -- all of this code existed pretty much exactly as-is before. But if there is a desire to refactor this, I think that should be a separate patch.

================
Comment at: lib/Analysis/AliasAnalysis.cpp:171
@@ +170,3 @@
+  // modify the memory location.
+  if ((Result & MRI_Mod) &&
+      pointsToConstantMemory(Loc, /*OrLocal*/ false))
----------------
Gerolf wrote:
> Why could Mod be set in this case? Wouldn't that be a bug?
If this is a bug, it is a bug in existing code, so you should be able to find a test case and fix it separately from this patch?


http://reviews.llvm.org/D17329