Merge pull request #63435 from nate-chandler/cherrypick/508708919ae6bd49d85a6fe3da2ed53ed2cb3a1c

ClosureLifetimeFixup: Fix corner case of unconnected basic blocks

Author: tbkka

lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

@@ -21,6 +21,7 @@
 #include "swift/SIL/BasicBlockDatastructures.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/SILSSAUpdater.h"
@@ -477,6 +478,7 @@ static SILValue tryRewriteToPartialApplyStack(
     ConvertEscapeToNoEscapeInst *cvt, SILInstruction *closureUser,
     DominanceAnalysis *dominanceAnalysis, InstructionDeleter &deleter,
     llvm::DenseMap<SILInstruction *, SILInstruction *> &memoized,
+    llvm::DenseSet<SILBasicBlock *> &unreachableBlocks,
     const bool &modifiedCFG) {
 
   auto *origPA = dyn_cast<PartialApplyInst>(skipConvert(cvt->getOperand()));
@@ -583,6 +585,13 @@ static SILValue tryRewriteToPartialApplyStack(
   dominanceAnalysis->invalidate(closureUser->getFunction(),
                                 analysisInvalidationKind(modifiedCFG));
   // Insert dealloc_stacks of any in_guaranteed captures.
+
+  // Don't run insertDeallocOfCapturedArguments if newPA is in an unreachable
+  // block insertDeallocOfCapturedArguments will run code that computes the DF
+  // for newPA that will loop infinetly.
+  if (unreachableBlocks.count(newPA->getParent()))
+    return closure;
+
   insertDeallocOfCapturedArguments(
       newPA, dominanceAnalysis->get(closureUser->getFunction()));
   return closure;
@@ -591,6 +600,7 @@ static SILValue tryRewriteToPartialApplyStack(
 static bool tryExtendLifetimeToLastUse(
     ConvertEscapeToNoEscapeInst *cvt, DominanceAnalysis *dominanceAnalysis,
     llvm::DenseMap<SILInstruction *, SILInstruction *> &memoized,
+    llvm::DenseSet<SILBasicBlock *> &unreachableBlocks,
     InstructionDeleter &deleter, const bool &modifiedCFG) {
   // If there is a single user that is an apply this is simple: extend the
   // lifetime of the operand until after the apply.
@@ -614,7 +624,7 @@ static bool tryExtendLifetimeToLastUse(
 
   if (SILValue closure = tryRewriteToPartialApplyStack(
           cvt, singleUser, dominanceAnalysis, deleter, memoized,
-          /*const*/ modifiedCFG)) {
+          unreachableBlocks, /*const*/ modifiedCFG)) {
     if (auto *cfi = dyn_cast<ConvertFunctionInst>(closure))
       closure = cfi->getOperand();
     if (endAsyncLet && isa<MarkDependenceInst>(closure)) {
@@ -1009,6 +1019,22 @@ static bool fixupCopyBlockWithoutEscaping(CopyBlockWithoutEscapingInst *cb,
   return true;
 }
 
+static void computeUnreachableBlocks(
+  llvm::DenseSet<SILBasicBlock*> &unreachableBlocks,
+  SILFunction &fn) {
+
+  ReachableBlocks isReachable(&fn);
+  llvm::DenseSet<SILBasicBlock *> reachable;
+  isReachable.visit([&] (SILBasicBlock *block) -> bool {
+                    reachable.insert(block);
+                    return true;
+                   });
+  for (auto &block : fn) {
+    if (!reachable.count(&block))
+      unreachableBlocks.insert(&block);
+  }
+}
+
 static bool fixupClosureLifetimes(SILFunction &fn,
                                   DominanceAnalysis *dominanceAnalysis,
                                   bool &checkStackNesting, bool &modifiedCFG) {
@@ -1018,6 +1044,9 @@ static bool fixupClosureLifetimes(SILFunction &fn,
   // queries.
   llvm::DenseMap<SILInstruction *, SILInstruction *> memoizedQueries;
 
+  llvm::DenseSet<SILBasicBlock *> unreachableBlocks;
+  computeUnreachableBlocks(unreachableBlocks, fn);
+
   for (auto &block : fn) {
     SILSSAUpdater updater;
 
@@ -1045,7 +1074,7 @@ static bool fixupClosureLifetimes(SILFunction &fn,
       }
 
       if (tryExtendLifetimeToLastUse(cvt, dominanceAnalysis, memoizedQueries,
-                                     updater.getDeleter(),
+                                     unreachableBlocks, updater.getDeleter(),
                                      /*const*/ modifiedCFG)) {
         changed = true;
         checkStackNesting = true;

test/SILOptimizer/closure-lifetime-fixup-dead-block.sil

@@ -0,0 +1,54 @@
+// RUN: %target-sil-opt -enable-sil-verify-all -closure-lifetime-fixup %s | %FileCheck %s
+
+sil_stage raw
+
+
+import Builtin
+import Swift
+
+sil @cl : $@convention(thin) (@in_guaranteed Builtin.Int64) -> ()
+sil @f : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
+
+// This test case used to loop infinitely because it processed the dead block as
+// part of a dominance based algorithm.
+
+// CHECK: partial_apply
+
+sil [ossa] @repo : $@convention(thin) (Builtin.Int1, Builtin.Int1, @in_guaranteed Builtin.Int64) -> () {
+bb0(%0 : $Builtin.Int1, %1 : $Builtin.Int1, %2: $*Builtin.Int64):
+  cond_br %0, bb2, bb1
+
+bb1:
+  br bb7
+
+bb2:
+  br bb3
+
+bb3:
+  cond_br %1, bb4, bb5
+
+bb4:
+  br bb8
+
+bb5:
+  br bb8
+
+bb6:
+  %41 = alloc_stack $Builtin.Int64
+  copy_addr %2 to [init] %41 : $*Builtin.Int64
+        %40 = function_ref @cl : $@convention(thin) (@in_guaranteed Builtin.Int64) -> ()
+  %43 = partial_apply [callee_guaranteed] %40(%41) : $@convention(thin) (@in_guaranteed Builtin.Int64) -> ()
+  %44 = convert_escape_to_noescape [not_guaranteed] %43 : $@callee_guaranteed () -> () to $@noescape @callee_guaranteed () -> ()
+  %55 = function_ref @f : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
+  %56 = apply %55(%44) : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
+  destroy_value %43 : $@callee_guaranteed () -> ()
+  dealloc_stack %41 : $*Builtin.Int64
+  br bb8
+
+bb7:
+  br bb8
+
+bb8:
+  %r = tuple()
+  return %r : $()
+}