[Exclusivity] Switch static checking to use IndexTrie instead of Proj…

include/swift/SILOptimizer/Analysis/AccessSummaryAnalysis.h

@@ -22,6 +22,7 @@
 #include "swift/SIL/SILFunction.h"
 #include "swift/SIL/SILInstruction.h"
 #include "swift/SILOptimizer/Analysis/BottomUpIPAnalysis.h"
+#include "swift/SILOptimizer/Utils/IndexTrie.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 
@@ -129,12 +130,27 @@ class AccessSummaryAnalysis : public BottomUpIPAnalysis {
   llvm::DenseMap<SILFunction *, FunctionInfo *> FunctionInfos;
 
   llvm::SpecificBumpPtrAllocator<FunctionInfo> Allocator;
+
+  /// A trie of integer indices that gives pointer identity to a path of
+  /// projections. This is shared between all functions in the module.
+  IndexTrieNode *SubPathTrie;
+
 public:
-  AccessSummaryAnalysis() : BottomUpIPAnalysis(AnalysisKind::AccessSummary) {}
+  AccessSummaryAnalysis() : BottomUpIPAnalysis(AnalysisKind::AccessSummary) {
+    SubPathTrie = new IndexTrieNode();
+  }
+
+  ~AccessSummaryAnalysis() {
+    delete SubPathTrie;
+  }
 
   /// Returns a summary of the accesses performed by the given function.
   const FunctionSummary &getOrCreateSummary(SILFunction *Fn);
 
+  IndexTrieNode *getSubPathTrieRoot() {
+    return SubPathTrie;
+  }
+
   virtual void initialize(SILPassManager *PM) override {}
   virtual void invalidate() override;
   virtual void invalidate(SILFunction *F, InvalidationKind K) override;

include/swift/SILOptimizer/Utils/IndexTrie.h

@@ -24,11 +24,12 @@ class IndexTrieNode {
   static const unsigned RootIdx = ~0U;
   unsigned Index;
   llvm::SmallVector<IndexTrieNode*, 8> Children;
+  IndexTrieNode *Parent;
 
 public:
-  IndexTrieNode(): Index(RootIdx) {}
+  IndexTrieNode(): Index(RootIdx), Parent(nullptr) {}
 
-  explicit IndexTrieNode(unsigned V): Index(V) {}
+  explicit IndexTrieNode(unsigned V, IndexTrieNode *P): Index(V), Parent(P) {}
 
   IndexTrieNode(IndexTrieNode &) =delete;
   IndexTrieNode &operator=(const IndexTrieNode&) =delete;
@@ -53,12 +54,29 @@ class IndexTrieNode {
                               });
     if (I != Children.end() && (*I)->Index == Idx)
       return *I;
-    auto *N = new IndexTrieNode(Idx);
+    auto *N = new IndexTrieNode(Idx, this);
     Children.insert(I, N);
     return N;
   }
 
   ArrayRef<IndexTrieNode*> getChildren() const { return Children; }
+
+  IndexTrieNode *getParent() const { return Parent; }
+
+  /// Returns true when the sequence of indices represented by this
+  /// node is a prefix of the sequence represented by the passed-in node.
+  bool isPrefixOf(const IndexTrieNode *Other) const {
+    const IndexTrieNode *I = Other;
+
+    do {
+      if (this == I)
+        return true;
+
+      I = I->getParent();
+    } while (I);
+
+    return false;
+  }
 };
 
 } // end namespace swift

lib/SILOptimizer/Analysis/AccessSummaryAnalysis.cpp

@@ -301,6 +301,8 @@ AccessSummaryAnalysis::getOrCreateSummary(SILFunction *fn) {
 void AccessSummaryAnalysis::AccessSummaryAnalysis::invalidate() {
   FunctionInfos.clear();
   Allocator.DestroyAll();
+  delete SubPathTrie;
+  SubPathTrie = new IndexTrieNode();
 }
 
 void AccessSummaryAnalysis::invalidate(SILFunction *F, InvalidationKind K) {

lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

@@ -35,6 +35,7 @@
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILInstruction.h"
 #include "swift/SIL/Projection.h"
+#include "swift/SILOptimizer/Analysis/AccessSummaryAnalysis.h"
 #include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
@@ -167,10 +168,10 @@ class AccessedStorage {
 class RecordedAccess {
 private:
   BeginAccessInst *Inst;
-  ProjectionPath SubPath;
+  const IndexTrieNode *SubPath;
 
 public:
-  RecordedAccess(BeginAccessInst *BAI, const ProjectionPath &SubPath)
+  RecordedAccess(BeginAccessInst *BAI, const IndexTrieNode *SubPath)
       : Inst(BAI), SubPath(SubPath) {}
 
   BeginAccessInst *getInstruction() const { return Inst; }
@@ -179,15 +180,15 @@ class RecordedAccess {
 
   SILLocation getAccessLoc() const { return Inst->getLoc(); }
 
-  const ProjectionPath &getSubPath() const { return SubPath; }
+  const IndexTrieNode *getSubPath() const { return SubPath; }
 };
 
 /// Records the in-progress accesses to a given sub path.
 class SubAccessInfo {
 public:
-  SubAccessInfo(const ProjectionPath &P) : Path(P) {}
+  SubAccessInfo(const IndexTrieNode *P) : Path(P) {}
 
-  ProjectionPath Path;
+  const IndexTrieNode *Path;
 
   /// The number of in-progress 'read' accesses (that is 'begin_access [read]'
   /// instructions that have not yet had the corresponding 'end_access').
@@ -202,7 +203,7 @@ class SubAccessInfo {
 
 public:
   /// Increment the count for given access.
-  void beginAccess(BeginAccessInst *BAI, const ProjectionPath &SubPath) {
+  void beginAccess(BeginAccessInst *BAI, const IndexTrieNode *SubPath) {
     if (!FirstAccess) {
       assert(Reads == 0 && NonReads == 0);
       FirstAccess = RecordedAccess(BAI, SubPath);
@@ -250,14 +251,17 @@ class SubAccessInfo {
   }
 
   bool conflictsWithAccess(SILAccessKind Kind,
-                           const ProjectionPath &SubPath) const {
+                           const IndexTrieNode *SubPath) const {
     if (!canConflictWithAccessOfKind(Kind))
       return false;
 
-    SubSeqRelation_t Relation = Path.computeSubSeqRelation(SubPath);
-    // If the one path is a subsequence of the other (or they are the same)
-    // then access the same storage.
-    return (Relation != SubSeqRelation_t::Unknown);
+    return pathsConflict(Path, SubPath);
+  }
+
+  /// Returns true when the two subpaths access overlapping memory.
+  bool pathsConflict(const IndexTrieNode *Path1,
+                     const IndexTrieNode *Path2) const {
+    return Path1->isPrefixOf(Path2) || Path2->isPrefixOf(Path1);
   }
 };
 
@@ -271,7 +275,7 @@ class AccessInfo {
   SubAccessVector SubAccesses;
 
   /// Returns the SubAccess info for accessing at the given SubPath.
-  SubAccessInfo &findOrCreateSubAccessInfo(const ProjectionPath &SubPath) {
+  SubAccessInfo &findOrCreateSubAccessInfo(const IndexTrieNode *SubPath) {
     for (auto &Info : SubAccesses) {
       if (Info.Path == SubPath)
         return Info;
@@ -283,7 +287,7 @@ class AccessInfo {
 
   SubAccessVector::const_iterator
   findFirstSubPathWithConflict(SILAccessKind OtherKind,
-                               const ProjectionPath &OtherSubPath) const {
+                               const IndexTrieNode *OtherSubPath) const {
     // Note this iteration requires deterministic ordering for repeatable
     // diagnostics.
     for (auto I = SubAccesses.begin(), E = SubAccesses.end(); I != E; ++I) {
@@ -299,7 +303,7 @@ class AccessInfo {
   // Returns the previous access when beginning an access of the given Kind will
   // result in a conflict with a previous access.
   Optional<RecordedAccess>
-  conflictsWithAccess(SILAccessKind Kind, const ProjectionPath &SubPath) const {
+  conflictsWithAccess(SILAccessKind Kind, const IndexTrieNode *SubPath) const {
     auto I = findFirstSubPathWithConflict(Kind, SubPath);
     if (I == SubAccesses.end())
       return None;
@@ -326,13 +330,13 @@ class AccessInfo {
   }
 
   /// Increment the count for given access.
-  void beginAccess(BeginAccessInst *BAI, const ProjectionPath &SubPath) {
+  void beginAccess(BeginAccessInst *BAI, const IndexTrieNode *SubPath) {
     SubAccessInfo &SubAccess = findOrCreateSubAccessInfo(SubPath);
     SubAccess.beginAccess(BAI, SubPath);
   }
 
   /// Decrement the count for given access.
-  void endAccess(EndAccessInst *EAI, const ProjectionPath &SubPath) {
+  void endAccess(EndAccessInst *EAI, const IndexTrieNode *SubPath) {
     SubAccessInfo &SubAccess = findOrCreateSubAccessInfo(SubPath);
     SubAccess.endAccess(EAI);
   }
@@ -580,36 +584,46 @@ tryFixItWithCallToCollectionSwapAt(const BeginAccessInst *Access1,
 /// that stored-property relaxation supports: struct stored properties
 /// and tuple elements.
 static std::string getPathDescription(DeclName BaseName, SILType BaseType,
-                                      ProjectionPath SubPath, SILModule &M) {
+                                      const IndexTrieNode *SubPath,
+                                      SILModule &M) {
+  // Walk the trie to the root to collection the sequence (in reverse order).
+  llvm::SmallVector<unsigned, 4> ReversedIndices;
+  const IndexTrieNode *I = SubPath;
+  while (!I->isRoot()) {
+    ReversedIndices.push_back(I->getIndex());
+    I = I->getParent();
+  }
+
   std::string sbuf;
   llvm::raw_string_ostream os(sbuf);
 
   os << "'" << BaseName;
 
   SILType ContainingType = BaseType;
-  for (auto &P : SubPath) {
+  for (unsigned Index : reversed(ReversedIndices)) {
     os << ".";
-    switch (P.getKind()) {
-    case ProjectionKind::Struct:
-      os << P.getVarDecl(ContainingType)->getBaseName();
-      break;
-    case ProjectionKind::Tuple: {
-      // Use the tuple element's name if present, otherwise use its index.
-      Type SwiftTy = ContainingType.getSwiftRValueType();
-      TupleType *TupleTy = SwiftTy->getAs<TupleType>();
-      assert(TupleTy && "Tuple projection on non-tuple type!?");
 
-      Identifier ElementName = TupleTy->getElement(P.getIndex()).getName();
+    if (ContainingType.getAs<StructType>()) {
+      NominalTypeDecl *D = ContainingType.getNominalOrBoundGenericNominal();
+      auto Iter = D->getStoredProperties().begin();
+      std::advance(Iter, Index);
+      VarDecl *VD = *Iter;
+      os << VD->getBaseName();
+      ContainingType = ContainingType.getFieldType(VD, M);
+      continue;
+    }
+
+    if (auto TupleTy = ContainingType.getAs<TupleType>()) {
+      Identifier ElementName = TupleTy->getElement(Index).getName();
       if (ElementName.empty())
-        os << P.getIndex();
+        os << Index;
       else
         os << ElementName;
-      break;
-    }
-    default:
-      llvm_unreachable("Unexpected projection kind in SubPath!");
+      ContainingType = ContainingType.getTupleElementType(Index);
+      continue;
     }
-    ContainingType = P.getType(ContainingType, M);
+
+    llvm_unreachable("Unexpected type in projection SubPath!");
   }
 
   os << "'";
@@ -784,8 +798,14 @@ static bool isCallToStandardLibrarySwap(ApplyInst *AI, ASTContext &Ctx) {
   return FD == Ctx.getSwap(nullptr);
 }
 
-static SILInstruction *getSingleAddressProjectionUser(SILInstruction *I) {
+/// If the instruction is a field or tuple projection and it has a single
+/// user return a pair of the single user and the projection index.
+/// Otherwise, return a pair with the component nullptr and the second
+/// unspecified.
+static std::pair<SILInstruction *, unsigned>
+getSingleAddressProjectionUser(SILInstruction *I) {
   SILInstruction *SingleUser = nullptr;
+  unsigned ProjectionIndex = 0;
 
   for (Operand *Use : I->getUses()) {
     SILInstruction *User = Use->getUser();
@@ -794,28 +814,43 @@ static SILInstruction *getSingleAddressProjectionUser(SILInstruction *I) {
 
     // We have more than a single user so bail.
     if (SingleUser)
-      return nullptr;
+      return std::make_pair(nullptr, 0);
 
     switch (User->getKind()) {
     case ValueKind::StructElementAddrInst:
+      ProjectionIndex = cast<StructElementAddrInst>(User)->getFieldNo();
+      SingleUser = User;
+      break;
     case ValueKind::TupleElementAddrInst:
+      ProjectionIndex = cast<TupleElementAddrInst>(User)->getFieldNo();
       SingleUser = User;
       break;
     default:
-      return nullptr;
+      return std::make_pair(nullptr, 0);
     }
   }
 
-  return SingleUser;
+  return std::make_pair(SingleUser, ProjectionIndex);
 }
 
-static ProjectionPath findSubPathAccessed(BeginAccessInst *BAI) {
-  ProjectionPath SubPath(BAI->getType(), BAI->getType());
+/// Returns an IndexTrieNode that represents the single subpath accessed from
+/// BAI or the root if no such node exists.
+static const IndexTrieNode *findSubPathAccessed(BeginAccessInst *BAI,
+                                                IndexTrieNode *Root) {
+  IndexTrieNode *SubPath = Root;
 
+  // For each single-user projection of BAI, construct or get a node
+  // from the trie representing the index of the field or tuple element
+  // accessed by that projection.
   SILInstruction *Iter = BAI;
+  while (true) {
+    std::pair<SILInstruction *, unsigned> ProjectionUser =
+        getSingleAddressProjectionUser(Iter);
+    if (!ProjectionUser.first)
+      break;
 
-  while ((Iter = getSingleAddressProjectionUser(Iter))) {
-    SubPath.push_back(Projection(Iter));
+    SubPath = SubPath->getChild(ProjectionUser.second);
+    Iter = ProjectionUser.first;
   }
 
   return SubPath;
@@ -826,14 +861,15 @@ static ProjectionPath findSubPathAccessed(BeginAccessInst *BAI) {
 /// with. Otherwise, returns None.
 Optional<RecordedAccess> shouldReportAccess(const AccessInfo &Info,
                                             swift::SILAccessKind Kind,
-                                            const ProjectionPath &SubPath) {
+                                            const IndexTrieNode *SubPath) {
   if (Info.alreadyHadConflict())
     return None;
 
   return Info.conflictsWithAccess(Kind, SubPath);
 }
 
-static void checkStaticExclusivity(SILFunction &Fn, PostOrderFunctionInfo *PO) {
+static void checkStaticExclusivity(SILFunction &Fn, PostOrderFunctionInfo *PO,
+                                   AccessSummaryAnalysis *ASA) {
   // The implementation relies on the following SIL invariants:
   //    - All incoming edges to a block must have the same in-progress
   //      accesses. This enables the analysis to not perform a data flow merge
@@ -899,7 +935,8 @@ static void checkStaticExclusivity(SILFunction &Fn, PostOrderFunctionInfo *PO) {
         SILAccessKind Kind = BAI->getAccessKind();
         const AccessedStorage &Storage = findAccessedStorage(BAI->getSource());
         AccessInfo &Info = Accesses[Storage];
-        ProjectionPath SubPath = findSubPathAccessed(BAI);
+        const IndexTrieNode *SubPath =
+            findSubPathAccessed(BAI, ASA->getSubPathTrieRoot());
         if (auto Conflict = shouldReportAccess(Info, Kind, SubPath)) {
           ConflictingAccesses.emplace_back(Storage, *Conflict,
                                            RecordedAccess(BAI, SubPath));
@@ -914,7 +951,8 @@ static void checkStaticExclusivity(SILFunction &Fn, PostOrderFunctionInfo *PO) {
         AccessInfo &Info = It->getSecond();
 
         BeginAccessInst *BAI = EAI->getBeginAccess();
-        const ProjectionPath &SubPath = findSubPathAccessed(BAI);
+        const IndexTrieNode *SubPath =
+            findSubPathAccessed(BAI, ASA->getSubPathTrieRoot());
         Info.endAccess(EAI, SubPath);
 
         // If the storage location has no more in-progress accesses, remove
@@ -958,7 +996,8 @@ class DiagnoseStaticExclusivity : public SILFunctionTransform {
       return;
 
     PostOrderFunctionInfo *PO = getAnalysis<PostOrderAnalysis>()->get(Fn);
-    checkStaticExclusivity(*Fn, PO);
+    auto *ASA = getAnalysis<AccessSummaryAnalysis>();
+    checkStaticExclusivity(*Fn, PO, ASA);
   }
 };