[GMR] Teach GlobalsModRef to distinguish an important and safe case of
[oota-llvm.git] / lib / Analysis / IPA / GlobalsModRef.cpp
index ae4eafbc77d3feb236690bd69e362007dc047032..77fb41be8e2213eb668c5a7a723652661fee7492 100644 (file)
@@ -16,6 +16,7 @@
 
 #include "llvm/Analysis/Passes.h"
 #include "llvm/ADT/SCCIterator.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/CallGraph.h"
@@ -29,7 +30,7 @@
 #include "llvm/IR/Module.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
-#include <set>
+#include <list>
 using namespace llvm;
 
 #define DEBUG_TYPE "globalsmodref-aa"
@@ -41,53 +42,224 @@ STATISTIC(NumNoMemFunctions, "Number of functions that do not access memory");
 STATISTIC(NumReadMemFunctions, "Number of functions that only read memory");
 STATISTIC(NumIndirectGlobalVars, "Number of indirect global objects");
 
+// An option to enable unsafe alias results from the GlobalsModRef analysis.
+// When enabled, GlobalsModRef will provide no-alias results which in extremely
+// rare cases may not be conservatively correct. In particular, in the face of
+// transforms which cause assymetry between how effective GetUnderlyingObject
+// is for two pointers, it may produce incorrect results.
+//
+// These unsafe results have been returned by GMR for many years without
+// causing significant issues in the wild and so we provide a mechanism to
+// re-enable them for users of LLVM that have a particular performance
+// sensitivity and no known issues. The option also makes it easy to evaluate
+// the performance impact of these results.
+static cl::opt<bool> EnableUnsafeGlobalsModRefAliasResults(
+    "enable-unsafe-globalsmodref-alias-results", cl::init(false), cl::Hidden);
+
 namespace {
-/// FunctionRecord - One instance of this structure is stored for every
-/// function in the program.  Later, the entries for these functions are
-/// removed if the function is found to call an external function (in which
-/// case we know nothing about it.
-struct FunctionRecord {
-  /// GlobalInfo - Maintain mod/ref info for all of the globals without
-  /// addresses taken that are read or written (transitively) by this
-  /// function.
-  std::map<const GlobalValue *, unsigned> GlobalInfo;
-
-  /// MayReadAnyGlobal - May read global variables, but it is not known which.
-  bool MayReadAnyGlobal;
-
-  unsigned getInfoForGlobal(const GlobalValue *GV) const {
-    unsigned Effect = MayReadAnyGlobal ? AliasAnalysis::Ref : 0;
-    std::map<const GlobalValue *, unsigned>::const_iterator I =
-        GlobalInfo.find(GV);
-    if (I != GlobalInfo.end())
-      Effect |= I->second;
-    return Effect;
+/// The mod/ref information collected for a particular function.
+///
+/// We collect information about mod/ref behavior of a function here, both in
+/// general and as pertains to specific globals. We only have this detailed
+/// information when we know *something* useful about the behavior. If we
+/// saturate to fully general mod/ref, we remove the info for the function.
+class FunctionInfo {
+  typedef SmallDenseMap<const GlobalValue *, ModRefInfo, 16> GlobalInfoMapType;
+
+  /// Build a wrapper struct that has 8-byte alignment. All heap allocations
+  /// should provide this much alignment at least, but this makes it clear we
+  /// specifically rely on this amount of alignment.
+  struct LLVM_ALIGNAS(8) AlignedMap {
+    AlignedMap() {}
+    AlignedMap(const AlignedMap &Arg) : Map(Arg.Map) {}
+    GlobalInfoMapType Map;
+  };
+
+  /// Pointer traits for our aligned map.
+  struct AlignedMapPointerTraits {
+    static inline void *getAsVoidPointer(AlignedMap *P) { return P; }
+    static inline AlignedMap *getFromVoidPointer(void *P) {
+      return (AlignedMap *)P;
+    }
+    enum { NumLowBitsAvailable = 3 };
+    static_assert(AlignOf<AlignedMap>::Alignment >= (1 << NumLowBitsAvailable),
+                  "AlignedMap insufficiently aligned to have enough low bits.");
+  };
+
+  /// The bit that flags that this function may read any global. This is
+  /// chosen to mix together with ModRefInfo bits.
+  enum { MayReadAnyGlobal = 4 };
+
+  /// Checks to document the invariants of the bit packing here.
+  static_assert((MayReadAnyGlobal & MRI_ModRef) == 0,
+                "ModRef and the MayReadAnyGlobal flag bits overlap.");
+  static_assert(((MayReadAnyGlobal | MRI_ModRef) >>
+                 AlignedMapPointerTraits::NumLowBitsAvailable) == 0,
+                "Insufficient low bits to store our flag and ModRef info.");
+
+public:
+  FunctionInfo() : Info() {}
+  ~FunctionInfo() {
+    delete Info.getPointer();
+  }
+  // Spell out the copy ond move constructors and assignment operators to get
+  // deep copy semantics and correct move semantics in the face of the
+  // pointer-int pair.
+  FunctionInfo(const FunctionInfo &Arg)
+      : Info(nullptr, Arg.Info.getInt()) {
+    if (const auto *ArgPtr = Arg.Info.getPointer())
+      Info.setPointer(new AlignedMap(*ArgPtr));
+  }
+  FunctionInfo(FunctionInfo &&Arg)
+      : Info(Arg.Info.getPointer(), Arg.Info.getInt()) {
+    Arg.Info.setPointerAndInt(nullptr, 0);
+  }
+  FunctionInfo &operator=(const FunctionInfo &RHS) {
+    delete Info.getPointer();
+    Info.setPointerAndInt(nullptr, RHS.Info.getInt());
+    if (const auto *RHSPtr = RHS.Info.getPointer())
+      Info.setPointer(new AlignedMap(*RHSPtr));
+    return *this;
+  }
+  FunctionInfo &operator=(FunctionInfo &&RHS) {
+    delete Info.getPointer();
+    Info.setPointerAndInt(RHS.Info.getPointer(), RHS.Info.getInt());
+    RHS.Info.setPointerAndInt(nullptr, 0);
+    return *this;
+  }
+
+  /// Returns the \c ModRefInfo info for this function.
+  ModRefInfo getModRefInfo() const {
+    return ModRefInfo(Info.getInt() & MRI_ModRef);
   }
 
-  /// FunctionEffect - Capture whether or not this function reads or writes to
-  /// ANY memory.  If not, we can do a lot of aggressive analysis on it.
-  unsigned FunctionEffect;
+  /// Adds new \c ModRefInfo for this function to its state.
+  void addModRefInfo(ModRefInfo NewMRI) {
+    Info.setInt(Info.getInt() | NewMRI);
+  }
+
+  /// Returns whether this function may read any global variable, and we don't
+  /// know which global.
+  bool mayReadAnyGlobal() const { return Info.getInt() & MayReadAnyGlobal; }
+
+  /// Sets this function as potentially reading from any global.
+  void setMayReadAnyGlobal() { Info.setInt(Info.getInt() | MayReadAnyGlobal); }
+
+  /// Returns the \c ModRefInfo info for this function w.r.t. a particular
+  /// global, which may be more precise than the general information above.
+  ModRefInfo getModRefInfoForGlobal(const GlobalValue &GV) const {
+    ModRefInfo GlobalMRI = mayReadAnyGlobal() ? MRI_Ref : MRI_NoModRef;
+    if (AlignedMap *P = Info.getPointer()) {
+      auto I = P->Map.find(&GV);
+      if (I != P->Map.end())
+        GlobalMRI = ModRefInfo(GlobalMRI | I->second);
+    }
+    return GlobalMRI;
+  }
+
+  /// Add mod/ref info from another function into ours, saturating towards
+  /// MRI_ModRef.
+  void addFunctionInfo(const FunctionInfo &FI) {
+    addModRefInfo(FI.getModRefInfo());
+
+    if (FI.mayReadAnyGlobal())
+      setMayReadAnyGlobal();
+
+    if (AlignedMap *P = FI.Info.getPointer())
+      for (const auto &G : P->Map)
+        addModRefInfoForGlobal(*G.first, G.second);
+  }
+
+  void addModRefInfoForGlobal(const GlobalValue &GV, ModRefInfo NewMRI) {
+    AlignedMap *P = Info.getPointer();
+    if (!P) {
+      P = new AlignedMap();
+      Info.setPointer(P);
+    }
+    auto &GlobalMRI = P->Map[&GV];
+    GlobalMRI = ModRefInfo(GlobalMRI | NewMRI);
+  }
 
-  FunctionRecord() : MayReadAnyGlobal(false), FunctionEffect(0) {}
+  /// Clear a global's ModRef info. Should be used when a global is being
+  /// deleted.
+  void eraseModRefInfoForGlobal(const GlobalValue &GV) {
+    if (AlignedMap *P = Info.getPointer())
+      P->Map.erase(&GV);
+  }
+
+private:
+  /// All of the information is encoded into a single pointer, with a three bit
+  /// integer in the low three bits. The high bit provides a flag for when this
+  /// function may read any global. The low two bits are the ModRefInfo. And
+  /// the pointer, when non-null, points to a map from GlobalValue to
+  /// ModRefInfo specific to that GlobalValue.
+  PointerIntPair<AlignedMap *, 3, unsigned, AlignedMapPointerTraits> Info;
 };
 
 /// GlobalsModRef - The actual analysis pass.
 class GlobalsModRef : public ModulePass, public AliasAnalysis {
-  /// NonAddressTakenGlobals - The globals that do not have their addresses
-  /// taken.
-  std::set<const GlobalValue *> NonAddressTakenGlobals;
+  /// The globals that do not have their addresses taken.
+  SmallPtrSet<const GlobalValue *, 8> NonAddressTakenGlobals;
 
   /// IndirectGlobals - The memory pointed to by this global is known to be
   /// 'owned' by the global.
-  std::set<const GlobalValue *> IndirectGlobals;
+  SmallPtrSet<const GlobalValue *, 8> IndirectGlobals;
 
   /// AllocsForIndirectGlobals - If an instruction allocates memory for an
   /// indirect global, this map indicates which one.
-  std::map<const Value *, const GlobalValue *> AllocsForIndirectGlobals;
+  DenseMap<const Value *, const GlobalValue *> AllocsForIndirectGlobals;
+
+  /// For each function, keep track of what globals are modified or read.
+  DenseMap<const Function *, FunctionInfo> FunctionInfos;
+
+  /// Handle to clear this analysis on deletion of values.
+  struct DeletionCallbackHandle final : CallbackVH {
+    GlobalsModRef &GMR;
+    std::list<DeletionCallbackHandle>::iterator I;
+
+    DeletionCallbackHandle(GlobalsModRef &GMR, Value *V)
+        : CallbackVH(V), GMR(GMR) {}
+
+    void deleted() override {
+      Value *V = getValPtr();
+      if (auto *F = dyn_cast<Function>(V))
+        GMR.FunctionInfos.erase(F);
+
+      if (GlobalValue *GV = dyn_cast<GlobalValue>(V)) {
+        if (GMR.NonAddressTakenGlobals.erase(GV)) {
+          // This global might be an indirect global.  If so, remove it and
+          // remove any AllocRelatedValues for it.
+          if (GMR.IndirectGlobals.erase(GV)) {
+            // Remove any entries in AllocsForIndirectGlobals for this global.
+            for (auto I = GMR.AllocsForIndirectGlobals.begin(),
+                      E = GMR.AllocsForIndirectGlobals.end();
+                 I != E; ++I)
+              if (I->second == GV)
+                GMR.AllocsForIndirectGlobals.erase(I);
+          }
 
-  /// FunctionInfo - For each function, keep track of what globals are
-  /// modified or read.
-  std::map<const Function *, FunctionRecord> FunctionInfo;
+          // Scan the function info we have collected and remove this global
+          // from all of them.
+          for (auto &FIPair : GMR.FunctionInfos)
+            FIPair.second.eraseModRefInfoForGlobal(*GV);
+        }
+      }
+
+      // If this is an allocation related to an indirect global, remove it.
+      GMR.AllocsForIndirectGlobals.erase(V);
+
+      // And clear out the handle.
+      setValPtr(nullptr);
+      GMR.Handles.erase(I);
+      // This object is now destroyed!
+    }
+  };
+
+  /// List of callbacks for globals being tracked by this analysis. Note that
+  /// these objects are quite large, but we only anticipate having one per
+  /// global tracked by this analysis. There are numerous optimizations we
+  /// could perform to the memory utilization here if this becomes a problem.
+  std::list<DeletionCallbackHandle> Handles;
 
 public:
   static char ID;
@@ -112,79 +284,76 @@ public:
     AU.setPreservesAll(); // Does not transform code
   }
 
+  /// getAdjustedAnalysisPointer - This method is used when a pass implements
+  /// an analysis interface through multiple inheritance.  If needed, it
+  /// should override this to adjust the this pointer as needed for the
+  /// specified pass info.
+  void *getAdjustedAnalysisPointer(AnalysisID PI) override {
+    if (PI == &AliasAnalysis::ID)
+      return (AliasAnalysis *)this;
+    return this;
+  }
+
   //------------------------------------------------
   // Implement the AliasAnalysis API
   //
   AliasResult alias(const MemoryLocation &LocA,
                     const MemoryLocation &LocB) override;
-  ModRefResult getModRefInfo(ImmutableCallSite CS,
-                             const MemoryLocation &Loc) override;
-  ModRefResult getModRefInfo(ImmutableCallSite CS1,
-                             ImmutableCallSite CS2) override {
+  ModRefInfo getModRefInfo(ImmutableCallSite CS,
+                           const MemoryLocation &Loc) override;
+  ModRefInfo getModRefInfo(ImmutableCallSite CS1,
+                           ImmutableCallSite CS2) override {
     return AliasAnalysis::getModRefInfo(CS1, CS2);
   }
 
   /// getModRefBehavior - Return the behavior of the specified function if
   /// called from the specified call site.  The call site may be null in which
   /// case the most generic behavior of this function should be returned.
-  ModRefBehavior getModRefBehavior(const Function *F) override {
-    ModRefBehavior Min = UnknownModRefBehavior;
-
-    if (FunctionRecord *FR = getFunctionInfo(F)) {
-      if (FR->FunctionEffect == 0)
-        Min = DoesNotAccessMemory;
-      else if ((FR->FunctionEffect & Mod) == 0)
-        Min = OnlyReadsMemory;
+  FunctionModRefBehavior getModRefBehavior(const Function *F) override {
+    FunctionModRefBehavior Min = FMRB_UnknownModRefBehavior;
+
+    if (FunctionInfo *FI = getFunctionInfo(F)) {
+      if (FI->getModRefInfo() == MRI_NoModRef)
+        Min = FMRB_DoesNotAccessMemory;
+      else if ((FI->getModRefInfo() & MRI_Mod) == 0)
+        Min = FMRB_OnlyReadsMemory;
     }
 
-    return ModRefBehavior(AliasAnalysis::getModRefBehavior(F) & Min);
+    return FunctionModRefBehavior(AliasAnalysis::getModRefBehavior(F) & Min);
   }
 
   /// getModRefBehavior - Return the behavior of the specified function if
   /// called from the specified call site.  The call site may be null in which
   /// case the most generic behavior of this function should be returned.
-  ModRefBehavior getModRefBehavior(ImmutableCallSite CS) override {
-    ModRefBehavior Min = UnknownModRefBehavior;
+  FunctionModRefBehavior getModRefBehavior(ImmutableCallSite CS) override {
+    FunctionModRefBehavior Min = FMRB_UnknownModRefBehavior;
 
     if (const Function *F = CS.getCalledFunction())
-      if (FunctionRecord *FR = getFunctionInfo(F)) {
-        if (FR->FunctionEffect == 0)
-          Min = DoesNotAccessMemory;
-        else if ((FR->FunctionEffect & Mod) == 0)
-          Min = OnlyReadsMemory;
+      if (FunctionInfo *FI = getFunctionInfo(F)) {
+        if (FI->getModRefInfo() == MRI_NoModRef)
+          Min = FMRB_DoesNotAccessMemory;
+        else if ((FI->getModRefInfo() & MRI_Mod) == 0)
+          Min = FMRB_OnlyReadsMemory;
       }
 
-    return ModRefBehavior(AliasAnalysis::getModRefBehavior(CS) & Min);
-  }
-
-  void deleteValue(Value *V) override;
-  void addEscapingUse(Use &U) override;
-
-  /// getAdjustedAnalysisPointer - This method is used when a pass implements
-  /// an analysis interface through multiple inheritance.  If needed, it
-  /// should override this to adjust the this pointer as needed for the
-  /// specified pass info.
-  void *getAdjustedAnalysisPointer(AnalysisID PI) override {
-    if (PI == &AliasAnalysis::ID)
-      return (AliasAnalysis *)this;
-    return this;
+    return FunctionModRefBehavior(AliasAnalysis::getModRefBehavior(CS) & Min);
   }
 
 private:
-  /// getFunctionInfo - Return the function info for the function, or null if
-  /// we don't have anything useful to say about it.
-  FunctionRecord *getFunctionInfo(const Function *F) {
-    std::map<const Function *, FunctionRecord>::iterator I =
-        FunctionInfo.find(F);
-    if (I != FunctionInfo.end())
+  /// Returns the function info for the function, or null if we don't have
+  /// anything useful to say about it.
+  FunctionInfo *getFunctionInfo(const Function *F) {
+    auto I = FunctionInfos.find(F);
+    if (I != FunctionInfos.end())
       return &I->second;
     return nullptr;
   }
 
   void AnalyzeGlobals(Module &M);
   void AnalyzeCallGraph(CallGraph &CG, Module &M);
-  bool AnalyzeUsesOfPointer(Value *V, std::vector<Function *> &Readers,
-                            std::vector<Function *> &Writers,
+  bool AnalyzeUsesOfPointer(Value *V,
+                            SmallPtrSetImpl<Function *> *Readers = nullptr,
+                            SmallPtrSetImpl<Function *> *Writers = nullptr,
                             GlobalValue *OkayStoreDest = nullptr);
   bool AnalyzeIndirectGlobalMemory(GlobalValue *GV);
 };
@@ -206,30 +375,44 @@ Pass *llvm::createGlobalsModRefPass() { return new GlobalsModRef(); }
 /// (really, their address passed to something nontrivial), record this fact,
 /// and record the functions that they are used directly in.
 void GlobalsModRef::AnalyzeGlobals(Module &M) {
-  std::vector<Function *> Readers, Writers;
+  SmallPtrSet<Function *, 64> TrackedFunctions;
   for (Function &F : M)
-    if (F.hasLocalLinkage()) {
-      if (!AnalyzeUsesOfPointer(&F, Readers, Writers)) {
+    if (F.hasLocalLinkage())
+      if (!AnalyzeUsesOfPointer(&F)) {
         // Remember that we are tracking this global.
         NonAddressTakenGlobals.insert(&F);
+        TrackedFunctions.insert(&F);
+        Handles.emplace_front(*this, &F);
+        Handles.front().I = Handles.begin();
         ++NumNonAddrTakenFunctions;
       }
-      Readers.clear();
-      Writers.clear();
-    }
 
+  SmallPtrSet<Function *, 64> Readers, Writers;
   for (GlobalVariable &GV : M.globals())
     if (GV.hasLocalLinkage()) {
-      if (!AnalyzeUsesOfPointer(&GV, Readers, Writers)) {
+      if (!AnalyzeUsesOfPointer(&GV, &Readers,
+                                GV.isConstant() ? nullptr : &Writers)) {
         // Remember that we are tracking this global, and the mod/ref fns
         NonAddressTakenGlobals.insert(&GV);
+        Handles.emplace_front(*this, &GV);
+        Handles.front().I = Handles.begin();
 
-        for (Function *Reader : Readers)
-          FunctionInfo[Reader].GlobalInfo[&GV] |= Ref;
+        for (Function *Reader : Readers) {
+          if (TrackedFunctions.insert(Reader).second) {
+            Handles.emplace_front(*this, Reader);
+            Handles.front().I = Handles.begin();
+          }
+          FunctionInfos[Reader].addModRefInfoForGlobal(GV, MRI_Ref);
+        }
 
         if (!GV.isConstant()) // No need to keep track of writers to constants
-          for (Function *Writer : Writers)
-            FunctionInfo[Writer].GlobalInfo[&GV] |= Mod;
+          for (Function *Writer : Writers) {
+            if (TrackedFunctions.insert(Writer).second) {
+              Handles.emplace_front(*this, Writer);
+              Handles.front().I = Handles.begin();
+            }
+            FunctionInfos[Writer].addModRefInfoForGlobal(GV, MRI_Mod);
+          }
         ++NumNonAddrTakenGlobalVars;
 
         // If this global holds a pointer type, see if it is an indirect global.
@@ -249,8 +432,8 @@ void GlobalsModRef::AnalyzeGlobals(Module &M) {
 ///
 /// If OkayStoreDest is non-null, stores into this global are allowed.
 bool GlobalsModRef::AnalyzeUsesOfPointer(Value *V,
-                                         std::vector<Function *> &Readers,
-                                         std::vector<Function *> &Writers,
+                                         SmallPtrSetImpl<Function *> *Readers,
+                                         SmallPtrSetImpl<Function *> *Writers,
                                          GlobalValue *OkayStoreDest) {
   if (!V->getType()->isPointerTy())
     return true;
@@ -258,10 +441,12 @@ bool GlobalsModRef::AnalyzeUsesOfPointer(Value *V,
   for (Use &U : V->uses()) {
     User *I = U.getUser();
     if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
-      Readers.push_back(LI->getParent()->getParent());
+      if (Readers)
+        Readers->insert(LI->getParent()->getParent());
     } else if (StoreInst *SI = dyn_cast<StoreInst>(I)) {
       if (V == SI->getOperand(1)) {
-        Writers.push_back(SI->getParent()->getParent());
+        if (Writers)
+          Writers->insert(SI->getParent()->getParent());
       } else if (SI->getOperand(1) != OkayStoreDest) {
         return true; // Storing the pointer
       }
@@ -276,10 +461,12 @@ bool GlobalsModRef::AnalyzeUsesOfPointer(Value *V,
       // passing into the function.
       if (!CS.isCallee(&U)) {
         // Detect calls to free.
-        if (isFreeCall(I, TLI))
-          Writers.push_back(CS->getParent()->getParent());
-        else
+        if (isFreeCall(I, TLI)) {
+          if (Writers)
+            Writers->insert(CS->getParent()->getParent());
+        } else {
           return true; // Argument of an unknown call.
+        }
       }
     } else if (ICmpInst *ICI = dyn_cast<ICmpInst>(I)) {
       if (!isa<ConstantPointerNull>(ICI->getOperand(1)))
@@ -311,8 +498,7 @@ bool GlobalsModRef::AnalyzeIndirectGlobalMemory(GlobalValue *GV) {
       // The pointer loaded from the global can only be used in simple ways:
       // we allow addressing of it and loading storing to it.  We do *not* allow
       // storing the loaded pointer somewhere else or passing to a function.
-      std::vector<Function *> ReadersWriters;
-      if (AnalyzeUsesOfPointer(LI, ReadersWriters, ReadersWriters))
+      if (AnalyzeUsesOfPointer(LI))
         return false; // Loaded pointer escapes.
       // TODO: Could try some IP mod/ref of the loaded pointer.
     } else if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
@@ -333,8 +519,8 @@ bool GlobalsModRef::AnalyzeIndirectGlobalMemory(GlobalValue *GV) {
 
       // Analyze all uses of the allocation.  If any of them are used in a
       // non-simple way (e.g. stored to another global) bail out.
-      std::vector<Function *> ReadersWriters;
-      if (AnalyzeUsesOfPointer(Ptr, ReadersWriters, ReadersWriters, GV))
+      if (AnalyzeUsesOfPointer(Ptr, /*Readers*/ nullptr, /*Writers*/ nullptr,
+                               GV))
         return false; // Loaded pointer escapes.
 
       // Remember that this allocation is related to the indirect global.
@@ -349,9 +535,13 @@ bool GlobalsModRef::AnalyzeIndirectGlobalMemory(GlobalValue *GV) {
   // this global in AllocsForIndirectGlobals.
   while (!AllocRelatedValues.empty()) {
     AllocsForIndirectGlobals[AllocRelatedValues.back()] = GV;
+    Handles.emplace_front(*this, AllocRelatedValues.back());
+    Handles.front().I = Handles.begin();
     AllocRelatedValues.pop_back();
   }
   IndirectGlobals.insert(GV);
+  Handles.emplace_front(*this, GV);
+  Handles.front().I = Handles.begin();
   return true;
 }
 
@@ -370,14 +560,12 @@ void GlobalsModRef::AnalyzeCallGraph(CallGraph &CG, Module &M) {
       // Calls externally - can't say anything useful.  Remove any existing
       // function records (may have been created when scanning globals).
       for (auto *Node : SCC)
-        FunctionInfo.erase(Node->getFunction());
+        FunctionInfos.erase(Node->getFunction());
       continue;
     }
 
-    FunctionRecord &FR = FunctionInfo[SCC[0]->getFunction()];
-
+    FunctionInfo &FI = FunctionInfos[SCC[0]->getFunction()];
     bool KnowNothing = false;
-    unsigned FunctionEffect = 0;
 
     // Collect the mod/ref properties due to called functions.  We only compute
     // one mod-ref set.
@@ -393,13 +581,13 @@ void GlobalsModRef::AnalyzeCallGraph(CallGraph &CG, Module &M) {
         if (F->doesNotAccessMemory()) {
           // Can't do better than that!
         } else if (F->onlyReadsMemory()) {
-          FunctionEffect |= Ref;
+          FI.addModRefInfo(MRI_Ref);
           if (!F->isIntrinsic())
             // This function might call back into the module and read a global -
             // consider every global as possibly being read by this function.
-            FR.MayReadAnyGlobal = true;
+            FI.setMayReadAnyGlobal();
         } else {
-          FunctionEffect |= ModRef;
+          FI.addModRefInfo(MRI_ModRef);
           // Can't say anything useful unless it's an intrinsic - they don't
           // read or write global variables of the kind considered here.
           KnowNothing = !F->isIntrinsic();
@@ -410,14 +598,9 @@ void GlobalsModRef::AnalyzeCallGraph(CallGraph &CG, Module &M) {
       for (CallGraphNode::iterator CI = SCC[i]->begin(), E = SCC[i]->end();
            CI != E && !KnowNothing; ++CI)
         if (Function *Callee = CI->second->getFunction()) {
-          if (FunctionRecord *CalleeFR = getFunctionInfo(Callee)) {
+          if (FunctionInfo *CalleeFI = getFunctionInfo(Callee)) {
             // Propagate function effect up.
-            FunctionEffect |= CalleeFR->FunctionEffect;
-
-            // Incorporate callee's effects on globals into our info.
-            for (const auto &G : CalleeFR->GlobalInfo)
-              FR.GlobalInfo[G.first] |= G.second;
-            FR.MayReadAnyGlobal |= CalleeFR->MayReadAnyGlobal;
+            FI.addFunctionInfo(*CalleeFI);
           } else {
             // Can't say anything about it.  However, if it is inside our SCC,
             // then nothing needs to be done.
@@ -431,49 +614,57 @@ void GlobalsModRef::AnalyzeCallGraph(CallGraph &CG, Module &M) {
     }
 
     // If we can't say anything useful about this SCC, remove all SCC functions
-    // from the FunctionInfo map.
+    // from the FunctionInfos map.
     if (KnowNothing) {
       for (auto *Node : SCC)
-        FunctionInfo.erase(Node->getFunction());
+        FunctionInfos.erase(Node->getFunction());
       continue;
     }
 
     // Scan the function bodies for explicit loads or stores.
-    for (unsigned i = 0, e = SCC.size(); i != e && FunctionEffect != ModRef;
-         ++i)
-      for (inst_iterator II = inst_begin(SCC[i]->getFunction()),
-                         E = inst_end(SCC[i]->getFunction());
-           II != E && FunctionEffect != ModRef; ++II)
-        if (LoadInst *LI = dyn_cast<LoadInst>(&*II)) {
-          FunctionEffect |= Ref;
-          if (LI->isVolatile())
-            // Volatile loads may have side-effects, so mark them as writing
-            // memory (for example, a flag inside the processor).
-            FunctionEffect |= Mod;
-        } else if (StoreInst *SI = dyn_cast<StoreInst>(&*II)) {
-          FunctionEffect |= Mod;
-          if (SI->isVolatile())
-            // Treat volatile stores as reading memory somewhere.
-            FunctionEffect |= Ref;
-        } else if (isAllocationFn(&*II, TLI) || isFreeCall(&*II, TLI)) {
-          FunctionEffect |= ModRef;
-        } else if (IntrinsicInst *Intrinsic = dyn_cast<IntrinsicInst>(&*II)) {
-          // The callgraph doesn't include intrinsic calls.
-          Function *Callee = Intrinsic->getCalledFunction();
-          ModRefBehavior Behaviour = AliasAnalysis::getModRefBehavior(Callee);
-          FunctionEffect |= (Behaviour & ModRef);
+    for (auto *Node : SCC) {
+      if (FI.getModRefInfo() == MRI_ModRef)
+        break; // The mod/ref lattice saturates here.
+      for (Instruction &I : inst_range(Node->getFunction())) {
+        if (FI.getModRefInfo() == MRI_ModRef)
+          break; // The mod/ref lattice saturates here.
+
+        // We handle calls specially because the graph-relevant aspects are
+        // handled above.
+        if (auto CS = CallSite(&I)) {
+          if (isAllocationFn(&I, TLI) || isFreeCall(&I, TLI)) {
+            // FIXME: It is completely unclear why this is necessary and not
+            // handled by the above graph code.
+            FI.addModRefInfo(MRI_ModRef);
+          } else if (Function *Callee = CS.getCalledFunction()) {
+            // The callgraph doesn't include intrinsic calls.
+            if (Callee->isIntrinsic()) {
+              FunctionModRefBehavior Behaviour =
+                  AliasAnalysis::getModRefBehavior(Callee);
+              FI.addModRefInfo(ModRefInfo(Behaviour & MRI_ModRef));
+            }
+          }
+          continue;
         }
 
-    if ((FunctionEffect & Mod) == 0)
+        // All non-call instructions we use the primary predicates for whether
+        // thay read or write memory.
+        if (I.mayReadFromMemory())
+          FI.addModRefInfo(MRI_Ref);
+        if (I.mayWriteToMemory())
+          FI.addModRefInfo(MRI_Mod);
+      }
+    }
+
+    if ((FI.getModRefInfo() & MRI_Mod) == 0)
       ++NumReadMemFunctions;
-    if (FunctionEffect == 0)
+    if (FI.getModRefInfo() == MRI_NoModRef)
       ++NumNoMemFunctions;
-    FR.FunctionEffect = FunctionEffect;
 
     // Finally, now that we know the full effect on this SCC, clone the
     // information to each function in the SCC.
     for (unsigned i = 1, e = SCC.size(); i != e; ++i)
-      FunctionInfo[SCC[i]->getFunction()] = FR;
+      FunctionInfos[SCC[i]->getFunction()] = FI;
   }
 }
 
@@ -499,10 +690,63 @@ AliasResult GlobalsModRef::alias(const MemoryLocation &LocA,
       GV2 = nullptr;
 
     // If the two pointers are derived from two different non-addr-taken
-    // globals, or if one is and the other isn't, we know these can't alias.
-    if ((GV1 || GV2) && GV1 != GV2)
+    // globals we know these can't alias.
+    if (GV1 && GV2 && GV1 != GV2)
       return NoAlias;
 
+    // If one is and the other isn't, it isn't strictly safe but we can fake
+    // this result if necessary for performance. This does not appear to be
+    // a common problem in practice.
+    if (EnableUnsafeGlobalsModRefAliasResults)
+      if ((GV1 || GV2) && GV1 != GV2)
+        return NoAlias;
+
+    // There are particular cases where we can conclude no-alias between
+    // a non-addr-taken global and some other underlying object. Specifically,
+    // a non-addr-taken global is known to not be escaped from any function. It
+    // is also incorrect for a transformation to introduce an escape of
+    // a global in a way that is observable when it was not there previously.
+    // One function being transformed to introduce an escape which could
+    // possibly be observed (via loading from a global or the return value for
+    // example) within another function is never safe. If the observation is
+    // made through non-atomic operations on different threads, it is
+    // a data-race and UB. If the observation is well defined, by being
+    // observed the transformation would have changed program behavior by
+    // introducing the observed escape, making it an invalid transform.
+    //
+    // This property does require that transformations which *temporarily*
+    // escape a global that was not previously escaped, prior to restoring
+    // it, cannot rely on the results of GMR::alias. This seems a reasonable
+    // restriction, although currently there is no way to enforce it. There is
+    // also no realistic optimization pass that would make this mistake. The
+    // closest example is a transformation pass which does reg2mem of SSA
+    // values but stores them into global variables temporarily before
+    // restoring the global variable's value. This could be useful to expose
+    // "benign" races for example. However, it seems reasonable to require that
+    // a pass which introduces escapes of global variables in this way to
+    // either not trust AA results while the escape is active, or to be forced
+    // to operate as a module pass that cannot co-exist with an alias analysis
+    // such as GMR.
+    if ((GV1 || GV2) && GV1 != GV2) {
+      const Value *UV = GV1 ? UV2 : UV1;
+
+      // In order to know that the underlying object cannot alias the
+      // non-addr-taken global, we must know that it would have to be an
+      // escape. Thus if the underlying object is a function argument, a load
+      // from a global, or the return of a function, it cannot alias.
+      if (isa<Argument>(UV) || isa<CallInst>(UV) || isa<InvokeInst>(UV)) {
+        // Arguments to functions or returns from functions are inherently
+        // escaping, so we can immediately classify those as not aliasing any
+        // non-addr-taken globals.
+        return NoAlias;
+      } else if (auto *LI = dyn_cast<LoadInst>(UV)) {
+        // A pointer loaded from a global would have been captured, and we know
+        // that GV is non-addr-taken, so no alias.
+        if (isa<GlobalValue>(LI->getPointerOperand()))
+          return NoAlias;
+      }
+    }
+
     // Otherwise if they are both derived from the same addr-taken global, we
     // can't know the two accesses don't overlap.
   }
@@ -522,24 +766,30 @@ AliasResult GlobalsModRef::alias(const MemoryLocation &LocA,
 
   // These pointers may also be from an allocation for the indirect global.  If
   // so, also handle them.
-  if (AllocsForIndirectGlobals.count(UV1))
-    GV1 = AllocsForIndirectGlobals[UV1];
-  if (AllocsForIndirectGlobals.count(UV2))
-    GV2 = AllocsForIndirectGlobals[UV2];
+  if (!GV1)
+    GV1 = AllocsForIndirectGlobals.lookup(UV1);
+  if (!GV2)
+    GV2 = AllocsForIndirectGlobals.lookup(UV2);
 
   // Now that we know whether the two pointers are related to indirect globals,
-  // use this to disambiguate the pointers.  If either pointer is based on an
-  // indirect global and if they are not both based on the same indirect global,
-  // they cannot alias.
-  if ((GV1 || GV2) && GV1 != GV2)
+  // use this to disambiguate the pointers. If the pointers are based on
+  // different indirect globals they cannot alias.
+  if (GV1 && GV2 && GV1 != GV2)
     return NoAlias;
 
+  // If one is based on an indirect global and the other isn't, it isn't
+  // strictly safe but we can fake this result if necessary for performance.
+  // This does not appear to be a common problem in practice.
+  if (EnableUnsafeGlobalsModRefAliasResults)
+    if ((GV1 || GV2) && GV1 != GV2)
+      return NoAlias;
+
   return AliasAnalysis::alias(LocA, LocB);
 }
 
-AliasAnalysis::ModRefResult
-GlobalsModRef::getModRefInfo(ImmutableCallSite CS, const MemoryLocation &Loc) {
-  unsigned Known = ModRef;
+ModRefInfo GlobalsModRef::getModRefInfo(ImmutableCallSite CS,
+                                        const MemoryLocation &Loc) {
+  unsigned Known = MRI_ModRef;
 
   // If we are asking for mod/ref info of a direct call with a pointer to a
   // global we are tracking, return information if we have it.
@@ -549,51 +799,10 @@ GlobalsModRef::getModRefInfo(ImmutableCallSite CS, const MemoryLocation &Loc) {
     if (GV->hasLocalLinkage())
       if (const Function *F = CS.getCalledFunction())
         if (NonAddressTakenGlobals.count(GV))
-          if (const FunctionRecord *FR = getFunctionInfo(F))
-            Known = FR->getInfoForGlobal(GV);
-
-  if (Known == NoModRef)
-    return NoModRef; // No need to query other mod/ref analyses
-  return ModRefResult(Known & AliasAnalysis::getModRefInfo(CS, Loc));
-}
-
-//===----------------------------------------------------------------------===//
-// Methods to update the analysis as a result of the client transformation.
-//
-void GlobalsModRef::deleteValue(Value *V) {
-  if (GlobalValue *GV = dyn_cast<GlobalValue>(V)) {
-    if (NonAddressTakenGlobals.erase(GV)) {
-      // This global might be an indirect global.  If so, remove it and remove
-      // any AllocRelatedValues for it.
-      if (IndirectGlobals.erase(GV)) {
-        // Remove any entries in AllocsForIndirectGlobals for this global.
-        for (std::map<const Value *, const GlobalValue *>::iterator
-                 I = AllocsForIndirectGlobals.begin(),
-                 E = AllocsForIndirectGlobals.end();
-             I != E;) {
-          if (I->second == GV) {
-            AllocsForIndirectGlobals.erase(I++);
-          } else {
-            ++I;
-          }
-        }
-      }
-    }
-  }
-
-  // Otherwise, if this is an allocation related to an indirect global, remove
-  // it.
-  AllocsForIndirectGlobals.erase(V);
-
-  AliasAnalysis::deleteValue(V);
-}
-
-void GlobalsModRef::addEscapingUse(Use &U) {
-  // For the purposes of this analysis, it is conservatively correct to treat
-  // a newly escaping value equivalently to a deleted one.  We could perhaps
-  // be more precise by processing the new use and attempting to update our
-  // saved analysis results to accommodate it.
-  deleteValue(U);
+          if (const FunctionInfo *FI = getFunctionInfo(F))
+            Known = FI->getModRefInfoForGlobal(*GV);
 
-  AliasAnalysis::addEscapingUse(U);
+  if (Known == MRI_NoModRef)
+    return MRI_NoModRef; // No need to query other mod/ref analyses
+  return ModRefInfo(Known & AliasAnalysis::getModRefInfo(CS, Loc));
 }