Revert "Merge pull request #73275 from nate-chandler/lifetime-completion/enable"

This reverts commit 840198e, reversing
changes made to b74303c.

Author: nate-chandler

include/swift/AST/SILOptions.h

@@ -180,8 +180,8 @@ class SILOptions {
   /// If set to true, compile with the SIL Opaque Values enabled.
   bool EnableSILOpaqueValues = false;
 
-  /// Introduce linear OSSA lifetimes after SILGen
-  bool OSSACompleteLifetimes = true;
+  /// Require linear OSSA lifetimes after SILGen
+  bool OSSACompleteLifetimes = false;
 
   /// Verify linear OSSA lifetimes after SILGen
   bool OSSAVerifyComplete = false;

lib/DriverTool/sil_opt_main.cpp

@@ -231,9 +231,9 @@ struct SILOptOptions {
   EnableSILOpaqueValues = llvm::cl::opt<bool>("enable-sil-opaque-values",
                         llvm::cl::desc("Compile the module with sil-opaque-values enabled."));
 
-  llvm::cl::opt<bool> EnableOSSACompleteLifetimes = llvm::cl::opt<bool>(
-      "enable-ossa-complete-lifetimes", llvm::cl::init(true),
-      llvm::cl::desc("Require linear OSSA lifetimes after SILGenCleanup."));
+  llvm::cl::opt<bool>
+  EnableOSSACompleteLifetimes = llvm::cl::opt<bool>("enable-ossa-complete-lifetimes",
+                        llvm::cl::desc("Require linear OSSA lifetimes after SILGenCleanup."));
   llvm::cl::opt<bool>
   EnableOSSAVerifyComplete = llvm::cl::opt<bool>("enable-ossa-verify-complete",
                         llvm::cl::desc("Verify linear OSSA lifetimes after SILGenCleanup."));

lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

@@ -14,22 +14,21 @@
 
 #include "swift/Basic/Assertions.h"
 #include "swift/Basic/Defer.h"
-#include "swift/SIL/BasicBlockDatastructures.h"
 #include "swift/SIL/DebugUtils.h"
 #include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/PrunedLiveness.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBuilder.h"
 #include "swift/SIL/SILInstruction.h"
 #include "swift/SIL/SILValue.h"
+#include "swift/SIL/BasicBlockDatastructures.h"
 #include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
-#include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h"
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "swift/SILOptimizer/Utils/BasicBlockOptUtils.h"
 #include "swift/SILOptimizer/Utils/CFGOptUtils.h"
-#include "swift/SILOptimizer/Utils/InstOptUtils.h"
 #include "swift/SILOptimizer/Utils/OwnershipOptUtils.h"
+#include "swift/SILOptimizer/Utils/InstOptUtils.h"
 #include "swift/SILOptimizer/Utils/SILSSAUpdater.h"
 #include "swift/SILOptimizer/Utils/StackNesting.h"
 
@@ -446,6 +445,14 @@ static BuiltinInst *getEndAsyncLet(BuiltinInst *startAsyncLet) {
 /// a closure is used by \p closureUser.
 static void insertAfterClosureUser(SILInstruction *closureUser,
                                    function_ref<void(SILBuilder &)> insertFn) {
+  // Don't insert any destroy or deallocation right before an unreachable.
+  // It's not needed an will only add up to code size.
+  auto insertAtNonUnreachable = [&](SILBuilder &builder) {
+    if (isa<UnreachableInst>(builder.getInsertionPoint()))
+      return;
+    insertFn(builder);
+  };
+
   {
     SILInstruction *userForBorrow = closureUser;
     if (auto *m = dyn_cast<MoveOnlyWrapperToCopyableValueInst>(userForBorrow))
@@ -461,7 +468,7 @@ static void insertAfterClosureUser(SILInstruction *closureUser,
 
       for (auto eb : endBorrows) {
         SILBuilderWithScope builder(std::next(eb->getIterator()));
-        insertFn(builder);
+        insertAtNonUnreachable(builder);
       }
       return;
     }
@@ -472,12 +479,12 @@ static void insertAfterClosureUser(SILInstruction *closureUser,
     if (!endAsyncLet)
       return;
     SILBuilderWithScope builder(std::next(endAsyncLet->getIterator()));
-    insertFn(builder);
+    insertAtNonUnreachable(builder);
     return;
   }
   FullApplySite fas = FullApplySite::isa(closureUser);
   assert(fas);
-  fas.insertAfterApplication(insertFn);
+  fas.insertAfterApplication(insertAtNonUnreachable);
 }
 
 static SILValue skipConvert(SILValue v) {
@@ -993,7 +1000,6 @@ static SILValue tryRewriteToPartialApplyStack(
 
 static bool tryExtendLifetimeToLastUse(
     ConvertEscapeToNoEscapeInst *cvt, DominanceAnalysis *dominanceAnalysis,
-    DeadEndBlocksAnalysis *deadEndBlocksAnalysis,
     llvm::DenseMap<SILInstruction *, SILInstruction *> &memoized,
     llvm::DenseSet<SILBasicBlock *> &unreachableBlocks,
     InstructionDeleter &deleter, const bool &modifiedCFG) {
@@ -1042,22 +1048,10 @@ static bool tryExtendLifetimeToLastUse(
   cvt->setLifetimeGuaranteed();
   cvt->setOperand(closureCopy);
 
-  auto *function = cvt->getFunction();
-  // The CFG may have been modified during this run, which would have made
-  // dead-end blocks analysis invalid.  Mark it invalid it now if that
-  // happened.  If the CFG hasn't been modified, this is a noop thanks to
-  // DeadEndBlocksAnalysis::shouldInvalidate.
-  deadEndBlocksAnalysis->invalidate(function,
-                                    analysisInvalidationKind(modifiedCFG));
-  auto *deadEndBlocks = deadEndBlocksAnalysis->get(function);
-
-  insertAfterClosureUser(
-      singleUser, [closureCopy, deadEndBlocks](SILBuilder &builder) {
-        auto loc = RegularLocation(builder.getInsertionPointLoc());
-        auto isDeadEnd = IsDeadEnd_t(
-            deadEndBlocks->isDeadEnd(builder.getInsertionPoint()->getParent()));
-        builder.createDestroyValue(loc, closureCopy, DontPoisonRefs, isDeadEnd);
-      });
+  insertAfterClosureUser(singleUser, [closureCopy](SILBuilder &builder) {
+    auto loc = RegularLocation(builder.getInsertionPointLoc());
+    builder.createDestroyValue(loc, closureCopy);
+  });
   /*
   llvm::errs() << "after lifetime extension of\n";
   escapingClosure->dump();
@@ -1446,7 +1440,6 @@ static void computeUnreachableBlocks(
 
 static bool fixupClosureLifetimes(SILFunction &fn,
                                   DominanceAnalysis *dominanceAnalysis,
-                                  DeadEndBlocksAnalysis *deadEndBlocksAnalysis,
                                   bool &checkStackNesting, bool &modifiedCFG) {
   bool changed = false;
 
@@ -1483,8 +1476,7 @@ static bool fixupClosureLifetimes(SILFunction &fn,
         }
       }
 
-      if (tryExtendLifetimeToLastUse(cvt, dominanceAnalysis,
-                                     deadEndBlocksAnalysis, memoizedQueries,
+      if (tryExtendLifetimeToLastUse(cvt, dominanceAnalysis, memoizedQueries,
                                      unreachableBlocks, updater.getDeleter(),
                                      /*const*/ modifiedCFG)) {
         changed = true;
@@ -1524,11 +1516,9 @@ class ClosureLifetimeFixup : public SILFunctionTransform {
     bool modifiedCFG = false;
 
     auto *dominanceAnalysis = PM->getAnalysis<DominanceAnalysis>();
-    auto *deadEndBlocksAnalysis = getAnalysis<DeadEndBlocksAnalysis>();
 
     if (fixupClosureLifetimes(*getFunction(), dominanceAnalysis,
-                              deadEndBlocksAnalysis, checkStackNesting,
-                              modifiedCFG)) {
+                              checkStackNesting, modifiedCFG)) {
       updateBorrowedFrom(getPassManager(), getFunction());
       if (checkStackNesting){
         modifiedCFG |=

lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

@@ -943,7 +943,7 @@ void CanonicalizeOSSALifetime::findExtendedBoundary(
 /// record it as a final consume.
 static void
 insertDestroyBeforeInstruction(SILInstruction *nextInstruction,
-                               SILValue currentDef, IsDeadEnd_t isDeadEnd,
+                               SILValue currentDef,
                                CanonicalOSSAConsumeInfo &consumes,
                                SmallVectorImpl<DestroyValueInst *> &destroys,
                                InstModCallbacks &callbacks) {
@@ -974,63 +974,13 @@ insertDestroyBeforeInstruction(SILInstruction *nextInstruction,
   SILBuilderWithScope builder(nextInstruction);
   auto loc =
       RegularLocation::getAutoGeneratedLocation(nextInstruction->getLoc());
-  auto *dvi =
-      builder.createDestroyValue(loc, currentDef, DontPoisonRefs, isDeadEnd);
+  auto *dvi = builder.createDestroyValue(loc, currentDef);
   callbacks.createdNewInst(dvi);
   consumes.recordFinalConsume(dvi);
   ++NumDestroysGenerated;
   destroys.push_back(dvi);
 }
 
-/// Whether a destroy created at \p inst should be marked [dead_end].
-///
-/// It should be if
-/// (1) \p inst is itself in a dead-end region
-/// (2) all destroys after \p inst are [dead_end]
-static IsDeadEnd_t
-isDeadEndDestroy(SILInstruction *inst,
-                 SmallPtrSetVector<SILInstruction *, 8> const &destroys,
-                 BasicBlockSet &semanticDestroysBlocks,
-                 DeadEndBlocks *deadEnds) {
-  auto *parent = inst->getParent();
-  if (!deadEnds->isDeadEnd(parent)) {
-    // Only destroys in dead-ends can be non-meaningful (aka "dead end").
-    return IsntDeadEnd;
-  }
-  if (semanticDestroysBlocks.contains(parent)) {
-    // `parent` has a semantic destroy somewhere.  Is it after `inst`?
-    for (auto *i = inst; i; i = i->getNextInstruction()) {
-      if (!destroys.contains(i)) {
-        continue;
-      }
-      auto *dvi = cast<DestroyValueInst>(i);
-      if (!dvi->isDeadEnd()) {
-        // Some subsequent destroy within `parent` was meaningful, so one
-        // created at `inst` must be too.
-        return IsntDeadEnd;
-      }
-    }
-  }
-  // Walk the portion of the dead-end region after `parent` to check that all
-  // destroys are non-meaningful.
-  BasicBlockWorklist worklist(inst->getFunction());
-  for (auto *successor : parent->getSuccessorBlocks()) {
-    worklist.push(successor);
-  }
-  while (auto *block = worklist.pop()) {
-    assert(deadEnds->isDeadEnd(block));
-    if (semanticDestroysBlocks.contains(block)) {
-      // Some subsequent destroy was meaningful, so one created at `inst`
-      // must be too.
-      return IsntDeadEnd;
-    }
-    for (auto *successor : block->getSuccessorBlocks()) {
-      worklist.pushIfNotVisited(successor);
-    }
-  }
-  return IsDeadEnd;
-}
-
 /// Inserts destroys along the boundary where needed and records all final
 /// consuming uses.
 ///
@@ -1042,18 +992,6 @@ isDeadEndDestroy(SILInstruction *inst,
 void CanonicalizeOSSALifetime::insertDestroysOnBoundary(
     PrunedLivenessBoundary const &boundary,
     SmallVectorImpl<DestroyValueInst *> &newDestroys) {
-  BasicBlockSet semanticDestroyBlocks(getCurrentDef()->getFunction());
-  for (auto *destroy : destroys) {
-    if (!cast<DestroyValueInst>(destroy)->isDeadEnd()) {
-      semanticDestroyBlocks.insert(destroy->getParent());
-    }
-  }
-  auto isDeadEnd = [&semanticDestroyBlocks,
-                    this](SILInstruction *inst) -> IsDeadEnd_t {
-    return isDeadEndDestroy(
-        inst, destroys, semanticDestroyBlocks,
-        deadEndBlocksAnalysis->get(getCurrentDef()->getFunction()));
-  };
   BasicBlockSet seenMergePoints(getCurrentDef()->getFunction());
   for (auto *instruction : boundary.lastUsers) {
     if (destroys.contains(instruction)) {
@@ -1075,8 +1013,7 @@ void CanonicalizeOSSALifetime::insertDestroysOnBoundary(
           }
           auto *insertionPoint = &*successor->begin();
           insertDestroyBeforeInstruction(insertionPoint, getCurrentDef(),
-                                         isDeadEnd(insertionPoint), consumes,
-                                         newDestroys, getCallbacks());
+                                         consumes, newDestroys, getCallbacks());
           LLVM_DEBUG(llvm::dbgs() << "  Destroy after terminator "
                                   << *instruction << " at beginning of ";
                      successor->printID(llvm::dbgs(), false);
@@ -1085,8 +1022,7 @@ void CanonicalizeOSSALifetime::insertDestroysOnBoundary(
         continue;
       }
       auto *insertionPoint = instruction->getNextInstruction();
-      insertDestroyBeforeInstruction(insertionPoint, getCurrentDef(),
-                                     isDeadEnd(insertionPoint), consumes,
+      insertDestroyBeforeInstruction(insertionPoint, getCurrentDef(), consumes,
                                      newDestroys, getCallbacks());
       LLVM_DEBUG(llvm::dbgs()
                  << "  Destroy at last use " << insertionPoint << "\n");
@@ -1095,25 +1031,22 @@ void CanonicalizeOSSALifetime::insertDestroysOnBoundary(
   }
   for (auto *edgeDestination : boundary.boundaryEdges) {
     auto *insertionPoint = &*edgeDestination->begin();
-    insertDestroyBeforeInstruction(insertionPoint, getCurrentDef(),
-                                   isDeadEnd(insertionPoint), consumes,
+    insertDestroyBeforeInstruction(insertionPoint, getCurrentDef(), consumes,
                                    newDestroys, getCallbacks());
     LLVM_DEBUG(llvm::dbgs() << "  Destroy on edge " << edgeDestination << "\n");
   }
   for (auto *def : boundary.deadDefs) {
     if (auto *arg = dyn_cast<SILArgument>(def)) {
       auto *insertionPoint = &*arg->getParent()->begin();
-      insertDestroyBeforeInstruction(insertionPoint, getCurrentDef(),
-                                     isDeadEnd(insertionPoint), consumes,
+      insertDestroyBeforeInstruction(insertionPoint, getCurrentDef(), consumes,
                                      newDestroys, getCallbacks());
       LLVM_DEBUG(llvm::dbgs()
                  << "  Destroy after dead def arg " << arg << "\n");
     } else {
       auto *instruction = cast<SILInstruction>(def);
       auto *insertionPoint = instruction->getNextInstruction();
       assert(insertionPoint && "def instruction was a terminator?!");
-      insertDestroyBeforeInstruction(insertionPoint, getCurrentDef(),
-                                     isDeadEnd(insertionPoint), consumes,
+      insertDestroyBeforeInstruction(insertionPoint, getCurrentDef(), consumes,
                                      newDestroys, getCallbacks());
       LLVM_DEBUG(llvm::dbgs()
                  << "  Destroy after dead def inst " << instruction << "\n");