[AST] Don't merge memory locations in AliasSetTracker (#65731)

This changes the AliasSetTracker to track memory locations instead of
pointers in its alias sets. The motivation for this is outlined in an RFC
posted on LLVM discourse:
https://discourse.llvm.org/t/rfc-dont-merge-memory-locations-in-aliassettracker/73336

In the data structures of the AST implementation, I made the choice to
replace the linked list of `PointerRec` entries (that had to go anyway)
with a simple flat vector of `MemoryLocation` objects, but for the
`AliasSet` objects referenced from a lookup table, I retained the
mechanism of a linked list, reference counting, forwarding, etc. The
data structures could be revised in a follow-up change.

Author: brunodf-snps

llvm/include/llvm/Analysis/AliasSetTracker.h

@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 //
 // This file defines two classes: AliasSetTracker and AliasSet. These interfaces
-// are used to classify a collection of pointer references into a maximal number
+// are used to classify a collection of memory locations into a maximal number
 // of disjoint sets. Each AliasSet object constructed by the AliasSetTracker
 // object refers to memory disjoint from the other sets.
 //
@@ -19,16 +19,14 @@
 #define LLVM_ANALYSIS_ALIASSETTRACKER_H
 
 #include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/ilist.h"
 #include "llvm/ADT/ilist_node.h"
 #include "llvm/Analysis/MemoryLocation.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/ValueHandle.h"
 #include <cassert>
-#include <cstddef>
-#include <iterator>
 #include <vector>
 
 namespace llvm {
@@ -49,99 +47,12 @@ class Value;
 class AliasSet : public ilist_node<AliasSet> {
   friend class AliasSetTracker;
 
-  class PointerRec {
-    Value *Val;  // The pointer this record corresponds to.
-    PointerRec **PrevInList = nullptr;
-    PointerRec *NextInList = nullptr;
-    AliasSet *AS = nullptr;
-    LocationSize Size = LocationSize::mapEmpty();
-    AAMDNodes AAInfo;
-
-    // Whether the size for this record has been set at all. This makes no
-    // guarantees about the size being known.
-    bool isSizeSet() const { return Size != LocationSize::mapEmpty(); }
-
-  public:
-    PointerRec(Value *V)
-      : Val(V), AAInfo(DenseMapInfo<AAMDNodes>::getEmptyKey()) {}
-
-    Value *getValue() const { return Val; }
-
-    PointerRec *getNext() const { return NextInList; }
-    bool hasAliasSet() const { return AS != nullptr; }
-
-    PointerRec** setPrevInList(PointerRec **PIL) {
-      PrevInList = PIL;
-      return &NextInList;
-    }
-
-    bool updateSizeAndAAInfo(LocationSize NewSize, const AAMDNodes &NewAAInfo) {
-      bool SizeChanged = false;
-      if (NewSize != Size) {
-        LocationSize OldSize = Size;
-        Size = isSizeSet() ? Size.unionWith(NewSize) : NewSize;
-        SizeChanged = OldSize != Size;
-      }
-
-      if (AAInfo == DenseMapInfo<AAMDNodes>::getEmptyKey())
-        // We don't have a AAInfo yet. Set it to NewAAInfo.
-        AAInfo = NewAAInfo;
-      else {
-        AAMDNodes Intersection(AAInfo.intersect(NewAAInfo));
-        SizeChanged |= Intersection != AAInfo;
-        AAInfo = Intersection;
-      }
-      return SizeChanged;
-    }
-
-    LocationSize getSize() const {
-      assert(isSizeSet() && "Getting an unset size!");
-      return Size;
-    }
-
-    /// Return the AAInfo, or null if there is no information or conflicting
-    /// information.
-    AAMDNodes getAAInfo() const {
-      // If we have missing or conflicting AAInfo, return null.
-      if (AAInfo == DenseMapInfo<AAMDNodes>::getEmptyKey() ||
-          AAInfo == DenseMapInfo<AAMDNodes>::getTombstoneKey())
-        return AAMDNodes();
-      return AAInfo;
-    }
-
-    AliasSet *getAliasSet(AliasSetTracker &AST) {
-      assert(AS && "No AliasSet yet!");
-      if (AS->Forward) {
-        AliasSet *OldAS = AS;
-        AS = OldAS->getForwardedTarget(AST);
-        AS->addRef();
-        OldAS->dropRef(AST);
-      }
-      return AS;
-    }
-
-    void setAliasSet(AliasSet *as) {
-      assert(!AS && "Already have an alias set!");
-      AS = as;
-    }
-
-    void eraseFromList() {
-      if (NextInList) NextInList->PrevInList = PrevInList;
-      *PrevInList = NextInList;
-      if (AS->PtrListEnd == &NextInList) {
-        AS->PtrListEnd = PrevInList;
-        assert(*AS->PtrListEnd == nullptr && "List not terminated right!");
-      }
-      delete this;
-    }
-  };
-
-  // Doubly linked list of nodes.
-  PointerRec *PtrList = nullptr;
-  PointerRec **PtrListEnd;
   // Forwarding pointer.
   AliasSet *Forward = nullptr;
 
+  /// Memory locations in this alias set.
+  SmallVector<MemoryLocation, 0> MemoryLocs;
+
   /// All instructions without a specific address in this alias set.
   std::vector<AssertingVH<Instruction>> UnknownInsts;
 
@@ -178,8 +89,6 @@ class AliasSet : public ilist_node<AliasSet> {
   };
   unsigned Alias : 1;
 
-  unsigned SetSize = 0;
-
   void addRef() { ++RefCount; }
 
   void dropRef(AliasSetTracker &AST) {
@@ -205,95 +114,40 @@ class AliasSet : public ilist_node<AliasSet> {
   /// Merge the specified alias set into this alias set.
   void mergeSetIn(AliasSet &AS, AliasSetTracker &AST, BatchAAResults &BatchAA);
 
-  // Alias Set iteration - Allow access to all of the pointers which are part of
-  // this alias set.
-  class iterator;
-  iterator begin() const { return iterator(PtrList); }
-  iterator end()   const { return iterator(); }
-  bool empty() const { return PtrList == nullptr; }
+  // Alias Set iteration - Allow access to all of the memory locations which are
+  // part of this alias set.
+  using iterator = SmallVectorImpl<MemoryLocation>::const_iterator;
+  iterator begin() const { return MemoryLocs.begin(); }
+  iterator end() const { return MemoryLocs.end(); }
 
-  // Unfortunately, ilist::size() is linear, so we have to add code to keep
-  // track of the list's exact size.
-  unsigned size() { return SetSize; }
+  unsigned size() { return MemoryLocs.size(); }
+
+  /// Retrieve the pointer values for the memory locations in this alias set.
+  /// The order matches that of the memory locations, but duplicate pointer
+  /// values are omitted.
+  using PointerVector = SmallVector<const Value *, 8>;
+  PointerVector getPointers() const;
 
   void print(raw_ostream &OS) const;
   void dump() const;
 
-  /// Define an iterator for alias sets... this is just a forward iterator.
-  class iterator {
-    PointerRec *CurNode;
-
-  public:
-    using iterator_category = std::forward_iterator_tag;
-    using value_type = PointerRec;
-    using difference_type = std::ptrdiff_t;
-    using pointer = value_type *;
-    using reference = value_type &;
-
-    explicit iterator(PointerRec *CN = nullptr) : CurNode(CN) {}
-
-    bool operator==(const iterator& x) const {
-      return CurNode == x.CurNode;
-    }
-    bool operator!=(const iterator& x) const { return !operator==(x); }
-
-    value_type &operator*() const {
-      assert(CurNode && "Dereferencing AliasSet.end()!");
-      return *CurNode;
-    }
-    value_type *operator->() const { return &operator*(); }
-
-    Value *getPointer() const { return CurNode->getValue(); }
-    LocationSize getSize() const { return CurNode->getSize(); }
-    AAMDNodes getAAInfo() const { return CurNode->getAAInfo(); }
-
-    iterator& operator++() {                // Preincrement
-      assert(CurNode && "Advancing past AliasSet.end()!");
-      CurNode = CurNode->getNext();
-      return *this;
-    }
-    iterator operator++(int) { // Postincrement
-      iterator tmp = *this; ++*this; return tmp;
-    }
-  };
-
 private:
   // Can only be created by AliasSetTracker.
   AliasSet()
-      : PtrListEnd(&PtrList), RefCount(0),  AliasAny(false), Access(NoAccess),
-        Alias(SetMustAlias) {}
-
-  PointerRec *getSomePointer() const {
-    return PtrList;
-  }
-
-  /// Return the real alias set this represents. If this has been merged with
-  /// another set and is forwarding, return the ultimate destination set. This
-  /// also implements the union-find collapsing as well.
-  AliasSet *getForwardedTarget(AliasSetTracker &AST) {
-    if (!Forward) return this;
-
-    AliasSet *Dest = Forward->getForwardedTarget(AST);
-    if (Dest != Forward) {
-      Dest->addRef();
-      Forward->dropRef(AST);
-      Forward = Dest;
-    }
-    return Dest;
-  }
+      : RefCount(0), AliasAny(false), Access(NoAccess), Alias(SetMustAlias) {}
 
   void removeFromTracker(AliasSetTracker &AST);
 
-  void addPointer(AliasSetTracker &AST, PointerRec &Entry, LocationSize Size,
-                  const AAMDNodes &AAInfo, bool KnownMustAlias = false,
-                  bool SkipSizeUpdate = false);
+  void addMemoryLocation(AliasSetTracker &AST, const MemoryLocation &MemLoc,
+                         bool KnownMustAlias = false);
   void addUnknownInst(Instruction *I, BatchAAResults &AA);
 
 public:
-  /// If the specified pointer "may" (or must) alias one of the members in the
-  /// set return the appropriate AliasResult. Otherwise return NoAlias.
-  AliasResult aliasesPointer(const Value *Ptr, LocationSize Size,
-                             const AAMDNodes &AAInfo, BatchAAResults &AA) const;
+  /// If the specified memory location "may" (or must) alias one of the members
+  /// in the set return the appropriate AliasResult. Otherwise return NoAlias.
+  AliasResult aliasesMemoryLocation(const MemoryLocation &MemLoc,
+                                    BatchAAResults &AA) const;
+
   ModRefInfo aliasesUnknownInst(const Instruction *Inst,
                                 BatchAAResults &AA) const;
 };
@@ -307,9 +161,10 @@ class AliasSetTracker {
   BatchAAResults &AA;
   ilist<AliasSet> AliasSets;
 
-  using PointerMapType = DenseMap<AssertingVH<Value>, AliasSet::PointerRec *>;
+  using PointerMapType = DenseMap<AssertingVH<const Value>, AliasSet *>;
 
-  // Map from pointers to their node
+  // Map from pointer values to the alias set holding one or more memory
+  // locations with that pointer value.
   PointerMapType PointerMap;
 
 public:
@@ -327,9 +182,6 @@ class AliasSetTracker {
   ///   3. If the instruction aliases multiple sets, merge the sets, and add
   ///      the instruction to the result.
   ///
-  /// These methods return true if inserting the instruction resulted in the
-  /// addition of a new alias set (i.e., the pointer did not alias anything).
-  ///
   void add(const MemoryLocation &Loc);
   void add(LoadInst *LI);
   void add(StoreInst *SI);
@@ -370,31 +222,39 @@ class AliasSetTracker {
 private:
   friend class AliasSet;
 
-  // The total number of pointers contained in all "may" alias sets.
-  unsigned TotalMayAliasSetSize = 0;
+  // The total number of memory locations contained in all alias sets.
+  unsigned TotalAliasSetSize = 0;
 
   // A non-null value signifies this AST is saturated. A saturated AST lumps
-  // all pointers into a single "May" set.
+  // all elements into a single "May" set.
   AliasSet *AliasAnyAS = nullptr;
 
   void removeAliasSet(AliasSet *AS);
 
-  /// Just like operator[] on the map, except that it creates an entry for the
-  /// pointer if it doesn't already exist.
-  AliasSet::PointerRec &getEntryFor(Value *V) {
-    AliasSet::PointerRec *&Entry = PointerMap[V];
-    if (!Entry)
-      Entry = new AliasSet::PointerRec(V);
-    return *Entry;
+  // Update an alias set field to point to its real destination. If the field is
+  // pointing to a set that has been merged with another set and is forwarding,
+  // the field is updated to point to the set obtained by following the
+  // forwarding links. The Forward fields of intermediate alias sets are
+  // collapsed as well, and alias set reference counts are updated to reflect
+  // the new situation.
+  void collapseForwardingIn(AliasSet *&AS) {
+    if (AS->Forward) {
+      collapseForwardingIn(AS->Forward);
+      // Swap out AS for AS->Forward, while updating reference counts.
+      AliasSet *NewAS = AS->Forward;
+      NewAS->addRef();
+      AS->dropRef(*this);
+      AS = NewAS;
+    }
   }
 
-  AliasSet &addPointer(MemoryLocation Loc, AliasSet::AccessLattice E);
-  AliasSet *mergeAliasSetsForPointer(const Value *Ptr, LocationSize Size,
-                                     const AAMDNodes &AAInfo,
-                                     bool &MustAliasAll);
+  AliasSet &addMemoryLocation(MemoryLocation Loc, AliasSet::AccessLattice E);
+  AliasSet *mergeAliasSetsForMemoryLocation(const MemoryLocation &MemLoc,
+                                            AliasSet *PtrAS,
+                                            bool &MustAliasAll);
 
-  /// Merge all alias sets into a single set that is considered to alias any
-  /// pointer.
+  /// Merge all alias sets into a single set that is considered to alias
+  /// any memory location or instruction.
   AliasSet &mergeAllAliasSets();
 
   AliasSet *findAliasSetForUnknownInst(Instruction *Inst);