Merge pull request #62454 from meg-gupta/guaranteedforwardingphiverify

Update guaranteed forwarding phi apis and verification

Author: swift-ci

include/swift/SIL/OwnershipUtils.h

@@ -69,50 +69,58 @@ struct BorrowedValue;
 // TODO: encapsulate in a ForwardingInstruction abstraction
 //===----------------------------------------------------------------------===//
 
-/// Is the opcode that produces \p value capable of forwarding guaranteed
+/// Is the opcode that produces \p value capable of forwarding inner guaranteed
 /// values?
 ///
 /// This may be true even if the current instance of the instruction is not a
 /// GuaranteedForwarding. If true, then the operation may be trivially rewritten
 /// with Guaranteed ownership.
-bool canOpcodeForwardGuaranteedValues(SILValue value);
+bool canOpcodeForwardInnerGuaranteedValues(SILValue value);
 
-/// Is the opcode that consumes \p use capable of forwarding guaranteed values?
+/// Is the opcode that consumes \p use capable of forwarding inner guaranteed
+/// values?
 ///
 /// 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);
+bool canOpcodeForwardInnerGuaranteedValues(Operand *use);
 
 // This is the use-def equivalent of use->getOperandOwnership() ==
 // 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;
+  // NOTE: canOpcodeForwardInnerGuaranteedValues returns true for transformation
+  // terminator results.
+  if (canOpcodeForwardInnerGuaranteedValues(value) ||
+      isa<SILFunctionArgument>(value)) {
+    return true;
   }
+  // If not a phi, return false
   auto *phi = dyn_cast<SILPhiArgument>(value);
   if (!phi || !phi->isPhi()) {
-    return true;
+    return false;
   }
-  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;
-      });
-
+  // For a phi, if we find GuaranteedForwarding phi operand on any incoming
+  // path, we return true. Additional verification is added to ensure
+  // GuaranteedForwarding phi operands are found on zero or all paths in the
+  // OwnershipVerifier.
+  bool isGuaranteedForwardingPhi = false;
+  phi->visitTransitiveIncomingPhiOperands([&](auto *, auto *op) -> bool {
+    auto opValue = op->get();
+    assert(opValue->getOwnershipKind().isCompatibleWith(
+        OwnershipKind::Guaranteed));
+    if (canOpcodeForwardInnerGuaranteedValues(opValue) ||
+        isa<SILFunctionArgument>(opValue)) {
+      isGuaranteedForwardingPhi = true;
+      return false;
+    }
+    auto *phi = dyn_cast<SILPhiArgument>(opValue);
+    if (!phi || !phi->isPhi()) {
+      return false;
+    }
+    return true;
+  });
   return isGuaranteedForwardingPhi;
 }
 
@@ -527,7 +535,7 @@ class BorrowedValueKind {
                        })) {
         return Kind::Invalid;
       }
-      if (isGuaranteedForwardingPhi(value)) {
+      if (isGuaranteedForwarding(value)) {
         return Kind::Invalid;
       }
       return Kind::Phi;

include/swift/SIL/SILValue.h

@@ -825,8 +825,6 @@ struct OperandOwnership {
     /// borrow scope.
     /// (tuple_extract, struct_extract, cast, switch)
     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)
@@ -919,7 +917,6 @@ inline OwnershipConstraint OperandOwnership::getOwnershipConstraint() {
     return {OwnershipKind::Owned, UseLifetimeConstraint::LifetimeEnding};
   case OperandOwnership::InteriorPointer:
   case OperandOwnership::GuaranteedForwarding:
-  case OperandOwnership::GuaranteedForwardingPhi:
     return {OwnershipKind::Guaranteed,
             UseLifetimeConstraint::NonLifetimeEnding};
   case OperandOwnership::EndBorrow:
@@ -948,7 +945,6 @@ inline bool canAcceptUnownedValue(OperandOwnership operandOwnership) {
   case OperandOwnership::ForwardingConsume:
   case OperandOwnership::InteriorPointer:
   case OperandOwnership::GuaranteedForwarding:
-  case OperandOwnership::GuaranteedForwardingPhi:
   case OperandOwnership::EndBorrow:
   case OperandOwnership::Reborrow:
     return false;

lib/SIL/IR/OperandOwnership.cpp

@@ -27,9 +27,6 @@ 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;
 }
 
@@ -430,8 +427,8 @@ OperandOwnership OperandOwnershipClassifier::visitBranchInst(BranchInst *bi) {
       bi->getDestBB()->getArgument(getOperandIndex())->getOwnershipKind();
 
   if (destBlockArgOwnershipKind == OwnershipKind::Guaranteed) {
-    return isGuaranteedForwardingPhi(getValue())
-               ? OperandOwnership::GuaranteedForwardingPhi
+    return isGuaranteedForwarding(getValue())
+               ? OperandOwnership::GuaranteedForwarding
                : OperandOwnership::Reborrow;
   }
   return destBlockArgOwnershipKind.getForwardingOperandOwnership(

lib/SIL/IR/SILValue.cpp

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

lib/SIL/Utils/OwnershipUtils.cpp

@@ -53,8 +53,6 @@ bool swift::hasPointerEscape(BorrowedValue value) {
     case OperandOwnership::InteriorPointer:
     case OperandOwnership::BitwiseEscape:
       break;
-
-    case OperandOwnership::GuaranteedForwardingPhi:
     case OperandOwnership::Reborrow: {
       SILArgument *phi = cast<BranchInst>(op->getUser())
                              ->getDestBB()
@@ -84,17 +82,23 @@ bool swift::hasPointerEscape(BorrowedValue value) {
   return false;
 }
 
-bool swift::canOpcodeForwardGuaranteedValues(SILValue value) {
-  if (auto *inst = value->getDefiningInstructionOrTerminator()) {
-    if (auto *mixin = OwnershipForwardingMixin::get(inst)) {
+bool swift::canOpcodeForwardInnerGuaranteedValues(SILValue value) {
+  // If we have an argument from a transforming terminator, we can forward
+  // guaranteed.
+  if (auto *arg = dyn_cast<SILArgument>(value))
+    if (auto *ti = arg->getSingleTerminator())
+      if (ti->mayHaveTerminatorResult())
+        return OwnershipForwardingMixin::get(ti)->preservesOwnership();
+
+  if (auto *inst = value->getDefiningInstruction())
+    if (auto *mixin = OwnershipForwardingMixin::get(inst))
       return mixin->preservesOwnership() &&
-        !isa<OwnedFirstArgForwardingSingleValueInst>(inst);
-    }
-  }
+             !isa<OwnedFirstArgForwardingSingleValueInst>(inst);
+
   return false;
 }
 
-bool swift::canOpcodeForwardGuaranteedValues(Operand *use) {
+bool swift::canOpcodeForwardInnerGuaranteedValues(Operand *use) {
   if (auto *mixin = OwnershipForwardingMixin::get(use->getUser()))
     return mixin->preservesOwnership() &&
            !isa<OwnedFirstArgForwardingSingleValueInst>(use->getUser());
@@ -213,6 +217,11 @@ bool swift::findInnerTransitiveGuaranteedUses(
         // Do not include transitive uses with 'none' ownership
         if (result->getOwnershipKind() == OwnershipKind::None)
           return true;
+        if (auto *phi = SILArgument::asPhi(result)) {
+          leafUse(use);
+          foundPointerEscape = true;
+          return true;
+        }
         for (auto *resultUse : result->getUses()) {
           if (resultUse->getOperandOwnership() != OperandOwnership::NonUse) {
             nonLeaf = true;
@@ -228,11 +237,6 @@ bool swift::findInnerTransitiveGuaranteedUses(
       }
       break;
     }
-    case OperandOwnership::GuaranteedForwardingPhi: {
-      leafUse(use);
-      foundPointerEscape = true;
-      break;
-    }
     case OperandOwnership::Borrow:
       // FIXME: Use visitExtendedScopeEndingUses and audit all clients to handle
       // reborrows.
@@ -330,17 +334,6 @@ bool swift::findExtendedUsesOfSimpleBorrowedValue(
       }
       recordUse(use);
       break;
-    // \p borrowedValue will dominate this GuaranteedForwardingPhi, because we
-    // return false in the case of Reborrow.
-    case OperandOwnership::GuaranteedForwardingPhi: {
-      SILArgument *phi = PhiOperand(use).getValue();
-      for (auto *use : phi->getUses()) {
-        if (use->getOperandOwnership() != OperandOwnership::NonUse)
-          worklist.insert(use);
-      }
-      recordUse(use);
-      break;
-    }
     case OperandOwnership::GuaranteedForwarding: {
       ForwardingOperand(use).visitForwardedValues([&](SILValue result) {
         // Do not include transitive uses with 'none' ownership
@@ -413,15 +406,12 @@ bool swift::visitGuaranteedForwardingPhisForSSAValue(
   // GuaranteedForwardingPhi uses.
   for (auto *use : value->getUses()) {
     if (use->getOperandOwnership() == OperandOwnership::GuaranteedForwarding) {
-      guaranteedForwardingOps.insert(use);
-      continue;
-    }
-    if (use->getOperandOwnership() ==
-        OperandOwnership::GuaranteedForwardingPhi) {
-      if (!visitor(use)) {
-        return false;
+      if (PhiOperand(use)) {
+        if (!visitor(use)) {
+          return false;
+        }
       }
-      continue;
+      guaranteedForwardingOps.insert(use);
     }
   }
 
@@ -431,15 +421,12 @@ bool swift::visitGuaranteedForwardingPhisForSSAValue(
       for (auto *valUse : val->getUses()) {
         if (valUse->getOperandOwnership() ==
             OperandOwnership::GuaranteedForwarding) {
-          guaranteedForwardingOps.insert(valUse);
-          continue;
-        }
-        if (valUse->getOperandOwnership() ==
-            OperandOwnership::GuaranteedForwardingPhi) {
-          if (!visitor(valUse)) {
-            return false;
+          if (PhiOperand(valUse)) {
+            if (!visitor(valUse)) {
+              return false;
+            }
           }
-          continue;
+          guaranteedForwardingOps.insert(valUse);
         }
       }
     }
@@ -1208,11 +1195,12 @@ bool swift::getAllBorrowIntroducingValues(SILValue inputValue,
   if (inputValue->getOwnershipKind() != OwnershipKind::Guaranteed)
     return false;
 
-  SmallVector<SILValue, 32> worklist;
-  worklist.emplace_back(inputValue);
+  SmallSetVector<SILValue, 32> worklist;
+  worklist.insert(inputValue);
 
-  while (!worklist.empty()) {
-    SILValue value = worklist.pop_back_val();
+  // worklist grows in this loop.
+  for (unsigned idx = 0; idx < worklist.size(); idx++) {
+    SILValue value = worklist[idx];
 
     // First check if v is an introducer. If so, stash it and continue.
     if (auto scopeIntroducer = BorrowedValue(value)) {
@@ -1230,11 +1218,26 @@ bool swift::getAllBorrowIntroducingValues(SILValue inputValue,
     // Otherwise if v is an ownership forwarding value, add its defining
     // instruction
     if (isGuaranteedForwarding(value)) {
-      if (auto *i = value->getDefiningInstructionOrTerminator()) {
-        llvm::copy(i->getNonTypeDependentOperandValues(),
-                   std::back_inserter(worklist));
+      if (auto *i = value->getDefiningInstruction()) {
+        for (SILValue opValue : i->getNonTypeDependentOperandValues()) {
+          worklist.insert(opValue);
+        }
         continue;
       }
+
+      // Otherwise, we should have a block argument that is defined by a single
+      // predecessor terminator.
+      auto *arg = cast<SILPhiArgument>(value);
+      if (arg->isTerminatorResult()) {
+        if (auto *forwardedOper = arg->forwardedTerminatorResultOperand()) {
+          worklist.insert(forwardedOper->get());
+          continue;
+        }
+      }
+      arg->visitIncomingPhiOperands([&](auto *operand) {
+        worklist.insert(operand->get());
+        return true;
+      });
     }
 
     // Otherwise, this is an introducer we do not understand. Bail and return
@@ -1373,31 +1376,16 @@ ForwardingOperand::ForwardingOperand(Operand *use) {
   if (use->isTypeDependent())
     return;
 
-  if (!OwnershipForwardingMixin::isa(use->getUser())) {
-    return;
-  }
-#ifndef NDEBUG
   switch (use->getOperandOwnership()) {
   case OperandOwnership::ForwardingUnowned:
   case OperandOwnership::ForwardingConsume:
   case OperandOwnership::GuaranteedForwarding:
+    this->use = use;
     break;
-  case OperandOwnership::NonUse:
-  case OperandOwnership::TrivialUse:
-  case OperandOwnership::InstantaneousUse:
-  case OperandOwnership::UnownedInstantaneousUse:
-  case OperandOwnership::PointerEscape:
-  case OperandOwnership::BitwiseEscape:
-  case OperandOwnership::Borrow:
-  case OperandOwnership::DestroyingConsume:
-  case OperandOwnership::InteriorPointer:
-  case OperandOwnership::GuaranteedForwardingPhi:
-  case OperandOwnership::EndBorrow:
-  case OperandOwnership::Reborrow:
-    llvm_unreachable("this isn't the operand being forwarding!");
+  default:
+    this->use = nullptr;
+    return;
   }
-#endif
-  this->use = use;
 }
 
 ValueOwnershipKind ForwardingOperand::getForwardingOwnershipKind() const {
@@ -1603,15 +1591,22 @@ bool ForwardingOperand::visitForwardedValues(
   // "transforming terminators"... We know that this means that we should at
   // most have a single phi argument.
   auto *ti = cast<TermInst>(user);
-  return llvm::all_of(ti->getSuccessorBlocks(), [&](SILBasicBlock *succBlock) {
-    // If we do not have any arguments, then continue.
-    if (succBlock->args_empty())
-      return true;
+  if (ti->mayHaveTerminatorResult()) {
+    return llvm::all_of(
+        ti->getSuccessorBlocks(), [&](SILBasicBlock *succBlock) {
+          // If we do not have any arguments, then continue.
+          if (succBlock->args_empty())
+            return true;
 
-    auto args = succBlock->getSILPhiArguments();
-    assert(args.size() == 1 && "Transforming terminator with multiple args?!");
-    return visitor(args[0]);
-  });
+          auto args = succBlock->getSILPhiArguments();
+          assert(args.size() == 1 &&
+                 "Transforming terminator with multiple args?!");
+          return visitor(args[0]);
+        });
+  }
+
+  auto *succArg = PhiOperand(use).getValue();
+  return visitor(succArg);
 }
 
 void swift::visitExtendedReborrowPhiBaseValuePairs(

lib/SIL/Utils/PrunedLiveness.cpp

@@ -318,10 +318,6 @@ PrunedLiveRange<LivenessWithDefs>::updateForDef(SILValue def) {
       updateForUse(use->getUser(), /*lifetimeEnding*/false);
       break;
     }
-    case OperandOwnership::GuaranteedForwardingPhi: {
-      updateForDef(PhiOperand(use).getValue());
-      break;
-    }
     default:
       updateForUse(use->getUser(), use->isLifetimeEnding());
       break;