Remove adjacent phis from OwnershipLiveness.

The extra complexity for traversing phis is not needed now that it handles
borrowed-from instructions. Remove the redundant logic because it complicates
the liveness algorithm and generates confusing results.

Author: atrick

include/swift/SIL/OwnershipLiveness.h

@@ -242,10 +242,6 @@ class InteriorLiveness : public OSSALiveness {
   // Summarize address uses
   AddressUseKind addressUseKind = AddressUseKind::Unknown;
 
-  // Record any guaranteed phi uses that are not already enclosed by an outer
-  // adjacent phi.
-  SmallVector<SILValue, 8> unenclosedPhis;
-
 public:
   InteriorLiveness(SILValue def): OSSALiveness(def) {}
 
@@ -259,8 +255,6 @@ class InteriorLiveness : public OSSALiveness {
 
   AddressUseKind getAddressUseKind() const { return addressUseKind; }
 
-  ArrayRef<SILValue> getUnenclosedPhis() const { return unenclosedPhis; }
-
   void print(llvm::raw_ostream &OS) const;
   void dump() const;
 };

include/swift/SIL/OwnershipUseVisitor.h

@@ -56,10 +56,6 @@ enum class OwnershipUseKind { LifetimeEnding, NonLifetimeEnding };
 ///
 /// - 'bool handleInnerBorrow(BorrowingOperand)' (default true)
 ///
-/// - 'bool handleInnerAdjacentReborrow(SILArgument *reborrow)' (default true)
-///
-/// - 'bool handleInnerReborrow(Operand *use)' (default true)
-///
 /// - 'bool handleScopedAddress(ScopedAddressValue)' (default true)
 ///
 /// These may be overridden with a transformation that adds uses to the use's
@@ -89,6 +85,9 @@ class OwnershipUseVisitorBase {
   /// continue processing extended liveness.
   ///
   /// Return true to continue visiting other uses.
+  ///
+  /// Note: this can lead to infinite recursion if the client does not maintain
+  /// a visited set.
   bool handleOuterReborrow(Operand *phiOper) { return true; }
 
   /// Handle a guaranteed forwarding instruction. After the returns, this will
@@ -110,24 +109,6 @@ class OwnershipUseVisitorBase {
     return true;
   }
 
-  /// Handles an inner adjacent phi where ownershipDef is a phi in the same
-  /// block.
-  ///
-  /// If this returns true, then handleUsePoint will be called on the
-  /// reborrow's immediate scope ending uses are visited, along with
-  /// handleInnerBorrow for any uses that are also reborrows.
-  bool handleInnerAdjacentReborrow(SILArgument *reborrow) {
-    return true;
-  }
-
-  /// Handle an inner reborrow. Later, the reborrow will itself be visited as a
-  /// use, but its scope's uses will not be transitively visited. The
-  /// implementation may track the reborrow to insert missing reborrows or to
-  /// continue processing extended liveness.
-  ///
-  /// Return true to continue visiting other uses.
-  bool handleInnerReborrow(Operand *phiOper) { return true; }
-
   /// If this returns true, then handleUsePoint will be called on the scope
   /// ending operands.
   ///
@@ -173,8 +154,6 @@ class OwnershipUseVisitor : OwnershipUseVisitorBase<Impl> {
 
   bool visitInnerBorrow(Operand *borrowingOperand);
 
-  bool visitInnerAdjacentReborrow(SILArgument *reborrow);
-  
   bool visitInnerBorrowScopeEnd(Operand *borrowEnd);
 
   bool visitOwnedUse(Operand *use);
@@ -295,32 +274,15 @@ bool OwnershipUseVisitor<Impl>::visitInnerBorrow(Operand *borrowingOperand) {
       });
 }
 
-template <typename Impl>
-bool OwnershipUseVisitor<Impl>::
-visitInnerAdjacentReborrow(SILArgument *reborrow) {
-  if (!asImpl().handleInnerAdjacentReborrow(reborrow))
-    return false;
-
-  return
-    BorrowedValue(reborrow).visitLocalScopeEndingUses([&](Operand *borrowEnd) {
-      return visitInnerBorrowScopeEnd(borrowEnd);
-    });
-}
-
 /// Note: borrowEnd->get() may be a borrow introducer for an inner scope, or a
 /// borrow scopes that does not introduce a borrowed value (begin_apply).
 template <typename Impl>
 bool OwnershipUseVisitor<Impl>::visitInnerBorrowScopeEnd(Operand *borrowEnd) {
   switch (borrowEnd->getOperandOwnership()) {
   case OperandOwnership::EndBorrow:
+  case OperandOwnership::Reborrow:
     return handleUsePoint(borrowEnd, UseLifetimeConstraint::NonLifetimeEnding);
 
-  case OperandOwnership::Reborrow: {
-    if (!asImpl().handleInnerReborrow(borrowEnd))
-      return false;
-
-    return handleUsePoint(borrowEnd, UseLifetimeConstraint::NonLifetimeEnding);
-  }
   case OperandOwnership::DestroyingConsume:
   case OperandOwnership::ForwardingConsume: {
     // partial_apply [on_stack] and mark_dependence [nonescaping] can introduce
@@ -353,17 +315,6 @@ bool OwnershipUseVisitor<Impl>::visitInnerBorrowScopeEnd(Operand *borrowEnd) {
 /// and treat them like inner borrows.
 template <typename Impl>
 bool OwnershipUseVisitor<Impl>::visitInteriorUses(SILValue ssaDef) {
-  // Inner adjacent reborrows are considered inner borrow scopes.
-  if (auto phi = SILArgument::asPhi(ssaDef)) {
-    if (!visitInnerAdjacentPhis(phi, [&](SILArgument *innerPhi) {
-          if (innerPhi->isGuaranteedForwarding()) {
-            return visitGuaranteedUses(innerPhi);
-          }
-          return visitInnerAdjacentReborrow(innerPhi);
-        })) {
-      return false;
-    }
-  }
   switch (ssaDef->getOwnershipKind()) {
   case OwnershipKind::Owned: {
     for (Operand *use : ssaDef->getUses()) {

lib/SIL/Utils/OSSALifetimeCompletion.cpp

@@ -524,11 +524,6 @@ bool OSSALifetimeCompletion::analyzeAndUpdateLifetime(SILValue value,
   }
   InteriorLiveness liveness(value);
   liveness.compute(domInfo, handleInnerScope);
-  // TODO: Rebuild outer adjacent phis on demand (SILGen does not currently
-  // produce guaranteed phis). See FindEnclosingDefs &
-  // findSuccessorDefsFromPredDefs. If no enclosing phi is found, we can create
-  // it here and use updateSSA to recursively populate phis.
-  assert(liveness.getUnenclosedPhis().empty());
   return endLifetimeAtBoundary(value, liveness.getLiveness(), boundary,
                                deadEndBlocks);
 }

lib/SIL/Utils/OwnershipLiveness.cpp

@@ -102,6 +102,14 @@ void LinearLiveness::compute() {
   LinearLivenessVisitor(*this).visitLifetimeEndingUses(ownershipDef);
 }
 
+/// Override OwnershipUseVisitor to callback to handleInnerScopeCallback for
+/// nested borrow scopes. This supports lifetime completion from the inside-out.
+///
+/// By default, handleOwnedPhi and handleOuterReborrow are already treated like
+/// a normal lifetime-ending use.
+///
+/// By default, handleGuaranteedForwardingPhi is already treated like a normal
+/// non-lifetime-ending use.
 struct InteriorLivenessVisitor :
   public OwnershipUseVisitor<InteriorLivenessVisitor> {
 
@@ -142,14 +150,7 @@ struct InteriorLivenessVisitor :
 
   bool handlePointerEscape(Operand *use) {
     interiorLiveness.addressUseKind = AddressUseKind::PointerEscape;
-    return true;
-  }
 
-  // handleOwnedPhi and handleOuterReborrow ends the linear lifetime.
-  // By default, they are treated like a normal lifetime-ending use.
-
-  bool handleGuaranteedForwardingPhi(Operand *use) {
-    recursivelyVisitInnerGuaranteedPhi(PhiOperand(use), /*reborrow*/false);
     return true;
   }
 
@@ -166,18 +167,6 @@ struct InteriorLivenessVisitor :
     return true;
   }
 
-  bool handleInnerAdjacentReborrow(SILArgument *reborrow) {
-    if (handleInnerScopeCallback) {
-      handleInnerScopeCallback(reborrow);
-    }
-    return true;
-  }
-
-  bool handleInnerReborrow(Operand *phiOper) {
-    recursivelyVisitInnerGuaranteedPhi(PhiOperand(phiOper), /*reborrow*/true);
-    return true;
-  }
-
   /// After this returns true, handleUsePoint will be called on the scope
   /// ending operands.
   ///
@@ -188,88 +177,8 @@ struct InteriorLivenessVisitor :
     }
     return true;
   }
-
-  void recursivelyVisitInnerGuaranteedPhi(PhiOperand phiOper, bool isReborrow);
 };
 
-// Dominating ownershipDef example: handleReborrow must continue visiting phi
-// uses:
-//
-// bb0:
-//  d1 = ...
-//  cond_br bb1, bb2
-// bb1:
-//   b1 = borrow d1
-//   br bb3(b1)
-// bb2:
-//   b2 = borrow d1
-//   br bb3(b2)
-// bb3(reborrow):
-//   u1 = d1
-//   u2 = reborrow
-//   // can't move destroy above u2
-//   destroy_value d1
-//
-// Dominating ownershipDef example: handleGuaranteedForwardingPhi must continue
-// visiting phi uses:
-//
-// bb0:
-//  b1 = borrow d1
-//  cond_br bb1, bb2
-// bb1:
-//   p1 = projection b1
-//   br bb3(p1)
-// bb2:
-//   p1 = projection b1
-//   br bb3(p2)
-// bb3(forwardingPhi):
-//   u1 = b1
-//   u2 = forwardingPhi
-//   // can't move end_borrow above u2
-//   end_borrow b1
-//
-// TODO: when phi's have a reborrow flag, remove \p isReborrow.
-void InteriorLivenessVisitor::
-recursivelyVisitInnerGuaranteedPhi(PhiOperand phiOper, bool isReborrow) {
-  SILValue phiValue = phiOper.getValue();
-  if (!visited.insert(phiValue))
-    return;
-
-  if (!visitEnclosingDefs(phiValue, [this](SILValue enclosingDef){
-    // If the enclosing def is \p ownershipDef, return false to check
-    // dominance.
-    if (enclosingDef == interiorLiveness.ownershipDef)
-      return false;
-
-    // Otherwise, phiValue is enclosed by an outer adjacent phi, so its scope
-    // does not contribute to the outer liveness. This phi will be recorded as a
-    // regular use by the visitor, and this enclosing def will be visited as
-    // separate lifetime-ending-use use. Return true to continue checking if any
-    // other enclosing defs do not have an outer adjacent reborrow.
-    return true;
-  })) {
-    // TODO: instead of relying on Dominance, we can reformulate this algorithm
-    // to detect redundant phis, similar to the SSAUpdater.
-    //
-    // At least one enclosing def is ownershipDef. If ownershipDef dominates
-    // phiValue, then this is consistent with a well-formed linear lifetime, and
-    // the phi's uses directly contribute to ownershipDef's liveness.
-    if (domInfo &&
-        domInfo->dominates(interiorLiveness.ownershipDef->getParentBlock(),
-                           phiValue->getParentBlock())) {
-      if (isReborrow) {
-        visitInnerBorrow(phiOper.getOperand());
-      } else {
-        visitInteriorUses(phiValue);
-      }
-      return;
-    }
-    // ownershipDef does not dominate this phi. Record it so the liveness
-    // client can use this information to insert the missing outer adjacent phi.
-    interiorLiveness.unenclosedPhis.push_back(phiValue);
-  }
-}
-
 void InteriorLiveness::compute(const DominanceInfo *domInfo, InnerScopeHandlerRef handleInnerScope) {
   liveness.initializeDef(ownershipDef);
   addressUseKind = AddressUseKind::NonEscaping;
@@ -291,11 +200,6 @@ void InteriorLiveness::print(llvm::raw_ostream &OS) const {
     OS << "Incomplete liveness: Unknown address use\n";
     break;
   }
-  OS << "Unenclosed phis {\n";
-  for (SILValue phi : getUnenclosedPhis()) {
-    OS << "  " << phi;
-  }
-  OS << "}\n";
 }
 
 void InteriorLiveness::dump() const { print(llvm::dbgs()); }