SIL: defer instruction deletion to the end of a pass run.

When an instruction is "deleted" from the SIL, it is put into the SILModule::scheduledForDeletion list.
The instructions in this list are eventually deleted for real in SILModule::flushDeletedInsts(), which is called by the pass manager after each pass run.
In other words: instruction deletion is deferred to the end of a pass.

This avoids dangling instruction pointers within the run of a pass and in analysis caches.
Note that the analysis invalidation mechanism ensures that analysis caches are invalidated before flushDeletedInsts().

Author: eeckstein

include/swift/SIL/SILBasicBlock.h

@@ -127,7 +127,10 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
   void push_back(SILInstruction *I);
   void push_front(SILInstruction *I);
   void remove(SILInstruction *I);
-  iterator erase(SILInstruction *I);
+  void erase(SILInstruction *I);
+  void erase(SILInstruction *I, SILModule &module);
+
+  void eraseAllInstructions(SILModule &module);
 
   SILInstruction &back() { return InstList.back(); }
   const SILInstruction &back() const {
@@ -439,8 +442,6 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
       I.dropAllReferences();
   }
 
-  void eraseInstructions();
-
 private:
   friend class SILArgument;
 

include/swift/SIL/SILFunction.h

@@ -981,6 +981,10 @@ class SILFunction
 
   void clear();
 
+  /// Like `clear`, but does not call `dropAllReferences`, which is the
+  /// responsibility of the caller.
+  void eraseAllBlocks();
+
   /// Return the identity substitutions necessary to forward this call if it is
   /// generic.
   SubstitutionMap getForwardingSubstitutionMap();

include/swift/SIL/SILGlobalVariable.h

@@ -181,6 +181,11 @@ class SILGlobalVariable
     StaticInitializerBlock.dropAllReferences();
   }
 
+  void clear() {
+    dropAllReferences();
+    StaticInitializerBlock.eraseAllInstructions(Module);
+  }
+
   /// Return whether this variable corresponds to a Clang node.
   bool hasClangNode() const;
 

include/swift/SIL/SILInstruction.h

@@ -320,6 +320,7 @@ class SILInstruction : public llvm::ilist_node<SILInstruction> {
   friend llvm::ilist_traits<SILInstruction>;
   friend llvm::ilist_traits<SILBasicBlock>;
   friend SILBasicBlock;
+  friend SILModule;
 
   /// A backreference to the containing basic block.  This is maintained by
   /// ilist_traits<SILInstruction>.
@@ -381,6 +382,10 @@ class SILInstruction : public llvm::ilist_node<SILInstruction> {
     return C.allocateInst(Bytes, Alignment);
   }
 
+  /// Returns true if this instruction is removed from its function and
+  /// scheduled to be deleted.
+  bool isDeleted() const { return !ParentBB; }
+
   enum class MemoryBehavior {
     None,
     /// The instruction may read memory.

include/swift/SIL/SILModule.h

@@ -189,6 +189,17 @@ class SILModule {
   /// For consistency checking.
   size_t numAllocatedSlabs = 0;
 
+  /// When an instruction is "deleted" from the SIL, it is put into this list.
+  /// The instructions in this list are eventually deleted for real in
+  /// flushDeletedInsts(), which is called by the pass manager after each pass
+  /// run.
+  /// In other words: instruction deletion is deferred to the end of a pass.
+  ///
+  /// This avoids dangling instruction pointers within the run of a pass and in
+  /// analysis caches. Note that the analysis invalidation mechanism ensures
+  /// that analysis caches are invalidated before flushDeletedInsts().
+  llvm::iplist<SILInstruction> scheduledForDeletion;
+
   /// The swift Module associated with this SILModule.
   ModuleDecl *TheSwiftModule;
 
@@ -849,8 +860,17 @@ class SILModule {
   /// Allocate memory for an instruction using the module's internal allocator.
   void *allocateInst(unsigned Size, unsigned Align) const;
 
-  /// Deallocate memory of an instruction.
-  void deallocateInst(SILInstruction *I);
+  /// Called before \p I is removed from its basic block and scheduled for
+  /// deletion.
+  void willDeleteInstruction(SILInstruction *I);
+
+  /// Schedules the (already removed) instruction \p I for deletion.
+  /// See scheduledForDeletion for details.
+  void scheduleForDeletion(SILInstruction *I);
+
+  /// Deletes all scheuled instructions for real.
+  /// See scheduledForDeletion for details.
+  void flushDeletedInsts();
 
   /// Looks up the llvm intrinsic ID and type for the builtin function.
   ///

lib/SIL/IR/SILBasicBlock.cpp

@@ -41,35 +41,8 @@ SILBasicBlock::~SILBasicBlock() {
     return;
   }
 
-  SILModule &M = getModule();
-
-  // Invalidate all of the basic block arguments.
-  for (auto *Arg : ArgumentList) {
-    M.notifyDeleteHandlers(Arg);
-  }
-
   dropAllReferences();
-
-  for (auto I = begin(), E = end(); I != E;) {
-    auto Inst = &*I;
-    ++I;
-    erase(Inst);
-  }
-  assert(InstList.empty());
-}
-
-void SILBasicBlock::clearStaticInitializerBlock(SILModule &module) {
-  assert(!getParent() && "not a global variable's static initializer block");
-  assert(ArgumentList.empty() &&
-         "a static initializer block must not have arguments");
-
-  for (auto I = begin(), E = end(); I != E;) {
-    SILInstruction *Inst = &*I;
-    ++I;
-    InstList.erase(Inst);
-    module.deallocateInst(Inst);
-  }
-  assert(InstList.empty());
+  eraseAllInstructions(getModule());
 }
 
 int SILBasicBlock::getDebugID() const {
@@ -104,22 +77,21 @@ void SILBasicBlock::remove(SILInstruction *I) {
   InstList.remove(I);
 }
 
-void SILBasicBlock::eraseInstructions() {
- for (auto It = begin(); It != end();) {
-    auto *Inst = &*It++;
-    Inst->replaceAllUsesOfAllResultsWithUndef();
-    Inst->eraseFromParent();
+void SILBasicBlock::eraseAllInstructions(SILModule &module) {
+  while (!empty()) {
+    erase(&*begin(), module);
   }
 }
 
 /// Returns the iterator following the erased instruction.
-SILBasicBlock::iterator SILBasicBlock::erase(SILInstruction *I) {
-  // Notify the delete handlers that this instruction is going away.
-  SILModule &module = getModule();
-  module.notifyDeleteHandlers(I->asSILNode());
-  auto nextIter = InstList.erase(I);
-  module.deallocateInst(I);
-  return nextIter;
+void SILBasicBlock::erase(SILInstruction *I) {
+  erase(I, getModule());
+}
+
+void SILBasicBlock::erase(SILInstruction *I, SILModule &module) {
+  module.willDeleteInstruction(I);
+  InstList.remove(I);
+  module.scheduleForDeletion(I);
 }
 
 /// This method unlinks 'self' from the containing SILFunction and deletes it.

lib/SIL/IR/SILFunction.cpp

@@ -201,16 +201,7 @@ SILFunction::~SILFunction() {
 
   auto &M = getModule();
   for (auto &BB : *this) {
-    for (auto I = BB.begin(), E = BB.end(); I != E;) {
-      auto Inst = &*I;
-      ++I;
-      SILInstruction::destroy(Inst);
-      // TODO: It is only safe to directly deallocate an
-      // instruction if this BB is being removed in scope
-      // of destructing a SILFunction.
-      M.deallocateInst(Inst);
-    }
-    BB.InstList.clearAndLeakNodesUnsafely();
+    BB.eraseAllInstructions(M);
   }
 
   assert(RefCount == 0 &&
@@ -689,6 +680,10 @@ bool SILFunction::isExternallyUsedSymbol() const {
 
 void SILFunction::clear() {
   dropAllReferences();
+  eraseAllBlocks();
+}
+
+void SILFunction::eraseAllBlocks() {
   BlockList.clear();
 }
 

lib/SIL/IR/SILGlobalVariable.cpp

@@ -58,8 +58,7 @@ SILGlobalVariable::SILGlobalVariable(SILModule &Module, SILLinkage Linkage,
 }
 
 SILGlobalVariable::~SILGlobalVariable() {
-  StaticInitializerBlock.dropAllReferences();
-  StaticInitializerBlock.clearStaticInitializerBlock(Module);
+  clear();
 }
 
 bool SILGlobalVariable::isPossiblyUsedExternally() const {

lib/SIL/IR/SILInstruction.cpp

@@ -68,7 +68,6 @@ void llvm::ilist_traits<SILInstruction>::addNodeToList(SILInstruction *I) {
 
 void llvm::ilist_traits<SILInstruction>::removeNodeFromList(SILInstruction *I) {
   // When an instruction is removed from a BB, clear the parent pointer.
-  assert(I->ParentBB && "Not in a list!");
   I->ParentBB = nullptr;
 }
 
@@ -147,6 +146,12 @@ void SILInstruction::dropAllReferences() {
     OpI->drop();
   }
 
+  if (auto *termInst = dyn_cast<TermInst>(this)) {
+    for (SILSuccessor &succ : termInst->getSuccessors()) {
+      succ = nullptr;
+    }
+  }
+
   // If we have a function ref inst, we need to especially drop its function
   // argument so that it gets a proper ref decrement.
   if (auto *FRI = dyn_cast<FunctionRefBaseInst>(this)) {

lib/SIL/IR/SILModule.cpp

@@ -126,8 +126,9 @@ SILModule::~SILModule() {
   assert(!hasUnresolvedOpenedArchetypeDefinitions());
 
   // Decrement ref count for each SILGlobalVariable with static initializers.
-  for (SILGlobalVariable &v : silGlobals)
-    v.dropAllReferences();
+  for (SILGlobalVariable &v : silGlobals) {
+    v.clear();
+  }
 
   for (auto vt : vtables)
     vt->~SILVTable();
@@ -143,10 +144,16 @@ SILModule::~SILModule() {
     F.dropDynamicallyReplacedFunction();
     F.clearSpecializeAttrs();
   }
+
+  for (SILFunction &F : *this) {
+    F.eraseAllBlocks();
+  }
+  flushDeletedInsts();
 }
 
 void SILModule::checkForLeaks() const {
-  int instsInModule = 0;
+  int instsInModule = std::distance(scheduledForDeletion.begin(),
+                                    scheduledForDeletion.end());
   for (const SILFunction &F : *this) {
     for (const SILBasicBlock &block : F) {
       instsInModule += std::distance(block.begin(), block.end());
@@ -231,8 +238,30 @@ void *SILModule::allocateInst(unsigned Size, unsigned Align) const {
   return AlignedAlloc(Size, Align);
 }
 
-void SILModule::deallocateInst(SILInstruction *I) {
-  AlignedFree(I);
+void SILModule::willDeleteInstruction(SILInstruction *I) {
+  // Update openedArchetypeDefs.
+  if (auto *svi = dyn_cast<SingleValueInstruction>(I)) {
+    if (CanArchetypeType archeTy = svi->getOpenedArchetype()) {
+      OpenedArchetypeKey key = {archeTy, svi->getFunction()};
+      assert(openedArchetypeDefs.lookup(key) == svi &&
+             "archetype def was not registered");
+      openedArchetypeDefs.erase(key);
+    }
+  }
+}
+
+void SILModule::scheduleForDeletion(SILInstruction *I) {
+  I->dropAllReferences();
+  scheduledForDeletion.push_back(I);
+  I->ParentBB = nullptr;
+}
+
+void SILModule::flushDeletedInsts() {
+  while (!scheduledForDeletion.empty()) {
+    SILInstruction *inst = &*scheduledForDeletion.begin();
+    scheduledForDeletion.erase(inst);
+    AlignedFree(inst);
+  }
 }
 
 SILWitnessTable *

lib/SIL/Utils/MemAccessUtils.cpp

@@ -1767,7 +1767,9 @@ SILBasicBlock::iterator swift::removeBeginAccess(BeginAccessInst *beginAccess) {
       op->set(beginAccess->getSource());
     }
   }
-  return beginAccess->getParent()->erase(beginAccess);
+  auto nextIter = std::next(beginAccess->getIterator());
+  beginAccess->getParent()->erase(beginAccess);
+  return nextIter;
 }
 
 //===----------------------------------------------------------------------===//

lib/SILOptimizer/Mandatory/AccessMarkerElimination.cpp

@@ -53,8 +53,9 @@ struct AccessMarkerElimination {
   }
 
   SILBasicBlock::iterator eraseInst(SILInstruction *inst) {
+    auto nextIter = std::next(inst->getIterator());
     notifyErased(inst);
-    return inst->getParent()->erase(inst);
+    return nextIter;
   };
 
   bool shouldPreserveAccess(SILAccessEnforcement enforcement);

lib/SILOptimizer/PassManager/PassManager.cpp

@@ -473,6 +473,7 @@ void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) {
   }
   assert(analysesUnlocked() && "Expected all analyses to be unlocked!");
   Mod->removeDeleteNotificationHandler(SFT);
+  Mod->flushDeletedInsts();
 
   auto Delta = (std::chrono::system_clock::now() - StartTime).count();
   if (SILPrintPassTime) {
@@ -619,6 +620,7 @@ void SILPassManager::runModulePass(unsigned TransIdx) {
   SMT->run();
   Mod->removeDeleteNotificationHandler(SMT);
   assert(analysesUnlocked() && "Expected all analyses to be unlocked!");
+  Mod->flushDeletedInsts();
 
   auto Delta = (std::chrono::system_clock::now() - StartTime).count();
   if (SILPrintPassTime) {

lib/SILOptimizer/Transforms/Outliner.cpp

@@ -306,6 +306,13 @@ CanSILFunctionType BridgedProperty::getOutlinedFunctionType(SILModule &M) {
   return FunctionType;
 }
 
+static void eraseBlock(SILBasicBlock *block) {
+  for (SILInstruction &inst : *block) {
+    inst.replaceAllUsesOfAllResultsWithUndef();
+  }
+  block->eraseFromParent();
+}
+
 std::pair<SILFunction *, SILBasicBlock::iterator>
 BridgedProperty::outline(SILModule &M) {
   // Get the function type.
@@ -362,14 +369,10 @@ BridgedProperty::outline(SILModule &M) {
     // Delete the outlined instructions/blocks.
     if (Release)
       Release->eraseFromParent();
-    OutlinedEntryBB->eraseInstructions();
-    OutlinedEntryBB->eraseFromParent();
-    switchInfo.NoneBB->eraseInstructions();
-    switchInfo.NoneBB->eraseFromParent();
-    switchInfo.SomeBB->eraseInstructions();
-    switchInfo.SomeBB->eraseFromParent();
-    OldMergeBB->eraseInstructions();
-    OldMergeBB->eraseFromParent();
+    eraseBlock(OutlinedEntryBB);
+    eraseBlock(switchInfo.NoneBB);
+    eraseBlock(switchInfo.SomeBB);
+    eraseBlock(OldMergeBB);
     return std::make_pair(nullptr, std::prev(StartBB->end()));
   }
 
@@ -904,14 +907,10 @@ void BridgedReturn::outline(SILFunction *Fun, ApplyInst *NewOutlinedCall) {
   // Outlined function already existed. Just delete instructions and wire up
   // blocks.
   if (!Fun) {
-    OutlinedEntryBB->eraseInstructions();
-    OutlinedEntryBB->eraseFromParent();
-    switchInfo.NoneBB->eraseInstructions();
-    switchInfo.NoneBB->eraseFromParent();
-    switchInfo.SomeBB->eraseInstructions();
-    switchInfo.SomeBB->eraseFromParent();
-    OldMergeBB->eraseInstructions();
-    OldMergeBB->eraseFromParent();
+    eraseBlock(OutlinedEntryBB);
+    eraseBlock(switchInfo.NoneBB);
+    eraseBlock(switchInfo.SomeBB);
+    eraseBlock(OldMergeBB);
     return;
   }