Rewrite instruction deletion logic in many passes

Fix innumerable latent bugs with iterator invalidation and callback invocation.

Removes dead code earlier and chips away at all the redundant copies the compiler generates.

Author: atrick

include/swift/SIL/SILInstructionWorklist.h

@@ -128,13 +128,15 @@ class SILInstructionWorklist : SILInstructionWorklistBase {
     }
   }
 
-  /// All operands of \p instruction to the worklist when performing 2 stage
-  /// instruction deletion. Meant to be used right before deleting an
-  /// instruction in callbacks like InstModCallback::onNotifyWillBeDeleted().
+  /// Add operands of \p instruction to the worklist. Meant to be used once it
+  /// is certain that \p instruction will be deleted but may have operands that
+  /// are still alive. With fewer uses, the operand definition may be
+  /// optimizable.
+  ///
+  /// \p instruction may still have uses because this is called before
+  /// InstructionDeleter begins deleting it and some instructions are deleted at
+  /// the same time as their uses.
   void addOperandsToWorklist(SILInstruction &instruction) {
-    assert(!instruction.hasUsesOfAnyResult() &&
-           "Cannot erase instruction that is used!");
-
     // Make sure that we reprocess all operands now that we reduced their
     // use counts.
     if (instruction.getNumOperands() < 8) {

lib/SILOptimizer/IPO/GlobalOpt.cpp

@@ -457,8 +457,9 @@ bool SILGlobalOpt::optimizeInitializer(SILFunction *AddrF,
 
   // Remove "once" call from the addressor.
   removeToken(CallToOnce->getOperand(0));
-  eraseUsesOfInstruction(CallToOnce);
-  recursivelyDeleteTriviallyDeadInstructions(CallToOnce, true);
+  InstructionDeleter deleter;
+  deleter.forceDeleteWithUsers(CallToOnce);
+  deleter.cleanupDeadInstructions();
 
   // Create the constant initializer of the global variable.
   StaticInitCloner::appendToInitializer(SILG, InitVal);
@@ -818,11 +819,15 @@ bool SILGlobalOpt::run() {
   for (auto &allocPair : globalAllocPairs) {
     HasChanged |= tryRemoveGlobalAlloc(allocPair.first, allocPair.second);
   }
-
-  // Erase the instructions that we have marked for deletion.
-  for (auto *inst : InstToRemove) {
-    eraseUsesOfInstruction(inst);
-    inst->eraseFromParent();
+  if (HasChanged) {
+    // Erase the instructions that we have marked for deletion.
+    InstructionDeleter deleter;
+    for (auto *inst : InstToRemove) {
+      deleter.forceDeleteWithUsers(inst);
+    }
+    deleter.cleanupDeadInstructions();
+  } else {
+    assert(InstToRemove.empty());
   }
 
   for (auto &global : Module->getSILGlobals()) {

lib/SILOptimizer/IPO/LetPropertiesOpts.cpp

@@ -273,6 +273,8 @@ void LetPropertiesOpt::optimizeLetPropertyAccess(VarDecl *Property,
     return;
   }
 
+  InstructionDeleter deleter;
+
   auto &Loads = AccessMap[Property];
 
   unsigned NumReplaced = 0;
@@ -302,12 +304,12 @@ void LetPropertiesOpt::optimizeLetPropertyAccess(VarDecl *Property,
         continue;
 
       replaceLoadSequence(User, clonedInit);
-      eraseUsesOfInstruction(User);
-      User->eraseFromParent();
+      deleter.forceDeleteWithUsers(User);
       ++NumReplaced;
     }
     ChangedFunctions.insert(F);
   }
+  deleter.cleanupDeadInstructions();
 
   LLVM_DEBUG(llvm::dbgs() << "Access to " << *Property << " was replaced "
                           << NumReplaced << " time(s)\n");

lib/SILOptimizer/LoopTransforms/ForEachLoopUnroll.cpp

@@ -635,7 +635,7 @@ class ForEachLoopUnroller : public SILFunctionTransform {
     }
 
     if (changed) {
-      deleter.cleanUpDeadInstructions();
+      deleter.cleanupDeadInstructions();
       PM->invalidateAnalysis(&fun,
                   SILAnalysis::InvalidationKind::FunctionBody);
     }

lib/SILOptimizer/Mandatory/IRGenPrepare.cpp

@@ -63,7 +63,7 @@ static bool cleanFunction(SILFunction &fn) {
           deleter.forceDelete(bi);
           // StaticReport only takes trivial operands, and therefore doesn't
           // require fixing the lifetime of its operands.
-          deleter.cleanUpDeadInstructions();
+          deleter.cleanupDeadInstructions();
           madeChange = true;
           break;
         }

lib/SILOptimizer/Mandatory/MandatoryInlining.cpp

@@ -264,8 +264,16 @@ static  bool fixupReferenceCounts(
   return invalidatedStackNesting;
 }
 
-static SILValue cleanupLoadedCalleeValue(SILValue calleeValue, LoadInst *li) {
-  auto *pbi = dyn_cast<ProjectBoxInst>(li->getOperand());
+// Handle the case where the callee of the apply is either a load or a
+// project_box that was used by a deleted load. If we fail to optimize,
+// return an invalid SILValue.
+static SILValue cleanupLoadedCalleeValue(SILValue calleeValue) {
+  auto calleeSource = calleeValue;
+  auto *li = dyn_cast<LoadInst>(calleeValue);
+  if (li) {
+    calleeSource = li->getOperand();
+  }
+  auto *pbi = dyn_cast<ProjectBoxInst>(calleeSource);
   if (!pbi)
     return SILValue();
   auto *abi = dyn_cast<AllocBoxInst>(pbi->getOperand());
@@ -274,17 +282,18 @@ static SILValue cleanupLoadedCalleeValue(SILValue calleeValue, LoadInst *li) {
 
   // The load instruction must have no more uses or a single destroy left to
   // erase it.
-  if (li->getFunction()->hasOwnership()) {
-    // TODO: What if we have multiple destroy_value? That should be ok as well.
-    auto *dvi = li->getSingleUserOfType<DestroyValueInst>();
-    if (!dvi)
+  if (li) {
+    if (li->getFunction()->hasOwnership()) {
+      // TODO: What if we have multiple destroy_value? That should be ok.
+      auto *dvi = li->getSingleUserOfType<DestroyValueInst>();
+      if (!dvi)
+        return SILValue();
+      dvi->eraseFromParent();
+    } else if (!li->use_empty()) {
       return SILValue();
-    dvi->eraseFromParent();
-  } else if (!li->use_empty()) {
-    return SILValue();
+    }
+    li->eraseFromParent();
   }
-  li->eraseFromParent();
-
   // Look through uses of the alloc box the load is loading from to find up to
   // one store and up to one strong release.
   PointerUnion<StrongReleaseInst *, DestroyValueInst *> destroy;
@@ -356,15 +365,8 @@ static SILValue cleanupLoadedCalleeValue(SILValue calleeValue, LoadInst *li) {
 /// longer necessary after inlining.
 static void cleanupCalleeValue(SILValue calleeValue,
                                bool &invalidatedStackNesting) {
-  // Handle the case where the callee of the apply is a load instruction. If we
-  // fail to optimize, return. Otherwise, see if we can look through other
-  // abstractions on our callee.
-  if (auto *li = dyn_cast<LoadInst>(calleeValue)) {
-    calleeValue = cleanupLoadedCalleeValue(calleeValue, li);
-    if (!calleeValue) {
-      return;
-    }
-  }
+  if (auto loadedValue = cleanupLoadedCalleeValue(calleeValue))
+    calleeValue = loadedValue;
 
   calleeValue = stripCopiesAndBorrows(calleeValue);
 

lib/SILOptimizer/Mandatory/OSLogOptimization.cpp

@@ -1200,6 +1200,8 @@ static void forceDeleteAllocStack(SingleValueInstruction *inst,
       forceDeleteAllocStack(cast<BeginAccessInst>(user), deleter);
       continue;
     }
+    // Notify the deletion worklist in case user's other operands become dead.
+    deleter.getCallbacks().notifyWillBeDeleted(user);
     deleter.forceDeleteAndFixLifetimes(user);
   }
   deleter.forceDelete(inst);
@@ -1252,8 +1254,11 @@ static bool tryEliminateOSLogMessage(SingleValueInstruction *oslogMessage) {
     if (deletedInstructions.count(inst))
       continue;
     deleteInstructionWithUsersAndFixLifetimes(inst, deleter);
+    // Call cleanupDeadInstructions incrementally because it may expose a dead
+    // alloc_stack, which will only be deleted by this pass via
+    // deleteInstructionWithUsersAndFixLifetimes().
+    deleter.cleanupDeadInstructions();
   }
-  deleter.cleanUpDeadInstructions();
 
   // If the OSLogMessage instance is not deleted, either we couldn't see the
   // body of the log call or there is a bug in the library implementation.
@@ -1478,7 +1483,7 @@ suppressGlobalStringTablePointerError(SingleValueInstruction *oslogMessage) {
     // many instructions, do the cleanup at the end.
     deleter.trackIfDead(bi);
   }
-  deleter.cleanUpDeadInstructions();
+  deleter.cleanupDeadInstructions();
 }
 
 /// If the SILInstruction is an initialization of OSLogMessage, return the