[DCE] Process instructions added during deletion.

In cea0f00, `InstructionDeleter` began
deleting `load [take]` instructions.  Analogous to how it creates a
`destroy_value` when deleting an instruction which consumes a value, in
the case of deleting a `load [take]` the `InstructionDeleter` inserts a
compensating `destroy_addr`.

Previously, `DeadCodeElimination` did not observe the creation of any
instructions created by the `InstructionDeleter`.  In the case of the
newly created `destroy_addr`, DCE didn't mark that the `destroy_addr`
was live and so deleted it.  The result was a leak.

Here, this is fixed by passing an `InstModCallbacks`--with an
`onCreateNewInst` implementation--down into `erasePhiArgument` that
eventually invokes the `InstructionDeleter`.  When the
`InstructionDeleter` creates a new instruction, DCE marks it live.

Author: nate-chandler

include/swift/SILOptimizer/Utils/CFGOptUtils.h

@@ -25,6 +25,7 @@
 
 #include "swift/SIL/SILBuilder.h"
 #include "swift/SIL/SILInstruction.h"
+#include "swift/SILOptimizer/Utils/InstModCallbacks.h"
 #include "swift/SILOptimizer/Utils/InstructionDeleter.h"
 
 namespace llvm {
@@ -36,7 +37,6 @@ namespace swift {
 class DominanceInfo;
 class SILLoop;
 class SILLoopInfo;
-struct InstModCallbacks;
 
 /// Adds a new argument to an edge between a branch and a destination
 /// block. Allows for user injected callbacks via \p callbacks.
@@ -65,14 +65,16 @@ TermInst *changeEdgeValue(TermInst *branch, SILBasicBlock *dest, size_t idx,
 /// specified index. Asserts internally that the argument along the edge does
 /// not have uses.
 TermInst *deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
-                          size_t argIndex, bool cleanupDeadPhiOp = true);
+                          size_t argIndex, bool cleanupDeadPhiOp = true,
+                          InstModCallbacks callbacks = InstModCallbacks());
 
 /// Erase the \p argIndex phi argument from \p block. Asserts that the argument
 /// is a /real/ phi argument. Removes all incoming values for the argument from
 /// predecessor terminators. Asserts internally that it only ever is given
 /// "true" phi argument.
 void erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
-                      bool cleanupDeadPhiOp = true);
+                      bool cleanupDeadPhiOp = true,
+                      InstModCallbacks callbacks = InstModCallbacks());
 
 /// Check if the edge from the terminator is critical.
 bool isCriticalEdge(TermInst *t, unsigned edgeIdx);

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

@@ -657,7 +657,9 @@ bool DCE::removeDead() {
 
         endLifetimeOfLiveValue(phiArg->getIncomingPhiValue(pred), insertPt);
       }
-      erasePhiArgument(&BB, i);
+      erasePhiArgument(&BB, i, /*cleanupDeadPhiOps=*/true,
+                       InstModCallbacks().onCreateNewInst(
+                           [&](auto *inst) { markInstructionLive(inst); }));
       Changed = true;
       BranchesChanged = true;
     }

lib/SILOptimizer/Utils/CFGOptUtils.cpp

@@ -74,21 +74,22 @@ TermInst *swift::addNewEdgeValueToBranch(TermInst *branch, SILBasicBlock *dest,
   return newBr;
 }
 
-static void
-deleteTriviallyDeadOperandsOfDeadArgument(MutableArrayRef<Operand> termOperands,
-                                          unsigned deadArgIndex) {
+static void deleteTriviallyDeadOperandsOfDeadArgument(
+    MutableArrayRef<Operand> termOperands, unsigned deadArgIndex,
+    InstModCallbacks callbacks = InstModCallbacks()) {
   Operand &op = termOperands[deadArgIndex];
   auto *i = op.get()->getDefiningInstruction();
   if (!i)
     return;
   op.set(SILUndef::get(op.get()->getType(), *i->getFunction()));
-  eliminateDeadInstruction(i);
+  eliminateDeadInstruction(i, callbacks);
 }
 
 // Our implementation assumes that our caller is attempting to remove a dead
 // SILPhiArgument from a SILBasicBlock and has already RAUWed the argument.
 TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
-                                 size_t argIndex, bool cleanupDeadPhiOps) {
+                                 size_t argIndex, bool cleanupDeadPhiOps,
+                                 InstModCallbacks callbacks) {
   if (auto *cbi = dyn_cast<CondBranchInst>(branch)) {
     SmallVector<SILValue, 8> trueArgs;
     SmallVector<SILValue, 8> falseArgs;
@@ -99,15 +100,15 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
     if (destBlock == cbi->getTrueBB()) {
       if (cleanupDeadPhiOps) {
         deleteTriviallyDeadOperandsOfDeadArgument(cbi->getTrueOperands(),
-                                                  argIndex);
+                                                  argIndex, callbacks);
       }
       trueArgs.erase(trueArgs.begin() + argIndex);
     }
 
     if (destBlock == cbi->getFalseBB()) {
       if (cleanupDeadPhiOps) {
         deleteTriviallyDeadOperandsOfDeadArgument(cbi->getFalseOperands(),
-                                                  argIndex);
+                                                  argIndex, callbacks);
       }
       falseArgs.erase(falseArgs.begin() + argIndex);
     }
@@ -125,7 +126,8 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
     SmallVector<SILValue, 8> args;
     llvm::copy(bi->getArgs(), std::back_inserter(args));
     if (cleanupDeadPhiOps) {
-      deleteTriviallyDeadOperandsOfDeadArgument(bi->getAllOperands(), argIndex);
+      deleteTriviallyDeadOperandsOfDeadArgument(bi->getAllOperands(), argIndex,
+                                                callbacks);
     }
     args.erase(args.begin() + argIndex);
     auto *result = SILBuilderWithScope(bi).createBranch(bi->getLoc(),
@@ -138,7 +140,8 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
 }
 
 void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
-                             bool cleanupDeadPhiOps) {
+                             bool cleanupDeadPhiOps,
+                             InstModCallbacks callbacks) {
   assert(block->getArgument(argIndex)->isPhi()
          && "Only should be used on phi arguments");
   block->eraseArgument(argIndex);
@@ -155,7 +158,8 @@ void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
     predBlocks.insert(pred);
 
   for (auto *pred : predBlocks)
-    deleteEdgeValue(pred->getTerminator(), block, argIndex, cleanupDeadPhiOps);
+    deleteEdgeValue(pred->getTerminator(), block, argIndex, cleanupDeadPhiOps,
+                    callbacks);
 }
 
 /// Changes the edge value between a branch and destination basic block

test/SILOptimizer/dead_code_elimination_ossa.sil

@@ -432,3 +432,40 @@ bb0(%0 : @owned $MO):
   %63 = tuple ()
   return %63 : $()
 }
+
+// The InstructionDeleter will delete the `load [take]` and insert a
+// `destroy_addr`.  Observe the creation of the new destroy_addr instruction
+// that occurs when deleting the `load [take]` and mark it live.  Prevents a
+// leak.
+// CHECK-LABEL: sil [ossa] @keep_new_destroy_addr : {{.*}} {
+// CHECK:         destroy_addr
+// CHECK-LABEL: } // end sil function 'keep_new_destroy_addr'
+sil [ossa] @keep_new_destroy_addr : $@convention(thin) () -> () {
+bb0:
+  try_apply undef() : $@convention(thin) () -> @error any Error, normal bb1, error bb5
+
+bb1(%4 : $()):
+  %err = alloc_stack $any Error
+  try_apply undef<any Error>(%err) : $@convention(method) <τ_1_1 where τ_1_1 : Error> () -> (@out τ_1_1, @error any Error), normal bb2, error bb6
+
+bb6(%e : @owned $any Error):
+  destroy_value %e : $any Error
+  dealloc_stack %err : $*any Error
+  br bb3
+
+bb4(%24 : @owned $any Error):
+  destroy_value %24 : $any Error
+  br bb3
+
+bb5(%30 : @owned $any Error):
+  br bb4(%30 : $any Error)
+
+bb2(%15 : $()):
+  %33 = load [take] %err : $*any Error
+  dealloc_stack %err : $*any Error
+  br bb4(%33 : $any Error)
+
+bb3:
+  %22 = tuple ()
+  return %22 : $()
+}