migrate SILInliner to use InstructionDeleter

eeckstein · eeckstein · commit 7c28d7a6254b · 2021-10-28T18:43:14.000+02:00
diff --git a/include/swift/SILOptimizer/Utils/SILInliner.h b/include/swift/SILOptimizer/Utils/SILInliner.h
@@ -26,6 +26,7 @@
 namespace swift {
 
 class SILOptFunctionBuilder;
+class InstructionDeleter;
 
 // For now Free is 0 and Expensive is 1. This can be changed in the future by
 // adding more categories.
@@ -46,15 +47,16 @@ class SILInliner {
 
 private:
   SILOptFunctionBuilder &FuncBuilder;
+  InstructionDeleter &deleter;
+
   InlineKind IKind;
   SubstitutionMap ApplySubs;
 
-  DeletionFuncTy DeletionCallback;
-
 public:
-  SILInliner(SILOptFunctionBuilder &FuncBuilder, InlineKind IKind,
-             SubstitutionMap ApplySubs)
-      : FuncBuilder(FuncBuilder), IKind(IKind), ApplySubs(ApplySubs) {}
+  SILInliner(SILOptFunctionBuilder &FuncBuilder, InstructionDeleter &deleter,
+             InlineKind IKind, SubstitutionMap ApplySubs)
+      : FuncBuilder(FuncBuilder), deleter(deleter), IKind(IKind),
+        ApplySubs(ApplySubs) {}
 
   /// Returns true if we are able to inline \arg AI.
   ///
@@ -73,14 +75,6 @@ class SILInliner {
     return isa<BeginApplyInst>(apply);
   }
 
-  /// Allow the client to track instructions before they are deleted. The
-  /// registered callback is called from
-  /// recursivelyDeleteTriviallyDeadInstructions.
-  ///
-  /// (This is safer than the SILModule deletion callback because the
-  /// instruction is still in a valid form and its operands can be inspected.)
-  void setDeletionCallback(DeletionFuncTy f) { DeletionCallback = f; }
-
   /// Inline a callee function at the given apply site with the given
   /// arguments. Delete the apply and any dead arguments. Return a valid
   /// iterator to the first inlined instruction (or first instruction after the
@@ -108,11 +102,9 @@ class SILInliner {
   /// *NOTE*: Inlining can result in improperly nested stack allocations, which
   /// must be corrected after inlining. See invalidatesStackNesting().
   ///
-  /// Returns an iterator to the first inlined instruction (or the end of the
-  /// caller block for empty functions) and the last block in function order
-  /// containing inlined instructions (the original caller block for
-  /// single-block functions).
-  std::pair<SILBasicBlock::iterator, SILBasicBlock *>
+  /// Returns the last block in function order containing inlined instructions
+  /// (the original caller block for single-block functions).
+  SILBasicBlock *
   inlineFunction(SILFunction *calleeFunction, FullApplySite apply,
                  ArrayRef<SILValue> appliedArgs);
 
@@ -131,13 +123,12 @@ class SILInliner {
   /// *NOTE*: Inlining can result in improperly nested stack allocations, which
   /// must be corrected after inlining. See invalidatesStackNesting().
   ///
-  /// Returns an iterator to the first inlined instruction (or the end of the
-  /// caller block for empty functions) and the last block in function order
-  /// containing inlined instructions (the original caller block for
-  /// single-block functions).
-  static std::pair<SILBasicBlock::iterator, SILBasicBlock *>
+  /// Returns the last block in function order containing inlined instructions
+  /// (the original caller block for single-block functions).
+  static SILBasicBlock *
   inlineFullApply(FullApplySite apply, SILInliner::InlineKind inlineKind,
-                  SILOptFunctionBuilder &funcBuilder);
+                  SILOptFunctionBuilder &funcBuilder,
+                  InstructionDeleter &deleter);
 };
 
 } // end namespace swift
diff --git a/lib/SILOptimizer/Mandatory/MandatoryInlining.cpp b/lib/SILOptimizer/Mandatory/MandatoryInlining.cpp
@@ -874,8 +874,18 @@ runOnFunctionRecursively(SILOptFunctionBuilder &FuncBuilder, SILFunction *F,
                    ? PAI->getSubstitutionMap()
                    : InnerAI.getSubstitutionMap());
 
-      SILInliner Inliner(FuncBuilder, SILInliner::InlineKind::MandatoryInline,
-                         Subs);
+
+      // Register a callback to record potentially unused function values after
+      // inlining.
+      ClosureCleanup closureCleanup;
+
+      InstructionDeleter deleter(InstModCallbacks().onNotifyWillBeDeleted(
+        [&closureCleanup](SILInstruction *I) {
+          closureCleanup.recordDeadFunction(I);
+        }));
+
+      SILInliner Inliner(FuncBuilder, deleter,
+                         SILInliner::InlineKind::MandatoryInline, Subs);
       if (!Inliner.canInlineApplySite(InnerAI))
         continue;
 
@@ -910,23 +920,18 @@ runOnFunctionRecursively(SILOptFunctionBuilder &FuncBuilder, SILFunction *F,
                                           CapturedArgs, IsCalleeGuaranteed);
       }
 
-      // Register a callback to record potentially unused function values after
-      // inlining.
-      ClosureCleanup closureCleanup;
-      Inliner.setDeletionCallback([&closureCleanup](SILInstruction *I) {
-        closureCleanup.recordDeadFunction(I);
-      });
-
       invalidatedStackNesting |= Inliner.invalidatesStackNesting(InnerAI);
 
       // Inlining deletes the apply, and can introduce multiple new basic
       // blocks. After this, CalleeValue and other instructions may be invalid.
       // nextBB will point to the last inlined block
-      auto firstInlinedInstAndLastBB =
+      SILBasicBlock *lastBB =
           Inliner.inlineFunction(CalleeFunction, InnerAI, FullArgs);
-      nextBB = firstInlinedInstAndLastBB.second->getReverseIterator();
+      nextBB = lastBB->getReverseIterator();
       ++NumMandatoryInlines;
 
+      deleter.cleanupDeadInstructions();
+
       // The IR is now valid, and trivial dead arguments are removed. However,
       // we may be able to remove dead callee computations (e.g. dead
       // partial_apply closures).
diff --git a/lib/SILOptimizer/Transforms/CSE.cpp b/lib/SILOptimizer/Transforms/CSE.cpp
@@ -732,8 +732,10 @@ bool CSE::processLazyPropertyGetters() {
     SILBasicBlock *callBlock = ai->getParent();
 
     // Inline the getter...
+    InstructionDeleter deleter;
     SILInliner::inlineFullApply(ai, SILInliner::InlineKind::PerformanceInline,
-                                FuncBuilder);
+                                FuncBuilder, deleter);
+    deleter.cleanupDeadInstructions();
     
     // ...and fold the switch_enum in the first block to the Optional.some case.
     // The Optional.none branch becomes dead.
diff --git a/lib/SILOptimizer/Transforms/PerformanceInliner.cpp b/lib/SILOptimizer/Transforms/PerformanceInliner.cpp
@@ -946,6 +946,8 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller) {
   if (AppliesToInline.empty())
     return false;
 
+  InstructionDeleter deleter;
+
   // Second step: do the actual inlining.
   // We inline in reverse order, because for very large blocks with many applies
   // to inline, splitting the block at every apply would be quadratic.
@@ -979,9 +981,11 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller) {
     // If for whatever reason we can not inline this function, inlineFullApply
     // will assert, so we are safe making this assumption.
     SILInliner::inlineFullApply(AI, SILInliner::InlineKind::PerformanceInline,
-                                FuncBuilder);
+                                FuncBuilder, deleter);
     ++NumFunctionsInlined;
   }
+  deleter.cleanupDeadInstructions();
+  
   // The inliner splits blocks at call sites. Re-merge trivial branches to
   // reestablish a canonical CFG.
   mergeBasicBlocks(Caller);
diff --git a/lib/SILOptimizer/Utils/SILInliner.cpp b/lib/SILOptimizer/Utils/SILInliner.cpp
@@ -256,7 +256,7 @@ class SILInlineCloner
   FullApplySite Apply;
   Optional<BeginApplySite> BeginApply;
 
-  SILInliner::DeletionFuncTy DeletionCallback;
+  InstructionDeleter &deleter;
 
   /// The location representing the inlined instructions.
   ///
@@ -272,18 +272,15 @@ class SILInlineCloner
   // control path.
   SILBasicBlock *ReturnToBB = nullptr;
 
-  // Keep track of the next instruction after inlining the call.
-  SILBasicBlock::iterator NextIter;
-
 public:
   SILInlineCloner(SILFunction *CalleeFunction, FullApplySite Apply,
                   SILOptFunctionBuilder &FuncBuilder, InlineKind IKind,
                   SubstitutionMap ApplySubs,
-                  SILInliner::DeletionFuncTy deletionCallback);
+                  InstructionDeleter &deleter);
 
   SILFunction *getCalleeFunction() const { return &Original; }
 
-  SILBasicBlock::iterator cloneInline(ArrayRef<SILValue> AppliedArgs);
+  void cloneInline(ArrayRef<SILValue> AppliedArgs);
 
 protected:
   SILValue borrowFunctionArgument(SILValue callArg, FullApplySite AI);
@@ -335,7 +332,7 @@ class SILInlineCloner
 };
 } // namespace swift
 
-std::pair<SILBasicBlock::iterator, SILBasicBlock *>
+SILBasicBlock *
 SILInliner::inlineFunction(SILFunction *calleeFunction, FullApplySite apply,
                            ArrayRef<SILValue> appliedArgs) {
   PrettyStackTraceSILFunction calleeTraceRAII("inlining", calleeFunction);
@@ -344,15 +341,16 @@ SILInliner::inlineFunction(SILFunction *calleeFunction, FullApplySite apply,
          && "Asked to inline function that is unable to be inlined?!");
 
   SILInlineCloner cloner(calleeFunction, apply, FuncBuilder, IKind, ApplySubs,
-                         DeletionCallback);
-  auto nextI = cloner.cloneInline(appliedArgs);
-  return std::make_pair(nextI, cloner.getLastClonedBB());
+                         deleter);
+  cloner.cloneInline(appliedArgs);
+  return cloner.getLastClonedBB();
 }
 
-std::pair<SILBasicBlock::iterator, SILBasicBlock *>
+SILBasicBlock *
 SILInliner::inlineFullApply(FullApplySite apply,
                             SILInliner::InlineKind inlineKind,
-                            SILOptFunctionBuilder &funcBuilder) {
+                            SILOptFunctionBuilder &funcBuilder,
+                            InstructionDeleter &deleter) {
   assert(apply.canOptimize());
   assert(!apply.getFunction()->hasOwnership() ||
          apply.getReferencedFunctionOrNull()->hasOwnership());
@@ -361,7 +359,7 @@ SILInliner::inlineFullApply(FullApplySite apply,
   for (const auto arg : apply.getArguments())
     appliedArgs.push_back(arg);
 
-  SILInliner Inliner(funcBuilder, inlineKind, apply.getSubstitutionMap());
+  SILInliner Inliner(funcBuilder, deleter, inlineKind, apply.getSubstitutionMap());
   return Inliner.inlineFunction(apply.getReferencedFunctionOrNull(), apply,
                                 appliedArgs);
 }
@@ -370,11 +368,11 @@ SILInlineCloner::SILInlineCloner(
     SILFunction *calleeFunction, FullApplySite apply,
     SILOptFunctionBuilder &funcBuilder, InlineKind inlineKind,
     SubstitutionMap applySubs,
-    SILInliner::DeletionFuncTy deletionCallback)
+    InstructionDeleter &deleter)
     : SuperTy(*apply.getFunction(), *calleeFunction, applySubs,
               /*DT=*/nullptr, /*Inlining=*/true),
       FuncBuilder(funcBuilder), IKind(inlineKind), Apply(apply),
-      DeletionCallback(deletionCallback) {
+      deleter(deleter) {
 
   SILFunction &F = getBuilder().getFunction();
   assert(apply.getFunction() && apply.getFunction() == &F
@@ -419,10 +417,8 @@ SILInlineCloner::SILInlineCloner(
 }
 
 // Clone the entire callee function into the caller function at the apply site.
-// Delete the original apply and all dead arguments except the callee. Return an
-// iterator the the first instruction after the original apply.
-SILBasicBlock::iterator
-SILInlineCloner::cloneInline(ArrayRef<SILValue> AppliedArgs) {
+// Delete the original apply and all dead arguments except the callee.
+void SILInlineCloner::cloneInline(ArrayRef<SILValue> AppliedArgs) {
   assert(getCalleeFunction()->getArguments().size() == AppliedArgs.size()
          && "Unexpected number of callee arguments.");
 
@@ -506,8 +502,6 @@ SILInlineCloner::cloneInline(ArrayRef<SILValue> AppliedArgs) {
 
   // Visit original BBs in depth-first preorder, starting with the
   // entry block, cloning all instructions and terminators.
-  //
-  // NextIter is initialized during `fixUp`.
   cloneFunctionBody(getCalleeFunction(), callerBlock, entryArgs);
 
   // For non-throwing applies, the inlined body now unconditionally branches to
@@ -520,7 +514,6 @@ SILInlineCloner::cloneInline(ArrayRef<SILValue> AppliedArgs) {
   //
   // Once all calls in a function are inlined, unconditional branches are
   // eliminated by mergeBlocks.
-  return NextIter;
 }
 
 void SILInlineCloner::visitTerminator(SILBasicBlock *BB) {
@@ -578,25 +571,8 @@ void SILInlineCloner::preFixUp(SILFunction *calleeFunction) {
 }
 
 void SILInlineCloner::postFixUp(SILFunction *calleeFunction) {
-  NextIter = std::next(Apply.getInstruction()->getIterator());
-
-  assert(!Apply.getInstruction()->hasUsesOfAnyResult());
-
-  auto callbacks = InstModCallbacks().onDelete([&](SILInstruction *deadInst) {
-    if (NextIter == deadInst->getIterator())
-      ++NextIter;
-    deadInst->eraseFromParent();
-  });
-  if (DeletionCallback) {
-    callbacks =
-        callbacks.onNotifyWillBeDeleted([this](SILInstruction *toDeleteInst) {
-          DeletionCallback(toDeleteInst);
-        });
-  }
-  InstructionDeleter deleter(callbacks);
-  callbacks.notifyWillBeDeleted(Apply.getInstruction());
+  deleter.getCallbacks().notifyWillBeDeleted(Apply.getInstruction());
   deleter.forceDelete(Apply.getInstruction());
-  deleter.cleanupDeadInstructions();
 }
 
 SILValue SILInlineCloner::borrowFunctionArgument(SILValue callArg,
