Merge pull request #40500 from atrick/fix-semanticarc

SemanticARCOpts fix - remove deadEndBlocks checks

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.