Merge pull request #62493 from atrick/term-result-operand

APIs for terminator results that forward ownership

Author: atrick

include/swift/SIL/SILArgument.h

@@ -126,7 +126,7 @@ class SILArgument : public ValueBase {
   unsigned getIndex() const;
 
   /// Return non-null if \p value is a phi.
-  static SILPhiArgument *isPhi(SILValue value);
+  static SILPhiArgument *asPhi(SILValue value);
 
   /// Return non-null if \p value is a terminator result.
   static SILPhiArgument *isTerminatorResult(SILValue value);
@@ -145,10 +145,15 @@ class SILArgument : public ValueBase {
   /// If this argument is a phi, populate `OutArray` with the incoming phi
   /// values for each predecessor BB. If this argument is not a phi, return
   /// false.
+  ///
+  /// If this block has no predecessors, returnedPhiValues will be empty.
   bool getIncomingPhiValues(SmallVectorImpl<SILValue> &returnedPhiValues) const;
 
   /// If this argument is a phi, populate `OutArray` with each predecessor block
   /// and its incoming phi value. If this argument is not a phi, return false.
+  ///
+  /// If this block has no predecessors, returnedPredAndPhiValuePairs will be
+  /// empty.
   bool
   getIncomingPhiValues(SmallVectorImpl<std::pair<SILBasicBlock *, SILValue>>
                            &returnedPredAndPhiValuePairs) const;
@@ -187,14 +192,6 @@ class SILArgument : public ValueBase {
       SmallVectorImpl<std::pair<SILBasicBlock *, SILValue>>
           &returnedSingleTermOperands) const;
 
-  /// If this SILArgument's parent block has a single predecessor whose
-  /// terminator has a single operand, return the incoming operand of the
-  /// predecessor's terminator. Returns SILValue() otherwise.  Note that for
-  /// some predecessor terminators the incoming value is not exactly the
-  /// argument value. E.g. the incoming value for a switch_enum payload argument
-  /// is the enum itself (the operand of the switch_enum).
-  SILValue getSingleTerminatorOperand() const;
-
   /// If this SILArgument's parent block has a single predecessor whose
   /// terminator has a single operand, return that terminator.
   TermInst *getSingleTerminator() const;
@@ -203,6 +200,13 @@ class SILArgument : public ValueBase {
   /// otherwise return nullptr.
   TermInst *getTerminatorForResult() const;
 
+  /// If this terminator result forwards an operand, then return it.
+  ///
+  /// Precondition: this->isTerminatorResult()
+  ///
+  /// TODO: Move this and other APIs into a TerminatorResult abstraction.
+  const Operand *forwardedTerminatorResultOperand() const;
+
   /// Return the SILArgumentKind of this argument.
   SILArgumentKind getKind() const {
     return SILArgumentKind(ValueBase::getKind());
@@ -310,14 +314,6 @@ class SILPhiArgument : public SILArgument {
       SmallVectorImpl<std::pair<SILBasicBlock *, SILValue>>
           &returnedSingleTermOperands) const;
 
-  /// If this SILArgument's parent block has a single predecessor whose
-  /// terminator has a single operand, return the incoming operand of the
-  /// predecessor's terminator. Returns SILValue() otherwise.  Note that for
-  /// some predecessor terminators the incoming value is not exactly the
-  /// argument value. E.g. the incoming value for a switch_enum payload argument
-  /// is the enum itself (the operand of the switch_enum).
-  SILValue getSingleTerminatorOperand() const;
-
   /// If this SILArgument's parent block has a single predecessor whose
   /// terminator has a single operand, return that terminator.
   TermInst *getSingleTerminator() const;
@@ -406,7 +402,7 @@ class SILFunctionArgument : public SILArgument {
 //===----------------------------------------------------------------------===//
 
 /// Return non-null if \p value is a real phi argument.
-inline SILPhiArgument *SILArgument::isPhi(SILValue value) {
+inline SILPhiArgument *SILArgument::asPhi(SILValue value) {
   if (auto *arg = dyn_cast<SILPhiArgument>(value)) {
     if (arg->isPhi())
       return arg;

include/swift/SIL/SILBasicBlock.h

@@ -440,7 +440,7 @@ public SwiftObjectHeader {
   /// Whether the block has any phi arguments.
   ///
   /// Note that a block could have an argument and still return false.  The
-  /// argument must also satisfy SILPhiArgument::isPhiArgument.
+  /// argument must also satisfy SILPhiArgument::isPhi.
   bool hasPhi() const;
 
   //===--------------------------------------------------------------------===//
@@ -541,6 +541,8 @@ struct PhiOperand {
 
   PhiOperand() = default;
 
+  // This if \p operand is a CondBrInst operand, then this constructs and
+  // invalid PhiOperand. The abstraction only works for non-trivial OSSA values.
   PhiOperand(Operand *operand) {
     auto *branch = dyn_cast<BranchInst>(operand->getUser());
     if (!branch)
@@ -567,8 +569,8 @@ struct PhiOperand {
   }
 
   SILPhiArgument *getValue() const {
-    auto *branch = cast<BranchInst>(predBlock->getTerminator());
-    return cast<SILPhiArgument>(branch->getDestBB()->getArgument(argIndex));
+    return
+      cast<SILPhiArgument>(getBranch()->getDestBB()->getArgument(argIndex));
   }
 
   SILValue getSource() const {
@@ -591,12 +593,10 @@ struct PhiValue {
   PhiValue() = default;
 
   PhiValue(SILValue value) {
-    auto *blockArg = dyn_cast<SILPhiArgument>(value);
-    if (!blockArg || !blockArg->isPhi())
-      return;
-
-    phiBlock = blockArg->getParent();
-    argIndex = blockArg->getIndex();
+    if (auto *blockArg = SILArgument::asPhi(value)) {
+      phiBlock = blockArg->getParent();
+      argIndex = blockArg->getIndex();
+    }
   }
 
   explicit operator bool() const { return phiBlock != nullptr; }
@@ -612,8 +612,12 @@ struct PhiValue {
   }
 
   Operand *getOperand(SILBasicBlock *predecessor) {
-    auto *branch = cast<BranchInst>(predecessor->getTerminator());
-    return &branch->getAllOperands()[argIndex];
+    auto *term = predecessor->getTerminator();
+    if (auto *branch = dyn_cast<BranchInst>(term)) {
+      return &branch->getAllOperands()[argIndex];
+    }
+    // TODO: Support CondBr for legacy reasons
+    return cast<CondBranchInst>(term)->getOperandForDestBB(phiBlock, argIndex);
   }
 
   operator SILValue() const { return getValue(); }

include/swift/SIL/SILBuilder.h

@@ -831,12 +831,8 @@ class SILBuilder {
   }
 
   EndBorrowInst *createEndBorrow(SILLocation loc, SILValue borrowedValue) {
-    if (auto *arg = dyn_cast<SILPhiArgument>(borrowedValue)) {
-      if (auto *ti = arg->getSingleTerminator()) {
-        assert(!ti->isTransformationTerminator() &&
-               "Transforming terminators do not have end_borrow");
-      }
-    }
+    assert(!SILArgument::isTerminatorResult(borrowedValue) &&
+               "terminator results do not have end_borrow");
     return insert(new (getModule())
                       EndBorrowInst(getSILDebugLocation(loc), borrowedValue));
   }

include/swift/SIL/SILInstruction.h

@@ -1182,6 +1182,9 @@ class OwnershipForwardingMixin {
 ///
 /// The ownership kind is set on construction and afterwards must be changed
 /// explicitly using setOwnershipKind().
+///
+/// TODO: This name is extremely misleading because it may apply to an
+/// operation that has no operand at all, like `enum .None`.
 class FirstArgOwnershipForwardingSingleValueInst
     : public SingleValueInstruction,
       public OwnershipForwardingMixin {
@@ -1668,6 +1671,8 @@ class UnaryInstructionBase : public InstructionBase<Kind, Base> {
 
   Operand &getOperandRef() { return Operands[0]; }
 
+  const Operand &getOperandRef() const { return Operands[0]; }
+
   ArrayRef<Operand> getAllOperands() const { return Operands.asArray(); }
   MutableArrayRef<Operand> getAllOperands() { return Operands.asArray(); }
 
@@ -1808,6 +1813,10 @@ class UnaryInstructionWithTypeDependentOperandsBase
     return this->getAllOperands()[0];
   }
 
+  const Operand &getOperandRef() const {
+    return this->getAllOperands()[0];
+  }
+
   ArrayRef<Operand> getTypeDependentOperands() const {
     return this->getAllOperands().slice(1);
   }
@@ -6203,6 +6212,8 @@ class EnumInst
 
   Operand &getOperandRef() { return OptionalOperand->asArray()[0]; }
 
+  const Operand &getOperandRef() const { return OptionalOperand->asArray()[0]; }
+
   ArrayRef<Operand> getAllOperands() const {
     return OptionalOperand ? OptionalOperand->asArray() : ArrayRef<Operand>{};
   }
@@ -8384,31 +8395,50 @@ class TermInst : public NonValueInstruction {
 
   TermKind getTermKind() const { return TermKind(getKind()); }
 
-  /// Returns true if this is a transformation terminator.
+  /// Returns true if this terminator may have a result, represented as a block
+  /// argument in any of its successor blocks.
   ///
-  /// The first operand is the transformed source.
-  bool isTransformationTerminator() const {
+  /// Phis (whose operands originate from BranchInst terminators) are not
+  /// terminator results.
+  ///
+  /// CondBr might produce block arguments for legacy reasons. This is gradually
+  /// being deprecated. For now, they are considered phis. In OSSA, these "phis"
+  /// must be trivial and critical edges cannot be present.
+  bool mayHaveTerminatorResult() const {
     switch (getTermKind()) {
     case TermKind::UnwindInst:
     case TermKind::UnreachableInst:
     case TermKind::ReturnInst:
     case TermKind::ThrowInst:
     case TermKind::YieldInst:
-    case TermKind::TryApplyInst:
-    case TermKind::BranchInst:
     case TermKind::CondBranchInst:
-    case TermKind::SwitchValueInst:
+    case TermKind::BranchInst:
     case TermKind::SwitchEnumAddrInst:
-    case TermKind::DynamicMethodBranchInst:
     case TermKind::CheckedCastAddrBranchInst:
-    case TermKind::AwaitAsyncContinuationInst:
       return false;
-    case TermKind::SwitchEnumInst:
     case TermKind::CheckedCastBranchInst:
+    case TermKind::SwitchEnumInst:
+    case TermKind::SwitchValueInst:
+    case TermKind::TryApplyInst:
+    case TermKind::AwaitAsyncContinuationInst:
+    case TermKind::DynamicMethodBranchInst:
       return true;
     }
     llvm_unreachable("Covered switch isn't covered.");
   }
+
+  /// Returns an Operand reference if this terminator forwards ownership of a
+  /// single operand to a single result for at least one successor
+  /// block. Otherwise returns nullptr.
+  ///
+  /// By convention, terminators can forward ownership of at most one operand to
+  /// at most one result. The operand value might not be directly forwarded. For
+  /// example, a switch forwards ownership of the enum type into ownership of
+  /// the payload.
+  ///
+  /// Postcondition: each successor has zero or one block arguments which
+  /// represents the forwaded result.
+  const Operand *forwardedOperand() const;
 };
 
 // Forwards the first operand to a result in each successor block.
@@ -8442,6 +8472,10 @@ class OwnershipForwardingTermInst : public TermInst,
 
   SILValue getOperand() const { return getAllOperands()[0].get(); }
 
+  Operand &getOperandRef() { return getAllOperands()[0]; }
+
+  const Operand &getOperandRef() const { return getAllOperands()[0]; }
+
   /// Create a result for this terminator on the given successor block.
   SILPhiArgument *createResult(SILBasicBlock *succ, SILType resultTy);
 };

include/swift/SIL/SILValue.h

@@ -494,6 +494,13 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
   }
   SILInstruction *getDefiningInstruction();
 
+  /// Return the instruction that defines this value, terminator instruction
+  /// that produces this result, or null if it is not defined by an instruction.
+  const SILInstruction *getDefiningInstructionOrTerminator() const {
+    return const_cast<ValueBase*>(this)->getDefiningInstructionOrTerminator();
+  }
+  SILInstruction *getDefiningInstructionOrTerminator();
+
   /// Return the SIL instruction that can be used to describe the first time
   /// this value is available.
   ///

lib/SIL/IR/SILArgument.cpp

@@ -77,16 +77,17 @@ SILParameterInfo SILFunctionArgument::getKnownParameterInfo() const {
 //                              SILBlockArgument
 //===----------------------------------------------------------------------===//
 
-// FIXME: SILPhiArgument should only refer to branch arguments. They usually
-// need to be distinguished from projections and casts. Actual phi block
-// arguments are substitutable with their incoming values. It is also needlessly
-// expensive to call this helper instead of simply specifying phis with an
-// opcode. It results in repeated CFG traversals and repeated, unnecessary
-// switching over terminator opcodes.
+// FIXME: SILPhiArgument should only refer to phis (values merged from
+// BranchInst operands). Phis are directly substitutable with their incoming
+// values modulo control flow. They usually need to be distinguished from
+// projections and casts. It is needlessly expensive to call this helper instead
+// of simply specifying phis with an opcode. It results in repeated CFG
+// traversals and repeated, unnecessary switching over terminator opcodes.
 bool SILPhiArgument::isPhi() const {
-  // No predecessors indicates an unreachable block.
+  // No predecessors indicates an unreachable block. Treat this like a
+  // degenerate phi so we don't consider it a terminator result.
   if (getParent()->pred_empty())
-    return false;
+    return true;
 
   // Multiple predecessors require phis.
   auto *predBlock = getParent()->getSinglePredecessorBlock();
@@ -215,7 +216,6 @@ bool SILPhiArgument::getIncomingPhiValues(
     return false;
 
   const auto *parentBlock = getParent();
-  assert(!parentBlock->pred_empty());
 
   unsigned argIndex = getIndex();
   for (auto *predBlock : getParent()->getPredecessorBlocks()) {
@@ -323,14 +323,6 @@ bool SILPhiArgument::getSingleTerminatorOperands(
   return true;
 }
 
-SILValue SILPhiArgument::getSingleTerminatorOperand() const {
-  const auto *parentBlock = getParent();
-  const auto *predBlock = parentBlock->getSinglePredecessorBlock();
-  if (!predBlock)
-    return SILValue();
-  return getSingleTerminatorOperandForPred(parentBlock, predBlock, getIndex());
-}
-
 TermInst *SILPhiArgument::getSingleTerminator() const {
   auto *parentBlock = getParent();
   auto *predBlock = parentBlock->getSinglePredecessorBlock();
@@ -349,6 +341,11 @@ TermInst *SILPhiArgument::getTerminatorForResult() const {
   return nullptr;
 }
 
+const Operand *SILArgument::forwardedTerminatorResultOperand() const {
+  assert(isTerminatorResult() && "API is invalid for phis");
+  return getSingleTerminator()->forwardedOperand();
+}
+
 SILPhiArgument *BranchInst::getArgForOperand(const Operand *oper) {
   assert(oper->getUser() == this);
   return cast<SILPhiArgument>(

lib/SIL/IR/SILInstructions.cpp

@@ -1617,6 +1617,40 @@ TermInst::getSuccessorBlockArgumentLists() const {
   return SuccessorBlockArgumentListTy(getSuccessors(), op);
 }
 
+const Operand *TermInst::forwardedOperand() const {
+  switch (getTermKind()) {
+  case TermKind::UnwindInst:
+  case TermKind::UnreachableInst:
+  case TermKind::ReturnInst:
+  case TermKind::ThrowInst:
+  case TermKind::YieldInst:
+  case TermKind::TryApplyInst:
+  case TermKind::CondBranchInst:
+  case TermKind::BranchInst:
+  case TermKind::SwitchEnumAddrInst:
+  case TermKind::SwitchValueInst:
+  case TermKind::DynamicMethodBranchInst:
+  case TermKind::CheckedCastAddrBranchInst:
+  case TermKind::AwaitAsyncContinuationInst:
+    return nullptr;
+  case TermKind::SwitchEnumInst: {
+    auto *switchEnum = cast<SwitchEnumInst>(this);
+    if (!switchEnum->preservesOwnership())
+      return nullptr;
+
+    return &switchEnum->getOperandRef();
+  }
+  case TermKind::CheckedCastBranchInst: {
+    auto *checkedCast = cast<CheckedCastBranchInst>(this);
+    if (!checkedCast->preservesOwnership())
+      return nullptr;
+
+    return &checkedCast->getOperandRef();
+  }
+  }
+  llvm_unreachable("Covered switch isn't covered.");
+}
+
 YieldInst *YieldInst::create(SILDebugLocation loc,
                              ArrayRef<SILValue> yieldedValues,
                              SILBasicBlock *normalBB, SILBasicBlock *unwindBB,

lib/SIL/IR/SILValue.cpp

@@ -80,6 +80,16 @@ SILInstruction *ValueBase::getDefiningInstruction() {
   return nullptr;
 }
 
+SILInstruction *ValueBase::getDefiningInstructionOrTerminator() {
+  if (auto *inst = dyn_cast<SingleValueInstruction>(this))
+    return inst;
+  if (auto *result = dyn_cast<MultipleValueInstructionResult>(this))
+    return result->getParent();
+  if (auto *result = SILArgument::isTerminatorResult(this))
+    return result->getSingleTerminator();
+  return nullptr;
+}
+
 SILInstruction *ValueBase::getDefiningInsertionPoint() {
   if (auto *inst = getDefiningInstruction())
     return inst;