[ClosureLifetimeFixup] Flag dead-end destroys.

Author: nate-chandler

lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

@@ -14,21 +14,22 @@
 
 #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/OwnershipOptUtils.h"
 #include "swift/SILOptimizer/Utils/InstOptUtils.h"
+#include "swift/SILOptimizer/Utils/OwnershipOptUtils.h"
 #include "swift/SILOptimizer/Utils/SILSSAUpdater.h"
 #include "swift/SILOptimizer/Utils/StackNesting.h"
 
@@ -445,14 +446,6 @@ 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))
@@ -468,7 +461,7 @@ static void insertAfterClosureUser(SILInstruction *closureUser,
 
       for (auto eb : endBorrows) {
         SILBuilderWithScope builder(std::next(eb->getIterator()));
-        insertAtNonUnreachable(builder);
+        insertFn(builder);
       }
       return;
     }
@@ -479,12 +472,12 @@ static void insertAfterClosureUser(SILInstruction *closureUser,
     if (!endAsyncLet)
       return;
     SILBuilderWithScope builder(std::next(endAsyncLet->getIterator()));
-    insertAtNonUnreachable(builder);
+    insertFn(builder);
     return;
   }
   FullApplySite fas = FullApplySite::isa(closureUser);
   assert(fas);
-  fas.insertAfterApplication(insertAtNonUnreachable);
+  fas.insertAfterApplication(insertFn);
 }
 
 static SILValue skipConvert(SILValue v) {
@@ -1000,6 +993,7 @@ 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) {
@@ -1048,10 +1042,22 @@ static bool tryExtendLifetimeToLastUse(
   cvt->setLifetimeGuaranteed();
   cvt->setOperand(closureCopy);
 
-  insertAfterClosureUser(singleUser, [closureCopy](SILBuilder &builder) {
-    auto loc = RegularLocation(builder.getInsertionPointLoc());
-    builder.createDestroyValue(loc, 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);
+      });
   /*
   llvm::errs() << "after lifetime extension of\n";
   escapingClosure->dump();
@@ -1440,6 +1446,7 @@ static void computeUnreachableBlocks(
 
 static bool fixupClosureLifetimes(SILFunction &fn,
                                   DominanceAnalysis *dominanceAnalysis,
+                                  DeadEndBlocksAnalysis *deadEndBlocksAnalysis,
                                   bool &checkStackNesting, bool &modifiedCFG) {
   bool changed = false;
 
@@ -1476,7 +1483,8 @@ static bool fixupClosureLifetimes(SILFunction &fn,
         }
       }
 
-      if (tryExtendLifetimeToLastUse(cvt, dominanceAnalysis, memoizedQueries,
+      if (tryExtendLifetimeToLastUse(cvt, dominanceAnalysis,
+                                     deadEndBlocksAnalysis, memoizedQueries,
                                      unreachableBlocks, updater.getDeleter(),
                                      /*const*/ modifiedCFG)) {
         changed = true;
@@ -1516,9 +1524,11 @@ class ClosureLifetimeFixup : public SILFunctionTransform {
     bool modifiedCFG = false;
 
     auto *dominanceAnalysis = PM->getAnalysis<DominanceAnalysis>();
+    auto *deadEndBlocksAnalysis = getAnalysis<DeadEndBlocksAnalysis>();
 
     if (fixupClosureLifetimes(*getFunction(), dominanceAnalysis,
-                              checkStackNesting, modifiedCFG)) {
+                              deadEndBlocksAnalysis, checkStackNesting,
+                              modifiedCFG)) {
       updateBorrowedFrom(getPassManager(), getFunction());
       if (checkStackNesting){
         modifiedCFG |=

test/SILOptimizer/closure-lifetime-fixup.sil

@@ -14,6 +14,8 @@ sil @noescapeBlock3 : $@convention(c) (Optional<@convention(block) @noescape (Op
 sil @$sSS10FoundationE19_bridgeToObjectiveCSo8FakeNSStringCyF : $@convention(method) (@guaranteed String) -> @owned FakeNSString
 sil @$sSS21_builtinStringLiteral17utf8CodeUnitCount7isASCIISSBp_BwBi1_tcfC : $@convention(method) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) -> @owned String
 sil @$sSSSgIegg_So8FakeNSStringCSgIyBy_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> ()
+sil @nothrower : $@convention(thin) () -> Int 
+sil @rethrower : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> (Int, @error any Error)) -> (Int, @error any Error) 
 
 // Just make sure that we perform the optimization and do not trigger the ownership verifier.
 //
@@ -377,3 +379,34 @@ bb2(%19 : @owned $any Error):
   dealloc_stack %5 : $*Int
   throw %19 : $any Error
 }
+
+// The destroy that's created in the dead-end should be flagged [dead_end].
+// CHECK-LABEL: sil [ossa] @testRethrowingNonthrowing : {{.*}} {
+// CHECK:         [[NOTHROWER:%[^,]+]] = function_ref @nothrower
+// CHECK:         [[THICK_NOTHROWER:%[^,]+]] = thin_to_thick_function [[NOTHROWER]]
+// CHECK:         [[THROWING_NOTHROWER:%[^,]+]] = convert_function [[THICK_NOTHROWER]]
+// CHECK:         [[NOTHROWER_COPY:%[^,]+]] = copy_value [[THROWING_NOTHROWER]]
+// CHECK:       {{bb[0-9]+}}
+// CHECK:         destroy_value [[NOTHROWER_COPY]]
+// CHECK:       {{bb[0-9]+}}
+// CHECK:         destroy_value [dead_end] [[NOTHROWER_COPY]]
+// CHECK-LABEL: } // end sil function 'testRethrowingNonthrowing'
+sil [ossa] @testRethrowingNonthrowing : $@convention(thin) () -> () {
+bb0:
+  %nothrower = function_ref @nothrower : $@convention(thin) () -> Int 
+  %thick_nothrower = thin_to_thick_function %nothrower : $@convention(thin) () -> Int to $@callee_guaranteed () -> Int 
+  %throwing_nothrower = convert_function %thick_nothrower : $@callee_guaranteed () -> Int to $@callee_guaranteed () -> (Int, @error any Error) 
+  %nonescaping_nothrower = convert_escape_to_noescape [not_guaranteed] %throwing_nothrower : $@callee_guaranteed () -> (Int, @error any Error) to $@noescape @callee_guaranteed () -> (Int, @error any Error) 
+  %rethrower = function_ref @rethrower : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> (Int, @error any Error)) -> (Int, @error any Error) 
+  try_apply %rethrower(%nonescaping_nothrower) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> (Int, @error any Error)) -> (Int, @error any Error), normal bb1, error bb2 
+
+bb1(%val : $Int):                                   
+  destroy_value %nonescaping_nothrower : $@noescape @callee_guaranteed () -> (Int, @error any Error) 
+  %8 = tuple ()                                   
+  return %8 : $()                                 
+
+bb2(%error : @owned $any Error):                     
+  destroy_value [dead_end] %error : $any Error       
+  destroy_value [dead_end] %nonescaping_nothrower : $@noescape @callee_guaranteed () -> (Int, @error any Error) 
+  unreachable                                     
+}