[PM/AA] Rework the names and comments in AliasSetTracker to more accurately describe what is being tracked.

[chandlerc]

Summary

While these two enums do track mod/ref information and aliasing
information, they don't represent the exact same things as either the
mod/ref enums or the alias result enum in AA. They're definitions are
dominated by the structure of their lattice and the bit's various
semantics. This patch just calls them what they are and tries to spell
out usefully distinct names for these things.

This will clear the path for using a raw unscoped enum to represent some
of these concepts across LLVM's analysis library.

No functionality changed here.

[chandlerc]

Ping... anyone? should be a dirt simple review...

[• dberlin]

This looks great to me. I think it makes what is going on much cleaer.
Thanks for taking the time.

[chandlerc]

Thanks Danny, submitting.

llvm/trunk/include/llvm/Analysis/AliasSetTracker.h

class AliasSet : public ilist_node<AliasSet> {
 
   // All instructions without a specific address in this alias set.
   std::vector<AssertingVH<Instruction> > UnknownInsts;
 
   // RefCount - Number of nodes pointing to this AliasSet plus the number of
   // AliasSets forwarding to it.
   unsigned RefCount : 28;
 
-  /// AccessType - Keep track of whether this alias set merely refers to the
-  /// locations of memory, whether it modifies the memory, or whether it does
-  /// both.  The lattice goes from "NoModRef" to either Refs or Mods, then to
-  /// ModRef as necessary.
+  /// The kinds of access this alias set models.
   ///
-  enum AccessType {
-    NoModRef = 0, Refs = 1,         // Ref = bit 1
-    Mods     = 2, ModRef = 3        // Mod = bit 2
+  /// We keep track of whether this alias set merely refers to the locations of
+  /// memory (and not any particular access), whether it modifies or references
+  /// the memory, or whether it does both. The lattice goes from "NoAccess" to
+  /// either RefAccess or ModAccess, then to ModRefAccess as necessary.
+  enum AccessLattice {
+    NoAccess = 0,
+    RefAccess = 1,
+    ModAccess = 2,
+    ModRefAccess = RefAccess | ModAccess
   };
-  unsigned AccessTy : 2;
+  unsigned Access : 2;
 
-  /// AliasType - Keep track the relationships between the pointers in the set.
-  /// Lattice goes from MustAlias to MayAlias.
+  /// The kind of alias relationship between pointers of the set.
   ///
-  enum AliasType {
-    MustAlias = 0, MayAlias = 1
+  /// These represent conservatively correct alias results between any members
+  /// of the set. We represent these independently of the values of alias
+  /// results in order to pack it into a single bit. Lattice goes from
+  /// MustAlias to MayAlias.
+  enum AliasLattice {
+    SetMustAlias = 0, SetMayAlias = 1
   };
-  unsigned AliasTy : 1;
+  unsigned Alias : 1;
 
   // Volatile - True if this alias set contains volatile loads or stores.
   bool Volatile : 1;
 
   void addRef() { ++RefCount; }
   void dropRef(AliasSetTracker &AST) {
     assert(RefCount >= 1 && "Invalid reference count detected!");
     if (--RefCount == 0)
       removeFromTracker(AST);
   }
 
   Instruction *getUnknownInst(unsigned i) const {
     assert(i < UnknownInsts.size());
     return UnknownInsts[i];
   }
   
 public:
   /// Accessors...
-  bool isRef() const { return AccessTy & Refs; }
-  bool isMod() const { return AccessTy & Mods; }
-  bool isMustAlias() const { return AliasTy == MustAlias; }
-  bool isMayAlias()  const { return AliasTy == MayAlias; }
+  bool isRef() const { return Access & RefAccess; }
+  bool isMod() const { return Access & ModAccess; }
+  bool isMustAlias() const { return Alias == SetMustAlias; }
+  bool isMayAlias()  const { return Alias == SetMayAlias; }
 
   // isVolatile - Return true if this alias set contains volatile loads or
   // stores.
   bool isVolatile() const { return Volatile; }
 
   /// isForwardingAliasSet - Return true if this alias set should be ignored as
   /// part of the AliasSetTracker object.
   bool isForwardingAliasSet() const { return Forward; }
public:
   };
 
 private:
   // Can only be created by AliasSetTracker. Also, ilist creates one
   // to serve as a sentinel.
   friend struct ilist_sentinel_traits<AliasSet>;
   AliasSet()
     : PtrList(nullptr), PtrListEnd(&PtrList), Forward(nullptr), RefCount(0),
-      AccessTy(NoModRef), AliasTy(MustAlias), Volatile(false) {
+      Access(NoAccess), Alias(SetMustAlias), Volatile(false) {
   }
 
   AliasSet(const AliasSet &AS) = delete;
   void operator=(const AliasSet &AS) = delete;
 
   PointerRec *getSomePointer() const {
     return PtrList;
   }
private:
   AliasSet::PointerRec &getEntryFor(Value *V) {
     AliasSet::PointerRec *&Entry = PointerMap[ASTCallbackVH(V, this)];
     if (!Entry)
       Entry = new AliasSet::PointerRec(V);
     return *Entry;
   }
 
   AliasSet &addPointer(Value *P, uint64_t Size, const AAMDNodes &AAInfo,
-                       AliasSet::AccessType E,
+                       AliasSet::AccessLattice E,
                        bool &NewSet) {
     NewSet = false;
     AliasSet &AS = getAliasSetForPointer(P, Size, AAInfo, &NewSet);
-    AS.AccessTy |= E;
+    AS.Access |= E;
     return AS;
   }
   AliasSet *findAliasSetForPointer(const Value *Ptr, uint64_t Size,
                                    const AAMDNodes &AAInfo);
 
   AliasSet *findAliasSetForUnknownInst(Instruction *Inst);
 };
 
 inline raw_ostream& operator<<(raw_ostream &OS, const AliasSetTracker &AST) {
   AST.print(OS);
   return OS;
 }
 
 } // namespace llvm
 
 #endif

llvm/trunk/lib/Analysis/AliasSetTracker.cpp

 
 /// mergeSetIn - Merge the specified alias set into this alias set.
 ///
 void AliasSet::mergeSetIn(AliasSet &AS, AliasSetTracker &AST) {
   assert(!AS.Forward && "Alias set is already forwarding!");
   assert(!Forward && "This set is a forwarding set!!");
 
   // Update the alias and access types of this set...
-  AccessTy |= AS.AccessTy;
-  AliasTy  |= AS.AliasTy;
+  Access |= AS.Access;
+  Alias  |= AS.Alias;
   Volatile |= AS.Volatile;
 
-  if (AliasTy == MustAlias) {
+  if (Alias == SetMustAlias) {
     // Check that these two merged sets really are must aliases.  Since both
     // used to be must-alias sets, we can just check any pointer from each set
     // for aliasing.
     AliasAnalysis &AA = AST.getAliasAnalysis();
     PointerRec *L = getSomePointer();
     PointerRec *R = AS.getSomePointer();
 
     // If the pointers are not a must-alias pair, this set becomes a may alias.
     if (AA.alias(MemoryLocation(L->getValue(), L->getSize(), L->getAAInfo()),
                  MemoryLocation(R->getValue(), R->getSize(), R->getAAInfo())) !=
         AliasAnalysis::MustAlias)
-      AliasTy = MayAlias;
+      Alias = SetMayAlias;
   }
 
   bool ASHadUnknownInsts = !AS.UnknownInsts.empty();
   if (UnknownInsts.empty()) {            // Merge call sites...
     if (ASHadUnknownInsts) {
       std::swap(UnknownInsts, AS.UnknownInsts);
       addRef();
     }
void AliasSet::addPointer(AliasSetTracker &AST, PointerRec &Entry,
   // Check to see if we have to downgrade to _may_ alias.
   if (isMustAlias() && !KnownMustAlias)
     if (PointerRec *P = getSomePointer()) {
       AliasAnalysis &AA = AST.getAliasAnalysis();
       AliasAnalysis::AliasResult Result =
           AA.alias(MemoryLocation(P->getValue(), P->getSize(), P->getAAInfo()),
                    MemoryLocation(Entry.getValue(), Size, AAInfo));
       if (Result != AliasAnalysis::MustAlias)
-        AliasTy = MayAlias;
+        Alias = SetMayAlias;
       else                  // First entry of must alias must have maximum size!
         P->updateSizeAndAAInfo(Size, AAInfo);
       assert(Result != AliasAnalysis::NoAlias && "Cannot be part of must set!");
     }
 
   Entry.setAliasSet(this);
   Entry.updateSizeAndAAInfo(Size, AAInfo);
 
   // Add it to the end of the list...
   assert(*PtrListEnd == nullptr && "End of list is not null?");
   *PtrListEnd = &Entry;
   PtrListEnd = Entry.setPrevInList(PtrListEnd);
   assert(*PtrListEnd == nullptr && "End of list is not null?");
   addRef();               // Entry points to alias set.
 }
 
 void AliasSet::addUnknownInst(Instruction *I, AliasAnalysis &AA) {
   if (UnknownInsts.empty())
     addRef();
   UnknownInsts.emplace_back(I);
 
   if (!I->mayWriteToMemory()) {
-    AliasTy = MayAlias;
-    AccessTy |= Refs;
+    Alias = SetMayAlias;
+    Access |= RefAccess;
     return;
   }
 
   // FIXME: This should use mod/ref information to make this not suck so bad
-  AliasTy = MayAlias;
-  AccessTy = ModRef;
+  Alias = SetMayAlias;
+  Access = ModRefAccess;
 }
 
 /// aliasesPointer - Return true if the specified pointer "may" (or must)
 /// alias one of the members in the set.
 ///
 bool AliasSet::aliasesPointer(const Value *Ptr, uint64_t Size,
                               const AAMDNodes &AAInfo,
                               AliasAnalysis &AA) const {
-  if (AliasTy == MustAlias) {
+  if (Alias == SetMustAlias) {
     assert(UnknownInsts.empty() && "Illegal must alias set!");
 
     // If this is a set of MustAliases, only check to see if the pointer aliases
     // SOME value in the set.
     PointerRec *SomePtr = getSomePointer();
     assert(SomePtr && "Empty must-alias set??");
     return AA.alias(MemoryLocation(SomePtr->getValue(), SomePtr->getSize(),
                                    SomePtr->getAAInfo()),
AliasSet &AliasSetTracker::getAliasSetForPointer(Value *Pointer, uint64_t Size,
   // Otherwise create a new alias set to hold the loaded pointer.
   AliasSets.push_back(new AliasSet());
   AliasSets.back().addPointer(*this, Entry, Size, AAInfo);
   return AliasSets.back();
 }
 
 bool AliasSetTracker::add(Value *Ptr, uint64_t Size, const AAMDNodes &AAInfo) {
   bool NewPtr;
-  addPointer(Ptr, Size, AAInfo, AliasSet::NoModRef, NewPtr);
+  addPointer(Ptr, Size, AAInfo, AliasSet::NoAccess, NewPtr);
   return NewPtr;
 }
 
 
 bool AliasSetTracker::add(LoadInst *LI) {
   if (LI->getOrdering() > Monotonic) return addUnknown(LI);
 
   AAMDNodes AAInfo;
   LI->getAAMetadata(AAInfo);
 
-  AliasSet::AccessType ATy = AliasSet::Refs;
+  AliasSet::AccessLattice Access = AliasSet::RefAccess;
   bool NewPtr;
   AliasSet &AS = addPointer(LI->getOperand(0),
                             AA.getTypeStoreSize(LI->getType()),
-                            AAInfo, ATy, NewPtr);
+                            AAInfo, Access, NewPtr);
   if (LI->isVolatile()) AS.setVolatile();
   return NewPtr;
 }
 
 bool AliasSetTracker::add(StoreInst *SI) {
   if (SI->getOrdering() > Monotonic) return addUnknown(SI);
 
   AAMDNodes AAInfo;
   SI->getAAMetadata(AAInfo);
 
-  AliasSet::AccessType ATy = AliasSet::Mods;
+  AliasSet::AccessLattice Access = AliasSet::ModAccess;
   bool NewPtr;
   Value *Val = SI->getOperand(0);
   AliasSet &AS = addPointer(SI->getOperand(1),
                             AA.getTypeStoreSize(Val->getType()),
-                            AAInfo, ATy, NewPtr);
+                            AAInfo, Access, NewPtr);
   if (SI->isVolatile()) AS.setVolatile();
   return NewPtr;
 }
 
 bool AliasSetTracker::add(VAArgInst *VAAI) {
   AAMDNodes AAInfo;
   VAAI->getAAMetadata(AAInfo);
 
   bool NewPtr;
   addPointer(VAAI->getOperand(0), MemoryLocation::UnknownSize, AAInfo,
-             AliasSet::ModRef, NewPtr);
+             AliasSet::ModRefAccess, NewPtr);
   return NewPtr;
 }
 
 
 bool AliasSetTracker::addUnknown(Instruction *Inst) {
   if (isa<DbgInfoIntrinsic>(Inst)) 
     return true; // Ignore DbgInfo Intrinsics.
   if (!Inst->mayReadOrWriteMemory())
for (const_iterator I = AST.begin(), E = AST.end(); I != E; ++I) {
     for (unsigned i = 0, e = AS.UnknownInsts.size(); i != e; ++i)
       add(AS.UnknownInsts[i]);
 
     // Loop over all of the pointers in this alias set.
     bool X;
     for (AliasSet::iterator ASI = AS.begin(), E = AS.end(); ASI != E; ++ASI) {
       AliasSet &NewAS = addPointer(ASI.getPointer(), ASI.getSize(),
                                    ASI.getAAInfo(),
-                                   (AliasSet::AccessType)AS.AccessTy, X);
+                                   (AliasSet::AccessLattice)AS.Access, X);
       if (AS.isVolatile()) NewAS.setVolatile();
     }
   }
 }
 
 /// remove - Remove the specified (potentially non-empty) alias set from the
 /// tracker.
 void AliasSetTracker::remove(AliasSet &AS) {

 
 
 //===----------------------------------------------------------------------===//
 //               AliasSet/AliasSetTracker Printing Support
 //===----------------------------------------------------------------------===//
 
 void AliasSet::print(raw_ostream &OS) const {
   OS << "  AliasSet[" << (const void*)this << ", " << RefCount << "] ";
-  OS << (AliasTy == MustAlias ? "must" : "may") << " alias, ";
-  switch (AccessTy) {
-  case NoModRef: OS << "No access "; break;
-  case Refs    : OS << "Ref       "; break;
-  case Mods    : OS << "Mod       "; break;
-  case ModRef  : OS << "Mod/Ref   "; break;
-  default: llvm_unreachable("Bad value for AccessTy!");
+  OS << (Alias == SetMustAlias ? "must" : "may") << " alias, ";
+  switch (Access) {
+  case NoAccess:     OS << "No access "; break;
+  case RefAccess:    OS << "Ref       "; break;
+  case ModAccess:    OS << "Mod       "; break;
+  case ModRefAccess: OS << "Mod/Ref   "; break;
+  default: llvm_unreachable("Bad value for Access!");
   }
   if (isVolatile()) OS << "[volatile] ";
   if (Forward)
     OS << " forwarding to " << (void*)Forward;
 
 
   if (!empty()) {
     OS << "Pointers: ";