[inst-simplify] Hide simplifyInstruction in favor of using simplifyAndReplaceAllSimplifiedUsesAndErase.

Currently all of these places in the code base perform simplifyInstruction and
then a replaceAllSimplifiedUsesAndErase(...). This is a bad pattern since:

1. simplifyInstruction assumes its result will be passed to
   replaceAllSimplifiedUsesAndErase. So by leaving these as separate things, we
   allow for users to pointlessly make this mistake.

2. I am going to implement in a subsequent commit a utility that lifetime
   extends interior pointer bases when replacing an address with an interior
   pointer derived address. To do this efficiently, I want to reuse state I
   compute during simplifyInstruction during the actual RAUW meaning that if the
   two operations are split, that is difficult without extending the API. So by
   removing this, I can make the transform and eliminate mistakes at the same
   time.

Author: gottesmm

include/swift/SILOptimizer/Analysis/SimplifyInstruction.h

@@ -27,31 +27,28 @@ namespace swift {
 class SILInstruction;
 class InstModCallbacks;
 
-/// Try to simplify the specified instruction, performing local
-/// analysis of the operands of the instruction, without looking at its uses
-/// (e.g. constant folding).  If a simpler result can be found, it is
-/// returned, otherwise a null SILValue is returned.
-///
-/// This is assumed to implement read-none transformations.
-SILValue simplifyInstruction(SILInstruction *I);
-
 /// Replace an instruction with a simplified result and erase it. If the
 /// instruction initiates a scope, do not replace the end of its scope; it will
 /// be deleted along with its parent.
 ///
-/// If it is nonnull, eraseNotify will be called before each instruction is
-/// deleted.
-///
-/// If it is nonnull and inst is in OSSA, newInstNotify will be called with each
-/// new instruction inserted to compensate for ownership.
-///
 /// NOTE: When OSSA is enabled this API assumes OSSA is properly formed and will
 /// insert compensating instructions.
 SILBasicBlock::iterator
 replaceAllSimplifiedUsesAndErase(SILInstruction *I, SILValue result,
                                  InstModCallbacks &callbacks,
                                  DeadEndBlocks *deadEndBlocks = nullptr);
 
+/// Attempt to map \p inst to a simplified result. Upon success, replace \p inst
+/// with this simplified result and erase \p inst. If the instruction initiates
+/// a scope, do not replace the end of its scope; it will be deleted along with
+/// its parent.
+///
+/// NOTE: When OSSA is enabled this API assumes OSSA is properly formed and will
+/// insert compensating instructions.
+SILBasicBlock::iterator simplifyAndReplaceAllSimplifiedUsesAndErase(
+    SILInstruction *I, InstModCallbacks &callbacks,
+    DeadEndBlocks *deadEndBlocks = nullptr);
+
 // Simplify invocations of builtin operations that may overflow.
 /// All such operations return a tuple (result, overflow_flag).
 /// This function try to simplify such operations, but returns only a

include/swift/SILOptimizer/Utils/InstOptUtils.h

@@ -399,6 +399,10 @@ class InstModCallbacks {
   }
 
   bool hadCallbackInvocation() const { return wereAnyCallbacksInvoked; }
+
+  /// Set \p wereAnyCallbacksInvoked to false. Useful if one wants to reuse an
+  /// InstModCallback in between iterations.
+  void resetHadCallbackInvocation() { wereAnyCallbacksInvoked = false; }
 };
 
 /// Get all consumed arguments of a partial_apply.

lib/SILOptimizer/Analysis/SimplifyInstruction.cpp

@@ -779,7 +779,7 @@ SILValue swift::simplifyOverflowBuiltinInstruction(BuiltinInst *BI) {
 ///
 /// NOTE: We assume that the insertion point associated with the SILValue must
 /// dominate \p i.
-SILValue swift::simplifyInstruction(SILInstruction *i) {
+static SILValue simplifyInstruction(SILInstruction *i) {
   SILValue result = InstSimplifier().visit(i);
   if (!result)
     return SILValue();
@@ -795,3 +795,25 @@ SILValue swift::simplifyInstruction(SILInstruction *i) {
 
   return result;
 }
+
+SILBasicBlock::iterator swift::simplifyAndReplaceAllSimplifiedUsesAndErase(
+    SILInstruction *i, InstModCallbacks &callbacks,
+    DeadEndBlocks *deadEndBlocks) {
+  auto next = std::next(i->getIterator());
+  auto *svi = dyn_cast<SingleValueInstruction>(i);
+  if (!svi)
+    return next;
+  SILValue result = simplifyInstruction(i);
+
+  // If we fail to simplify or the simplified value returned is our passed in
+  // value, just return std::next since we can't simplify.
+  if (!result || svi == result)
+    return next;
+
+  if (svi->getFunction()->hasOwnership()) {
+    JointPostDominanceSetComputer computer(*deadEndBlocks);
+    OwnershipFixupContext ctx{callbacks, *deadEndBlocks, computer};
+    return ctx.replaceAllUsesAndErase(svi, result);
+  }
+  return replaceAllUsesAndErase(svi, result, callbacks);
+}

lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

@@ -479,13 +479,11 @@ static bool stripOwnership(SILFunction &func) {
     auto value = visitor.instructionsToSimplify.pop_back_val();
     if (!value.hasValue())
       continue;
-    if (SILValue newValue = simplifyInstruction(*value)) {
-      InstModCallbacks callbacks([&](SILInstruction *instToErase) {
-        visitor.eraseInstruction(instToErase);
-      });
-      replaceAllSimplifiedUsesAndErase(*value, newValue, callbacks);
-      madeChange = true;
-    }
+    InstModCallbacks callbacks([&](SILInstruction *instToErase) {
+      visitor.eraseInstruction(instToErase);
+    });
+    simplifyAndReplaceAllSimplifiedUsesAndErase(*value, callbacks);
+    madeChange |= callbacks.hadCallbackInvocation();
   }
 
   return madeChange;

lib/SILOptimizer/Transforms/CSE.cpp

@@ -937,7 +937,6 @@ static bool isLazyPropertyGetter(ApplyInst *ai) {
 
 bool CSE::processNode(DominanceInfoNode *Node) {
   SILBasicBlock *BB = Node->getBlock();
-  InstModCallbacks callbacks;
   bool Changed = false;
 
   // See if any instructions in the block can be eliminated.  If so, do it.  If
@@ -961,12 +960,12 @@ bool CSE::processNode(DominanceInfoNode *Node) {
 
     // If the instruction can be simplified (e.g. X+0 = X) then replace it with
     // its simpler value.
-    if (SILValue V = simplifyInstruction(Inst)) {
-      LLVM_DEBUG(llvm::dbgs()
-                 << "SILCSE SIMPLIFY: " << *Inst << "  to: " << *V << '\n');
-      nextI = replaceAllSimplifiedUsesAndErase(Inst, V, callbacks, &DeadEndBBs);
-      Changed = true;
+    InstModCallbacks callbacks;
+    nextI = simplifyAndReplaceAllSimplifiedUsesAndErase(Inst, callbacks,
+                                                        &DeadEndBBs);
+    if (callbacks.hadCallbackInvocation()) {
       ++NumSimplify;
+      Changed = true;
       continue;
     }
 

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

@@ -1138,24 +1138,15 @@ bool SimplifyCFG::tryJumpThreading(BranchInst *BI) {
 /// simplifyBranchOperands - Simplify operands of branches, since it can
 /// result in exposing opportunities for CFG simplification.
 bool SimplifyCFG::simplifyBranchOperands(OperandValueArrayRef Operands) {
-  bool Simplified = false;
   InstModCallbacks callbacks;
   for (auto O = Operands.begin(), E = Operands.end(); O != E; ++O) {
     // All of our interesting simplifications are on single-value instructions
     // for now.
     if (auto *I = dyn_cast<SingleValueInstruction>(*O)) {
-      SILValue Result = simplifyInstruction(I);
-
-      // The Result can be the same instruction I in case it is in an
-      // unreachable block. In this case it can reference itself as operand.
-      if (Result && Result != I) {
-        LLVM_DEBUG(llvm::dbgs() << "simplify branch operand " << *I);
-        replaceAllSimplifiedUsesAndErase(I, Result, callbacks);
-        Simplified = true;
-      }
+      simplifyAndReplaceAllSimplifiedUsesAndErase(I, callbacks);
     }
   }
-  return Simplified;
+  return callbacks.hadCallbackInvocation();
 }
 
 static bool onlyHasTerminatorAndDebugInsts(SILBasicBlock *BB) {

lib/SILOptimizer/Transforms/TempRValueElimination.cpp

@@ -756,9 +756,7 @@ void TempRValueOptPass::run() {
     // Simplify any access scope markers that were only used by the dead
     // copy_addr and other potentially unused addresses.
     if (srcInst) {
-      if (SILValue result = simplifyInstruction(srcInst)) {
-        replaceAllSimplifiedUsesAndErase(srcInst, result, callbacks);
-      }
+      simplifyAndReplaceAllSimplifiedUsesAndErase(srcInst, callbacks);
     }
   }
   if (changed) {

lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp

@@ -81,24 +81,16 @@ killInstAndIncidentalUses(SingleValueInstruction *inst,
 // intruction that wasn't erased.
 static Optional<SILBasicBlock::iterator>
 simplifyAndReplace(SILInstruction *inst, CanonicalizeInstruction &pass) {
-  SILValue result = simplifyInstruction(inst);
-  if (!result)
-    return None;
-
-  ++NumSimplified;
-
-  LLVM_DEBUG(llvm::dbgs() << "Simplify Old = " << *inst
-                          << "    New = " << *result << '\n');
-
   // Erase the simplified instruction and any instructions that end its
   // scope. Nothing needs to be added to the worklist except for Result,
   // because the instruction and all non-replaced users will be deleted.
-  auto nextII = replaceAllSimplifiedUsesAndErase(inst, result, pass.callbacks,
-                                                 &pass.deadEndBlocks);
+  pass.callbacks.resetHadCallbackInvocation();
+  auto result = simplifyAndReplaceAllSimplifiedUsesAndErase(
+      inst, pass.callbacks, &pass.deadEndBlocks);
+  if (!pass.callbacks.hadCallbackInvocation())
+    return None;
 
-  // Push the new instruction and any users onto the worklist.
-  pass.notifyHasNewUsers(result);
-  return nextII;
+  return result;
 }
 
 //===----------------------------------------------------------------------===//

lib/SILOptimizer/Utils/InstOptUtils.cpp

@@ -1458,12 +1458,9 @@ bool swift::simplifyUsers(SingleValueInstruction *inst) {
     if (!svi)
       continue;
 
-    SILValue S = simplifyInstruction(svi);
-    if (!S)
-      continue;
-
-    replaceAllSimplifiedUsesAndErase(svi, S, callbacks);
-    changed = true;
+    callbacks.resetHadCallbackInvocation();
+    simplifyAndReplaceAllSimplifiedUsesAndErase(svi, callbacks);
+    changed |= callbacks.hadCallbackInvocation();
   }
 
   return changed;