SemanticARCOpts fix - remove deadEndBlocks checks

SemanticARCOptVisitor::performGuaranteedCopyValueOptimization was
converting this SIL

    %borrow = begin_borrow %copiedValue
    %copy = copy_value %borrow
    %borrowCopy = begin_borrow %copy
    end_borrow %borrow
    end_borrow %borrowCopy
    destroy_value %copy
    // something something
    unreachable

into

    %borrow = begin_borrow %copiedValue
    %innerBorrow = begin_borrow %borrow
    end_borrow %borrow
    end_borrow %innerBorrow
    // something something
    unreachable

Dead-end blocks are simply irrelevant for this
optimization. Unfortunately, there were multiple layers of attempted
workarounds that were hiding the real problem, except in rare cases.

Thanks Nate Chandler for reducing the test.

Author: atrick

include/swift/SIL/OwnershipUtils.h

@@ -566,8 +566,11 @@ struct BorrowedValue {
   /// This ignores reborrows. The assumption is that, since \p uses are
   /// dominated by this local scope, checking the extended borrow scope should
   /// not be necessary to determine they are within the scope.
+  ///
+  /// \p deadEndBlocks is optional during transition. It will be completely
+  /// removed in an upcoming commit.
   bool areUsesWithinLocalScope(ArrayRef<Operand *> uses,
-                               DeadEndBlocks &deadEndBlocks) const;
+                               DeadEndBlocks *deadEndBlocks) const;
 
   /// Given a local borrow scope introducer, visit all non-forwarding consuming
   /// users. This means that this looks through guaranteed block arguments. \p

include/swift/SIL/PrunedLiveness.h

@@ -334,8 +334,9 @@ class PrunedLiveness {
   /// client already knows that inst occurs after the start of liveness.
   bool isWithinBoundary(SILInstruction *inst) const;
 
+  /// \p deadEndBlocks is optional.
   bool areUsesWithinBoundary(ArrayRef<Operand *> uses,
-                             DeadEndBlocks &deadEndBlocks) const;
+                             DeadEndBlocks *deadEndBlocks) const;
 
   /// Compute liveness for a single SSA definition.
   void computeSSALiveness(SILValue def);

lib/SIL/Utils/OwnershipUtils.cpp

@@ -627,7 +627,7 @@ void BorrowedValue::computeLiveness(PrunedLiveness &liveness) const {
 }
 
 bool BorrowedValue::areUsesWithinLocalScope(
-    ArrayRef<Operand *> uses, DeadEndBlocks &deadEndBlocks) const {
+    ArrayRef<Operand *> uses, DeadEndBlocks *deadEndBlocks) const {
   // First make sure that we actually have a local scope. If we have a non-local
   // scope, then we have something (like a SILFunctionArgument) where a larger
   // semantic construct (in the case of SILFunctionArgument, the function
@@ -906,14 +906,14 @@ bool AddressOwnership::areUsesWithinLifetime(
   SILValue root = base.getOwnershipReferenceRoot();
   BorrowedValue borrow(root);
   if (borrow)
-    return borrow.areUsesWithinLocalScope(uses, deadEndBlocks);
+    return borrow.areUsesWithinLocalScope(uses, &deadEndBlocks);
 
   // --- A reference no borrow scope. Currently happens for project_box.
 
   // Compute the reference value's liveness.
   PrunedLiveness liveness;
   liveness.computeSSALiveness(root);
-  return liveness.areUsesWithinBoundary(uses, deadEndBlocks);
+  return liveness.areUsesWithinBoundary(uses, &deadEndBlocks);
 }
 
 //===----------------------------------------------------------------------===//

lib/SIL/Utils/PrunedLiveness.cpp

@@ -154,10 +154,14 @@ bool PrunedLiveness::isWithinBoundary(SILInstruction *inst) const {
 }
 
 bool PrunedLiveness::areUsesWithinBoundary(ArrayRef<Operand *> uses,
-                                           DeadEndBlocks &deadEndBlocks) const {
+                                           DeadEndBlocks *deadEndBlocks) const {
+  auto checkDeadEnd = [deadEndBlocks](SILInstruction *inst) {
+    return deadEndBlocks && deadEndBlocks->isDeadEnd(inst->getParent());
+  };
+
   for (auto *use : uses) {
     auto *user = use->getUser();
-    if (!isWithinBoundary(user) && !deadEndBlocks.isDeadEnd(user->getParent()))
+    if (!isWithinBoundary(user) && !checkDeadEnd(user))
       return false;
   }
   return true;

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

@@ -153,6 +153,10 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(
   // dead end blocks that use the value in a non-consuming way.
   //
   // TODO: There may be some way of sinking this into the loop below.
+  //
+  // FIXME: The haveAnyLocalScopes and destroy.empty() checks are relics of
+  // attempts to handle dead end blocks during areUsesWithinLocalScope. If we
+  // don't use dead end blocks at all, they should not be relevant.
   bool haveAnyLocalScopes =
       llvm::any_of(borrowScopeIntroducers, [](BorrowedValue borrowScope) {
         return borrowScope.isLocalScope();
@@ -177,17 +181,16 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(
   //    anyways, we want to be conservative here and optimize only if we do not
   //    need to insert an end_borrow since all of our borrow introducers are
   //    non-local scopes.
+  //
+  // The call to areUsesWithinLocalScope cannot consider dead-end blocks. A
+  // local borrow scope requires all its inner uses to be inside the borrow
+  // scope, regardless of whether the end of the scope is inside a dead-end
+  // block.
   {
-    bool foundNonDeadEnd = false;
-    for (auto *d : destroys) {
-      foundNonDeadEnd |= !getDeadEndBlocks().isDeadEnd(d->getParentBlock());
-    }
-    if (!foundNonDeadEnd && haveAnyLocalScopes)
-      return false;
     SmallVector<Operand *, 8> scratchSpace;
     if (llvm::any_of(borrowScopeIntroducers, [&](BorrowedValue borrowScope) {
-          return !borrowScope.areUsesWithinLocalScope(lr.getAllConsumingUses(),
-                                                      getDeadEndBlocks());
+      return !borrowScope.areUsesWithinLocalScope(lr.getAllConsumingUses(),
+                                                  nullptr);
         })) {
       return false;
     }
@@ -216,7 +219,7 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(
 
       if (llvm::any_of(borrowScopeIntroducers, [&](BorrowedValue borrowScope) {
             return !borrowScope.areUsesWithinLocalScope(
-                phiArgLR.getAllConsumingUses(), getDeadEndBlocks());
+                phiArgLR.getAllConsumingUses(), nullptr);
           })) {
         return false;
       }

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

@@ -176,7 +176,7 @@ GuaranteedOwnershipExtension::checkBorrowExtension(
   assert(guaranteedLiveness.empty());
   borrow.computeLiveness(guaranteedLiveness);
 
-  if (guaranteedLiveness.areUsesWithinBoundary(newUses, deBlocks))
+  if (guaranteedLiveness.areUsesWithinBoundary(newUses, &deBlocks))
     return Valid; // reuse the borrow scope as-is
 
   beginBorrow = dyn_cast<BeginBorrowInst>(borrow.value);
@@ -224,7 +224,7 @@ GuaranteedOwnershipExtension::checkLifetimeExtension(
       ownedConsumeBlocks.push_back(user->getParent());
     }
   }
-  if (ownedLifetime.areUsesWithinBoundary(newUses, deBlocks))
+  if (ownedLifetime.areUsesWithinBoundary(newUses, &deBlocks))
     return Valid;
 
   return ExtendLifetime; // Can't cover newUses without destroy sinking.