[PATCH] D11707: [GMR] Teach the conservative path of GMR to catch even more easy cases.

[Chandler Carruth]

chandlerc added a comment.

Updated, and responses to review comments below...


================
Comment at: lib/Analysis/IPA/GlobalsModRef.cpp:698
@@ +697,3 @@
+// with an alias analysis such as GMR.
+bool GlobalsModRef::isNonEscapingGlobalNoAlias(const GlobalValue *GV,
+                                               const Value *V) {
----------------
majnemer wrote:
> Where is `GV` used?
Sorry, I was trying to use GV to do some stuff that ended up with the FIXME comment instead. =] Based on Michael's comment, I'll add in the stub that uses it around the FIXME comment, which should also make it much more clear how to safely implement that FIXME.

================
Comment at: lib/Analysis/IPA/GlobalsModRef.cpp:762
@@ +761,3 @@
+
+    // Unknown instruction, bail.
+    return false;
----------------
mkuper wrote:
> It looks like this also covers the case where we end up reaching GV itself in the recursion. Perhaps make that explicit?
Yea, that also gives me a much better place to anchor the FIXME above. Should make sure that we don't introduce a bug about the same GV when addressing that FIXME.

================
Comment at: lib/Analysis/IPA/GlobalsModRef.cpp:806
@@ -741,1 +805,3 @@
+    if ((GV1 || GV2) && GV1 != GV2)
+      if (isNonEscapingGlobalNoAlias(GV1 ? GV1 : GV2, GV1 ? UV2 : UV1))
         return NoAlias;
----------------
mkuper wrote:
> This looks a bit confusing.
> Can we turn this into the more standard "if (!GV1) swap..." pattern?
I really don't like the swap pattern... Does doing this in separate variable initializers make it more clear? (Patch updated to do that...)


http://reviews.llvm.org/D11707