Merge pull request #61505 from meg-gupta/guaranteedphis

Support @guaranteed forwarding phis

Author: swift-ci

include/swift/SIL/LinearLifetimeChecker.h

@@ -53,7 +53,7 @@ class LinearLifetimeChecker {
   class ErrorBuilder;
 
 private:
-  friend class ReborrowVerifier;
+  friend class GuaranteedPhiVerifier;
   friend class SILOwnershipVerifier;
   friend class SILValueOwnershipChecker;
 

include/swift/SIL/OwnershipUtils.h

@@ -73,23 +73,49 @@ struct BorrowedValue;
 /// values?
 ///
 /// This may be true even if the current instance of the instruction is not a
-/// ForwardingBorrow. If true, then the operation may be trivially rewritten
+/// GuaranteedForwarding. If true, then the operation may be trivially rewritten
 /// with Guaranteed ownership.
 bool canOpcodeForwardGuaranteedValues(SILValue value);
 
 /// Is the opcode that consumes \p use capable of forwarding guaranteed values?
 ///
-/// This may be true even if \p use is not a ForwardingBorrow. If true, then the
-/// operation may be trivially rewritten with Guaranteed ownership.
+/// This may be true even if \p use is not a GuaranteedForwarding. If true, then
+/// the operation may be trivially rewritten with Guaranteed ownership.
 bool canOpcodeForwardGuaranteedValues(Operand *use);
 
 // This is the use-def equivalent of use->getOperandOwnership() ==
-// OperandOwnership::ForwardingBorrow.
-inline bool isForwardingBorrow(SILValue value) {
+// OperandOwnership::GuaranteedForwarding.
+inline bool isGuaranteedForwarding(SILValue value) {
   assert(value->getOwnershipKind() == OwnershipKind::Guaranteed);
   return canOpcodeForwardGuaranteedValues(value);
 }
 
+/// Returns true if it is a forwarding phi
+inline bool isGuaranteedForwardingPhi(SILValue value) {
+  if (value->getOwnershipKind() != OwnershipKind::Guaranteed) {
+    return false;
+  }
+  if (isa<BeginBorrowInst>(value) || isa<LoadBorrowInst>(value)) {
+    return false;
+  }
+  auto *phi = dyn_cast<SILPhiArgument>(value);
+  if (!phi || !phi->isPhi()) {
+    return true;
+  }
+  bool isGuaranteedForwardingPhi = true;
+  phi->visitTransitiveIncomingPhiOperands(
+      [&](auto *phi, auto *operand) -> bool {
+        if (isa<BeginBorrowInst>(operand->get()) ||
+            isa<LoadBorrowInst>(operand->get())) {
+          isGuaranteedForwardingPhi = false;
+          return false;
+        }
+        return true;
+      });
+
+  return isGuaranteedForwardingPhi;
+}
+
 /// Is the opcode that produces \p value capable of forwarding owned values?
 ///
 /// This may be true even if the current instance of the instruction is not a
@@ -215,6 +241,11 @@ bool findExtendedTransitiveGuaranteedUses(
 bool findUsesOfSimpleValue(SILValue value,
                            SmallVectorImpl<Operand *> *usePoints = nullptr);
 
+/// Visit all GuaranteedForwardingPhis of \p value, not looking through
+/// reborrows.
+bool visitGuaranteedForwardingPhisForSSAValue(
+    SILValue value, function_ref<bool(Operand *)> func);
+
 /// An operand that forwards ownership to one or more results.
 class ForwardingOperand {
   Operand *use = nullptr;
@@ -447,13 +478,7 @@ struct BorrowingOperand {
   /// set of its operands uses.
   ///
   /// E.x.: end_apply uses.
-  ///
-  /// \p errorFunction a callback that if non-null is passed an operand that
-  /// triggers a mal-formed SIL error. This is just needed for the ownership
-  /// verifier to emit good output.
-  void getImplicitUses(
-      SmallVectorImpl<Operand *> &foundUses,
-      std::function<void(Operand *)> *errorFunction = nullptr) const;
+  void getImplicitUses(SmallVectorImpl<Operand *> &foundUses) const;
 
   void print(llvm::raw_ostream &os) const;
   SWIFT_DEBUG_DUMP { print(llvm::dbgs()); }
@@ -1249,14 +1274,19 @@ OwnedValueIntroducer getSingleOwnedValueIntroducer(SILValue value);
 
 using BaseValueSet = SmallPtrSet<SILValue, 8>;
 
-/// Starting from \p initialScopeOperand, find all reborrows and their
-/// corresponding base values, and run the visitor function \p
-/// visitReborrowBaseValuePair on them.
-///  Note that a reborrow phi, can have different base values based on different
-/// control flow paths.
-void findTransitiveReborrowBaseValuePairs(
-    BorrowingOperand initialScopeOperand, SILValue origBaseValue,
-    function_ref<void(SILPhiArgument *, SILValue)> visitReborrowBaseValuePair);
+/// Starting from \p borrowInst, find all reborrows along with their base
+/// values, and run the visitor function \p visitReborrowPhiBaseValuePair on
+/// them.
+void visitExtendedReborrowPhiBaseValuePairs(
+    BeginBorrowInst *borrowInst, function_ref<void(SILPhiArgument *, SILValue)>
+                                     visitReborrowPhiBaseValuePair);
+
+/// Starting from \p borrow, find all GuaranteedForwardingPhi uses along with
+/// their base values, and run the visitor function \p
+/// visitGuaranteedForwardingPhiBaseValuePair on them.
+void visitExtendedGuaranteedForwardingPhiBaseValuePairs(
+    BorrowedValue borrow, function_ref<void(SILPhiArgument *, SILValue)>
+                              visitGuaranteedForwardingPhiBaseValuePair);
 
 /// Visit the phis in the same block as \p phi which are reborrows of a borrow
 /// of one of the values reaching \p phi.

include/swift/SIL/SILValue.h

@@ -832,7 +832,9 @@ struct OperandOwnership {
     /// Forwarded Borrow. Propagates the guaranteed value within the base's
     /// borrow scope.
     /// (tuple_extract, struct_extract, cast, switch)
-    ForwardingBorrow,
+    GuaranteedForwarding,
+    /// A GuaranteedForwarding value passed as a phi operand.
+    GuaranteedForwardingPhi,
     /// End Borrow. End the borrow scope opened directly by the operand.
     /// The operand must be a begin_borrow, begin_apply, or function argument.
     /// (end_borrow, end_apply)
@@ -886,10 +888,11 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
 ///
 /// Forwarding instructions that produce Owned or Guaranteed values always
 /// forward an operand of the same ownership kind. Each case has a distinct
-/// OperandOwnership (ForwardingConsume and ForwardingBorrow), which enforces a
-/// specific constraint on the operand's ownership. Forwarding instructions that
-/// produce an Unowned value, however, may forward an operand of any
-/// ownership. Therefore, ForwardingUnowned is mapped to OwnershipKind::Any.
+/// OperandOwnership (ForwardingConsume and GuaranteedForwarding), which
+/// enforces a specific constraint on the operand's ownership. Forwarding
+/// instructions that produce an Unowned value, however, may forward an operand
+/// of any ownership. Therefore, ForwardingUnowned is mapped to
+/// OwnershipKind::Any.
 ///
 /// This design yields the following advantages:
 ///
@@ -923,7 +926,8 @@ inline OwnershipConstraint OperandOwnership::getOwnershipConstraint() {
   case OperandOwnership::ForwardingConsume:
     return {OwnershipKind::Owned, UseLifetimeConstraint::LifetimeEnding};
   case OperandOwnership::InteriorPointer:
-  case OperandOwnership::ForwardingBorrow:
+  case OperandOwnership::GuaranteedForwarding:
+  case OperandOwnership::GuaranteedForwardingPhi:
     return {OwnershipKind::Guaranteed,
             UseLifetimeConstraint::NonLifetimeEnding};
   case OperandOwnership::EndBorrow:
@@ -951,7 +955,8 @@ inline bool canAcceptUnownedValue(OperandOwnership operandOwnership) {
   case OperandOwnership::DestroyingConsume:
   case OperandOwnership::ForwardingConsume:
   case OperandOwnership::InteriorPointer:
-  case OperandOwnership::ForwardingBorrow:
+  case OperandOwnership::GuaranteedForwarding:
+  case OperandOwnership::GuaranteedForwardingPhi:
   case OperandOwnership::EndBorrow:
   case OperandOwnership::Reborrow:
     return false;
@@ -990,7 +995,7 @@ ValueOwnershipKind::getForwardingOperandOwnership(bool allowUnowned) const {
   case OwnershipKind::None:
     return OperandOwnership::TrivialUse;
   case OwnershipKind::Guaranteed:
-    return OperandOwnership::ForwardingBorrow;
+    return OperandOwnership::GuaranteedForwarding;
   case OwnershipKind::Owned:
     return OperandOwnership::ForwardingConsume;
   }

lib/SIL/IR/OperandOwnership.cpp

@@ -27,6 +27,9 @@ bool swift::checkOperandOwnershipInvariants(const Operand *operand,
     // Must be a valid BorrowingOperand.
     return bool(BorrowingOperand(const_cast<Operand *>(operand)));
   }
+  if (opOwnership == OperandOwnership::GuaranteedForwarding) {
+    return canOpcodeForwardGuaranteedValues(const_cast<Operand *>(operand));
+  }
   return true;
 }
 
@@ -297,14 +300,14 @@ OPERAND_OWNERSHIP(InteriorPointer, HopToExecutor)
 OPERAND_OWNERSHIP(InteriorPointer, ExtractExecutor)
 
 // Instructions that propagate a value within a borrow scope.
-OPERAND_OWNERSHIP(ForwardingBorrow, TupleExtract)
-OPERAND_OWNERSHIP(ForwardingBorrow, StructExtract)
-OPERAND_OWNERSHIP(ForwardingBorrow, DifferentiableFunctionExtract)
-OPERAND_OWNERSHIP(ForwardingBorrow, LinearFunctionExtract)
+OPERAND_OWNERSHIP(GuaranteedForwarding, TupleExtract)
+OPERAND_OWNERSHIP(GuaranteedForwarding, StructExtract)
+OPERAND_OWNERSHIP(GuaranteedForwarding, DifferentiableFunctionExtract)
+OPERAND_OWNERSHIP(GuaranteedForwarding, LinearFunctionExtract)
 // FIXME: OpenExistential[Box]Value should be able to take owned values too by
 // using getForwardingOperandOwnership.
-OPERAND_OWNERSHIP(ForwardingBorrow, OpenExistentialValue)
-OPERAND_OWNERSHIP(ForwardingBorrow, OpenExistentialBoxValue)
+OPERAND_OWNERSHIP(GuaranteedForwarding, OpenExistentialValue)
+OPERAND_OWNERSHIP(GuaranteedForwarding, OpenExistentialBoxValue)
 
 OPERAND_OWNERSHIP(EndBorrow, EndBorrow)
 
@@ -335,7 +338,7 @@ OPERAND_OWNERSHIP(EndBorrow, AbortApply)
 #undef OPERAND_OWNERSHIP
 
 // Forwarding operations are conditionally either ForwardingConsumes or
-// ForwardingBorrows, depending on the instruction's constant ownership
+// GuaranteedForwarding, depending on the instruction's constant ownership
 // attribute.
 #define FORWARDING_OWNERSHIP(INST)                                             \
   OperandOwnership OperandOwnershipClassifier::visit##INST##Inst(              \
@@ -376,7 +379,7 @@ FORWARDING_ANY_OWNERSHIP(CheckedCastBranch)
 // the meet of its operands' ownership. A destructured member has the same
 // ownership as its aggregate unless its type gives it None ownership.
 //
-// TODO: Aggregate operations should be Reborrows, not ForwardingBorrows,
+// TODO: Aggregate operations should be Reborrows, not GuaranteedForwarding,
 // because the borrowed value is different on either side of the operation and
 // the lifetimes of borrowed members could differ.
 #define AGGREGATE_OWNERSHIP(INST)                                              \
@@ -422,15 +425,15 @@ OperandOwnershipClassifier::visitSelectEnumInst(SelectEnumInst *i) {
 OperandOwnership
 OperandOwnershipClassifier::visitSelectValueInst(SelectValueInst *i) {
   if (getValue() == i->getDefaultResult())
-    return OperandOwnership::ForwardingBorrow;
+    return OperandOwnership::GuaranteedForwarding;
 
   for (unsigned idx = 0, endIdx = i->getNumCases(); idx < endIdx; ++idx) {
     SILValue casevalue;
     SILValue result;
     std::tie(casevalue, result) = i->getCase(idx);
 
     if (getValue() == casevalue) {
-      return OperandOwnership::ForwardingBorrow;
+      return OperandOwnership::GuaranteedForwarding;
     }
   }
   return OperandOwnership::TrivialUse;
@@ -440,10 +443,10 @@ OperandOwnership OperandOwnershipClassifier::visitBranchInst(BranchInst *bi) {
   ValueOwnershipKind destBlockArgOwnershipKind =
       bi->getDestBB()->getArgument(getOperandIndex())->getOwnershipKind();
 
-  // FIXME: remove this special case once all aggregate operations behave just
-  // like phis.
   if (destBlockArgOwnershipKind == OwnershipKind::Guaranteed) {
-    return OperandOwnership::Reborrow;
+    return isGuaranteedForwardingPhi(getValue())
+               ? OperandOwnership::GuaranteedForwardingPhi
+               : OperandOwnership::Reborrow;
   }
   return destBlockArgOwnershipKind.getForwardingOperandOwnership(
     /*allowUnowned*/true);

lib/SIL/IR/SILArgument.cpp

@@ -240,9 +240,8 @@ bool SILPhiArgument::visitTransitiveIncomingPhiOperands(
     argument->getIncomingPhiOperands(operands);
 
     for (auto *operand : operands) {
-      SILPhiArgument *forwarded;
-      if ((forwarded = dyn_cast<SILPhiArgument>(operand->get())) &&
-          forwarded->isPhi()) {
+      SILPhiArgument *forwarded = dyn_cast<SILPhiArgument>(operand->get());
+      if (forwarded && forwarded->isPhi()) {
         worklist.insert(forwarded);
       }
       if (!visitor(argument, operand))

lib/SIL/IR/SILValue.cpp

@@ -430,8 +430,10 @@ StringRef OperandOwnership::asString() const {
     return "forwarding-consume";
   case OperandOwnership::InteriorPointer:
     return "interior-pointer";
-  case OperandOwnership::ForwardingBorrow:
-    return "forwarding-borrow";
+  case OperandOwnership::GuaranteedForwarding:
+    return "guaranteed-forwarding";
+  case OperandOwnership::GuaranteedForwardingPhi:
+    return "guaranteed-forwarding-phi";
   case OperandOwnership::EndBorrow:
     return "end-borrow";
   case OperandOwnership::Reborrow: