SIL: remove the notifyDeleteHandlers mechanism

eeckstein · eeckstein · commit 497785009294 · 2021-05-26T21:57:54.000+02:00
It's not needed anymore with delayed instruction deletion.
It was used for two purposes:
1. For analysis, which cache instructions, to avoid dangling instruction pointers
2. For passes, which maintain worklists of instructions, to remove a deleted instructions from the worklist. This is now done by checking SILInstruction::isDeleted().
diff --git a/include/swift/SIL/Notifications.h b/include/swift/SIL/Notifications.h
@@ -239,27 +239,6 @@ class DeserializationNotificationHandlerSet final
   void didDeserialize(ModuleDecl *mod,
                       SILDefaultWitnessTable *wtable) override;
 };
-
-/// A protocol (or interface) for handling value deletion notifications.
-///
-/// This class is used as a base class for any class that need to accept
-/// instruction deletion notification messages. This is used by passes and
-/// analysis that need to invalidate data structures that contain pointers.
-/// This is similar to LLVM's ValueHandle.
-struct DeleteNotificationHandler {
-  DeleteNotificationHandler() { }
-  virtual ~DeleteNotificationHandler() {}
-
-  /// Handle the invalidation message for the value \p Value.
-  virtual void handleDeleteNotification(SILNode *value) { }
-
-  /// Returns True if the pass, analysis or other entity wants to receive
-  /// notifications. This callback is called once when the class is being
-  /// registered, and not once per notification. Entities that implement
-  /// this callback should always return a constant answer (true/false).
-  virtual bool needsNotifications() { return false; }
-};
-
 } // namespace swift
 
 #endif
diff --git a/include/swift/SIL/SILModule.h b/include/swift/SIL/SILModule.h
@@ -345,10 +345,6 @@ class SILModule {
   /// Action to be executed for serializing the SILModule.
   ActionCallback SerializeSILAction;
 
-  /// A list of clients that need to be notified when an instruction
-  /// invalidation message is sent.
-  llvm::SetVector<DeleteNotificationHandler*> NotificationHandlers;
-
   SILModule(llvm::PointerUnion<FileUnit *, ModuleDecl *> context,
             Lowering::TypeConverter &TC, const SILOptions &Options);
 
@@ -424,16 +420,6 @@ class SILModule {
   /// Called after an instruction is moved from one function to another.
   void notifyMovedInstruction(SILInstruction *inst, SILFunction *fromFunction);
 
-  /// Add a delete notification handler \p Handler to the module context.
-  void registerDeleteNotificationHandler(DeleteNotificationHandler* Handler);
-
-  /// Remove the delete notification handler \p Handler from the module context.
-  void removeDeleteNotificationHandler(DeleteNotificationHandler* Handler);
-
-  /// Send the invalidation message that \p V is being deleted to all
-  /// registered handlers. The order of handlers is deterministic but arbitrary.
-  void notifyDeleteHandlers(SILNode *node);
-
   /// Set a serialization action.
   void setSerializeSILAction(ActionCallback SerializeSILAction);
   ActionCallback getSerializeSILAction() const;
diff --git a/include/swift/SILOptimizer/Analysis/Analysis.h b/include/swift/SILOptimizer/Analysis/Analysis.h
@@ -36,7 +36,7 @@ struct SILAnalysisKind {
 };
 
 /// The base class for all SIL-level analysis.
-class SILAnalysis : public DeleteNotificationHandler {
+class SILAnalysis {
 public:
   /// This is a list of values that allow passes to communicate to analysis
   /// which traits of the code were invalidated. Based on this information
diff --git a/include/swift/SILOptimizer/Analysis/EpilogueARCAnalysis.h b/include/swift/SILOptimizer/Analysis/EpilogueARCAnalysis.h
@@ -235,12 +235,6 @@ class EpilogueARCFunctionInfo {
   llvm::DenseMap<SILValue, ARCInstructions> EpilogueReleaseInstCache;
 
 public:
-  void handleDeleteNotification(SILNode *node) {
-    // Being conservative and clear everything for now.
-    EpilogueRetainInstCache.clear();
-    EpilogueReleaseInstCache.clear();
-  }
-
   /// Constructor.
   EpilogueARCFunctionInfo(SILFunction *F, PostOrderAnalysis *PO,
                           AliasAnalysis *AA, RCIdentityAnalysis *RC)
@@ -288,22 +282,6 @@ class EpilogueARCAnalysis : public FunctionAnalysisBase<EpilogueARCFunctionInfo>
   EpilogueARCAnalysis(const EpilogueARCAnalysis &) = delete;
   EpilogueARCAnalysis &operator=(const EpilogueARCAnalysis &) = delete;
 
-  virtual void handleDeleteNotification(SILNode *node) override {
-    // If the parent function of this instruction was just turned into an
-    // external declaration, bail. This happens during SILFunction destruction.
-    SILFunction *F = node->getFunction();
-    if (F->isExternalDeclaration()) {
-      return;
-    }
-
-    // If we do have an analysis, tell it to handle its delete notifications.
-    if (auto A = maybeGet(F)) {
-      A.get()->handleDeleteNotification(node);
-    }
-  }
-
-  virtual bool needsNotifications() override { return true; }
-
   static bool classof(const SILAnalysis *S) {
     return S->getKind() == SILAnalysisKind::EpilogueARC;
   }
diff --git a/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h b/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h
@@ -832,11 +832,6 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
     /// Propagates the escape states through the graph.
     void propagateEscapeStates();
 
-    /// Remove a value from the graph. Do not delete the mapped node, but reset
-    /// mappedValue if it is set to this value, and make sure that the node
-    /// cannot be looked up with getNode().
-    void removeFromGraph(ValueBase *V);
-
     enum class Traversal { Follow, Backtrack, Halt };
 
     /// Traverse backward from startNode, following predecessor edges.
@@ -1237,10 +1232,6 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
   /// Notify the analysis about changed witness or vtables.
   virtual void invalidateFunctionTables() override { }
 
-  virtual void handleDeleteNotification(SILNode *N) override;
-
-  virtual bool needsNotifications() override { return true; }
-
   virtual void verify() const override;
 
   virtual void verify(SILFunction *F) const override;
diff --git a/include/swift/SILOptimizer/Analysis/RCIdentityAnalysis.h b/include/swift/SILOptimizer/Analysis/RCIdentityAnalysis.h
@@ -69,20 +69,6 @@ class RCIdentityFunctionInfo {
   /// intermediate array.
   void visitRCUses(SILValue V, function_ref<void(Operand *)> Visitor);
 
-  void handleDeleteNotification(SILNode *node) {
-    auto value = dyn_cast<ValueBase>(node);
-    if (!value)
-      return;
-
-    // Check the cache. If we don't find it, there is nothing to do.
-    auto Iter = RCCache.find(SILValue(value));
-    if (Iter == RCCache.end())
-      return;
-
-    // Then erase Iter from the cache.
-    RCCache.erase(Iter);
-  }
-
 private:
   SILValue getRCIdentityRootInner(SILValue V, unsigned RecursionDepth);
   SILValue stripRCIdentityPreservingOps(SILValue V, unsigned RecursionDepth);
@@ -104,18 +90,6 @@ class RCIdentityAnalysis : public FunctionAnalysisBase<RCIdentityFunctionInfo> {
   RCIdentityAnalysis(const RCIdentityAnalysis &) = delete;
   RCIdentityAnalysis &operator=(const RCIdentityAnalysis &) = delete;
 
-  virtual void handleDeleteNotification(SILNode *node) override {
-    // If the parent function of this instruction was just turned into an
-    // external declaration, bail. This happens during SILFunction destruction.
-    SILFunction *F = node->getFunction();
-    if (F->isExternalDeclaration()) {
-      return;
-    }
-    get(F)->handleDeleteNotification(node);
-  }
-
-  virtual bool needsNotifications() override { return true; }
-
   static bool classof(const SILAnalysis *S) {
     return S->getKind() == SILAnalysisKind::RCIdentity;
   }
diff --git a/include/swift/SILOptimizer/PassManager/Transforms.h b/include/swift/SILOptimizer/PassManager/Transforms.h
@@ -22,7 +22,7 @@ namespace swift {
   class PrettyStackTraceSILFunctionTransform;
 
   /// The base class for all SIL-level transformations.
-  class SILTransform : public DeleteNotificationHandler {
+  class SILTransform {
   public:
     /// The kind of transformation passes we use.
     enum class TransformKind {
diff --git a/lib/SIL/IR/SILBasicBlock.cpp b/lib/SIL/IR/SILBasicBlock.cpp
@@ -158,9 +158,6 @@ SILFunctionArgument *SILBasicBlock::replaceFunctionArgument(
 
   assert(ArgumentList[i]->use_empty() && "Expected no uses of the old arg!");
 
-  // Notify the delete handlers that this argument is being deleted.
-  M.notifyDeleteHandlers(ArgumentList[i]);
-
   SILFunctionArgument *NewArg = new (M) SILFunctionArgument(Ty, Kind, D);
   NewArg->setParent(this);
 
@@ -184,9 +181,6 @@ SILPhiArgument *SILBasicBlock::replacePhiArgument(unsigned i, SILType Ty,
 
   assert(ArgumentList[i]->use_empty() && "Expected no uses of the old BB arg!");
 
-  // Notify the delete handlers that this argument is being deleted.
-  M.notifyDeleteHandlers(ArgumentList[i]);
-
   SILPhiArgument *NewArg = new (M) SILPhiArgument(Ty, Kind, D);
   NewArg->setParent(this);
 
@@ -246,8 +240,6 @@ SILPhiArgument *SILBasicBlock::insertPhiArgument(unsigned AtArgPos, SILType Ty,
 void SILBasicBlock::eraseArgument(int Index) {
   assert(getArgument(Index)->use_empty() &&
          "Erasing block argument that has uses!");
-  // Notify the delete handlers that this BB argument is going away.
-  getModule().notifyDeleteHandlers(getArgument(Index));
   ArgumentList.erase(ArgumentList.begin() + Index);
 }
 
diff --git a/lib/SIL/IR/SILModule.cpp b/lib/SIL/IR/SILModule.cpp
@@ -808,36 +808,6 @@ void SILModule::notifyMovedInstruction(SILInstruction *inst,
   }
 }
 
-void SILModule::registerDeleteNotificationHandler(
-    DeleteNotificationHandler *handler) {
-  // Ask the handler (that can be an analysis, a pass, or some other data
-  // structure) if it wants to receive delete notifications.
-  if (handler->needsNotifications()) {
-    NotificationHandlers.insert(handler);
-  }
-}
-
-void SILModule::
-removeDeleteNotificationHandler(DeleteNotificationHandler* Handler) {
-  NotificationHandlers.remove(Handler);
-}
-
-void SILModule::notifyDeleteHandlers(SILNode *node) {
-  // Update openedArchetypeDefs.
-  if (auto *svi = dyn_cast<SingleValueInstruction>(node)) {
-    if (CanArchetypeType archeTy = svi->getOpenedArchetype()) {
-      OpenedArchetypeKey key = {archeTy, svi->getFunction()};
-      assert(openedArchetypeDefs.lookup(key) == svi &&
-             "archetype def was not registered");
-      openedArchetypeDefs.erase(key);
-    }
-  }
-
-  for (auto *Handler : NotificationHandlers) {
-    Handler->handleDeleteNotification(node);
-  }
-}
-
 // TODO: We should have an "isNoReturn" bit on Swift's BuiltinInfo, but for
 // now, let's recognize noreturn intrinsics and builtins specially here.
 bool SILModule::isNoReturnBuiltinOrIntrinsic(Identifier Name) {
diff --git a/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp b/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp
@@ -1271,15 +1271,6 @@ bool EscapeAnalysis::ConnectionGraph::forwardTraverseDefer(
   return true;
 }
 
-void EscapeAnalysis::ConnectionGraph::removeFromGraph(ValueBase *V) {
-  CGNode *node = Values2Nodes.lookup(V);
-  if (!node)
-    return;
-  Values2Nodes.erase(V);
-  if (node->mappedValue == V)
-    node->mappedValue = nullptr;
-}
-
 //===----------------------------------------------------------------------===//
 //                      Dumping, Viewing and Verification
 //===----------------------------------------------------------------------===//
@@ -2922,21 +2913,6 @@ void EscapeAnalysis::invalidate(SILFunction *F, InvalidationKind K) {
   }
 }
 
-void EscapeAnalysis::handleDeleteNotification(SILNode *node) {
-  auto value = dyn_cast<ValueBase>(node);
-  if (!value) return;
-
-  if (SILBasicBlock *Parent = node->getParentBlock()) {
-    SILFunction *F = Parent->getParent();
-    if (FunctionInfo *FInfo = Function2Info.lookup(F)) {
-      if (FInfo->isValid()) {
-        FInfo->Graph.removeFromGraph(value);
-        FInfo->SummaryGraph.removeFromGraph(value);
-      }
-    }
-  }
-}
-
 void EscapeAnalysis::verify() const {
 #ifndef NDEBUG
   for (auto Iter : Function2Info) {
diff --git a/lib/SILOptimizer/Mandatory/MandatoryCombine.cpp b/lib/SILOptimizer/Mandatory/MandatoryCombine.cpp
@@ -296,6 +296,9 @@ bool MandatoryCombiner::doOneIteration(SILFunction &function,
     // its contents to the worklist and then clear said list in preparation
     // for the next iteration.
     for (SILInstruction *instruction : createdInstructions) {
+      if (instruction->isDeleted())
+        continue;
+
       LLVM_DEBUG(llvm::dbgs() << "MC: add " << *instruction
                               << " from tracking list to worklist\n");
       worklist.add(instruction);
@@ -400,26 +403,6 @@ class MandatoryCombine final : public SILFunctionTransform {
       invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
     }
   }
-
-protected:
-  void handleDeleteNotification(SILNode *node) override {
-    // Remove instructions that were both created and deleted from the list of
-    // created instructions which will eventually be added to the worklist.
-
-    auto *instruction = dyn_cast<SILInstruction>(node);
-    if (instruction == nullptr) {
-      return;
-    }
-
-    // Linear searching the tracking list doesn't hurt because usually it only
-    // contains a few elements.
-    auto iterator = find(createdInstructions, instruction);
-    if (createdInstructions.end() != iterator) {
-      createdInstructions.erase(iterator);
-    }
-  }
-
-  bool needsNotifications() override { return true; }
 };
 
 } // end anonymous namespace
diff --git a/lib/SILOptimizer/Mandatory/MandatoryInlining.cpp b/lib/SILOptimizer/Mandatory/MandatoryInlining.cpp
@@ -417,46 +417,6 @@ static void cleanupCalleeValue(SILValue calleeValue,
 namespace {
 /// Cleanup dead closures after inlining.
 class ClosureCleanup {
-  using DeadInstSet = SmallBlotSetVector<SILInstruction *, 4>;
-
-  /// A helper class to update the set of dead instructions.
-  ///
-  /// Since this is called by the SILModule callback, the instruction may longer
-  /// be well-formed. Do not visit its operands. However, it's position in the
-  /// basic block is still valid.
-  ///
-  /// FIXME: Using the Module's callback mechanism for this is terrible.
-  /// Instead, cleanupCalleeValue could be easily rewritten to use its own
-  /// instruction deletion helper and pass a callback to tryDeleteDeadClosure
-  /// and recursivelyDeleteTriviallyDeadInstructions.
-  class DeleteUpdateHandler : public DeleteNotificationHandler {
-    SILModule &Module;
-    DeadInstSet &DeadInsts;
-
-  public:
-    DeleteUpdateHandler(SILModule &M, DeadInstSet &DeadInsts)
-        : Module(M), DeadInsts(DeadInsts) {
-      Module.registerDeleteNotificationHandler(this);
-    }
-
-    ~DeleteUpdateHandler() override {
-      // Unregister the handler.
-      Module.removeDeleteNotificationHandler(this);
-    }
-
-    // Handling of instruction removal notifications.
-    bool needsNotifications() override { return true; }
-
-    // Handle notifications about removals of instructions.
-    void handleDeleteNotification(SILNode *node) override {
-      auto deletedI = dyn_cast<SILInstruction>(node);
-      if (!deletedI)
-        return;
-
-      DeadInsts.erase(deletedI);
-    }
-  };
-
   SmallBlotSetVector<SILInstruction *, 4> deadFunctionVals;
 
 public:
@@ -500,9 +460,8 @@ class ClosureCleanup {
   // set needs to continue to be updated (by this handler) when deleting
   // instructions. This assumes that DeadFunctionValSet::erase() is stable.
   void cleanupDeadClosures(SILFunction *F) {
-    DeleteUpdateHandler deleteUpdate(F->getModule(), deadFunctionVals);
     for (Optional<SILInstruction *> I : deadFunctionVals) {
-      if (!I.hasValue())
+      if (!I.hasValue() || I.getValue()->isDeleted())
         continue;
 
       if (auto *SVI = dyn_cast<SingleValueInstruction>(I.getValue()))
diff --git a/lib/SILOptimizer/PassManager/PassManager.cpp b/lib/SILOptimizer/PassManager/PassManager.cpp
diff --git a/lib/SILOptimizer/SILCombiner/SILCombine.cpp b/lib/SILOptimizer/SILCombiner/SILCombine.cpp
diff --git a/lib/SILOptimizer/Transforms/DeadObjectElimination.cpp b/lib/SILOptimizer/Transforms/DeadObjectElimination.cpp
