[ownership] Add a subclass for all OwnershipForwarding instructions and fix the classof to the various ownership abstract classes so isa/dyn_cast work.

I think what was happening here was that we were using one of the superclass
classofs and were getting lucky since in the place I was using this I was
guaranteed to have single value instructions and that is what I wrote as my
first case X ).

I also added more robust checks tieing the older isGuaranteed...* APIs to the
ForwardingOperand API. I also eliminated the notion of Branch being an owned
forwarding instruction. We only used this in one place in the compiler (when
finding owned value introducers), yet we treat a phi as an introducer, so we
would never hit a branch in our search since we would stop at the phi argument.
The bigger picture here is that this means that all "forwarding instructions"
either forward ownership for everything or for everything but owned/unowned.

And for those listening in, I did find one instruction that was from an
ownership forwarding subclass but was not marked as forwarding:
DifferentiableFunctionInst. With this change, we can no longer by mistake have
such errors enter the code base.

Author: gottesmm

include/swift/SIL/OwnershipUtils.h

@@ -35,12 +35,13 @@ bool isValueAddressOrTrivial(SILValue v);
 /// These operations forward both owned and guaranteed ownership.
 bool isOwnershipForwardingValueKind(SILNodeKind kind);
 
-/// Is this an instruction that can forward both owned and guaranteed ownership
+/// Is this an operand that can forward both owned and guaranteed ownership
 /// kinds.
-bool isOwnershipForwardingInst(SILInstruction *i);
+bool isOwnershipForwardingUse(Operand *op);
 
-/// Is this an instruction that can forward guaranteed ownership.
-bool isGuaranteedForwardingInst(SILInstruction *i);
+/// Is this an operand that forwards guaranteed ownership from its value to a
+/// result of the using instruction.
+bool isGuaranteedForwardingUse(Operand *op);
 
 /// These operations forward guaranteed ownership, but don't necessarily forward
 /// owned values.
@@ -54,12 +55,12 @@ bool isGuaranteedForwardingValue(SILValue value);
 /// forward guaranteed ownership.
 bool isOwnedForwardingValueKind(SILNodeKind kind);
 
-/// Does this SILInstruction 'forward' owned ownership, but may not be able to
-/// forward guaranteed ownership.
-bool isOwnedForwardingInstruction(SILInstruction *inst);
-
-/// Does this value 'forward' owned ownership, but may not be able to forward
+/// Does this operand 'forward' owned ownership, but may not be able to forward
 /// guaranteed ownership.
+bool isOwnedForwardingUse(Operand *use);
+
+/// Is this value the result of an instruction that 'forward's owned ownership,
+/// but may not be able to forward guaranteed ownership.
 ///
 /// This will be either a multiple value instruction resuilt, a single value
 /// instruction that forwards or an argument that forwards the ownership from a
@@ -78,6 +79,10 @@ class ForwardingOperand {
   void setOwnershipKind(ValueOwnershipKind newKind) const;
   void replaceOwnershipKind(ValueOwnershipKind oldKind,
                             ValueOwnershipKind newKind) const;
+
+  OwnershipForwardingInst *getUser() const {
+    return cast<OwnershipForwardingInst>(use->getUser());
+  }
 };
 
 /// Returns true if the instruction is a 'reborrow'.

include/swift/SIL/SILInstruction.h

@@ -856,38 +856,95 @@ inline SingleValueInstruction *SILNode::castToSingleValueInstruction() {
   }
 }
 
-#define DEFINE_ABSTRACT_SINGLE_VALUE_INST_BOILERPLATE(ID)       \
-  static bool classof(const SILNode *node) {                    \
-    return node->getKind() >= SILNodeKind::First_##ID &&        \
-           node->getKind() <= SILNodeKind::Last_##ID;           \
-  }                                                             \
-  static bool classof(const SingleValueInstruction *inst) {     \
-    return inst->getKind() >= SILInstructionKind::First_##ID && \
-           inst->getKind() <= SILInstructionKind::Last_##ID;    \
+#define DEFINE_ABSTRACT_SINGLE_VALUE_INST_BOILERPLATE(ID)                      \
+  static bool classof(const SILNode *node) {                                   \
+    return node->getKind() >= SILNodeKind::First_##ID &&                       \
+           node->getKind() <= SILNodeKind::Last_##ID;                          \
+  }                                                                            \
+  static bool classof(const SingleValueInstruction *inst) {                    \
+    return inst->getKind() >= SILInstructionKind::First_##ID &&                \
+           inst->getKind() <= SILInstructionKind::Last_##ID;                   \
+  }
+
+/// Abstract base class used for isa checks on instructions to determine if they
+/// forward ownership and to verify that the set of ownership instructions and
+/// the ownership utilities stay in sync via assertions.
+///
+/// NOTE: We assume that the constructor for the instruction subclass that
+/// initializes the kind field on our object is run before our constructor runs.
+class OwnershipForwardingInst {
+  ValueOwnershipKind ownershipKind;
+
+protected:
+  OwnershipForwardingInst(SILInstructionKind kind,
+                          ValueOwnershipKind ownershipKind)
+      : ownershipKind(ownershipKind) {
+    assert(classof(kind) && "Invalid subclass?!");
+  }
+
+public:
+  ValueOwnershipKind getOwnershipKind() const { return ownershipKind; }
+
+  void setOwnershipKind(ValueOwnershipKind newKind) { ownershipKind = newKind; }
+
+  static bool classof(const SILNode *node) {
+    if (auto *i = dyn_cast<SILInstruction>(node))
+      return classof(i);
+    return false;
   }
 
+  // Defined inline below due to forward declaration issues.
+  static bool classof(const SILInstruction *inst);
+
+  /// Define inline below due to forward declaration issues.
+  static bool classof(SILInstructionKind kind);
+};
+
 /// A single value inst that forwards a static ownership from one (or all) of
 /// its operands.
 ///
 /// The ownership kind is set on construction and afterwards must be changed
 /// explicitly using setOwnershipKind().
-class OwnershipForwardingSingleValueInst : public SingleValueInstruction {
-  ValueOwnershipKind ownershipKind;
-
+class OwnershipForwardingSingleValueInst : public SingleValueInstruction,
+                                           public OwnershipForwardingInst {
 protected:
   OwnershipForwardingSingleValueInst(SILInstructionKind kind,
                                      SILDebugLocation debugLoc, SILType ty,
                                      ValueOwnershipKind ownershipKind)
       : SingleValueInstruction(kind, debugLoc, ty),
-        ownershipKind(ownershipKind) {
-    assert(ownershipKind);
+        OwnershipForwardingInst(kind, ownershipKind) {
+    assert(classof(kind) && "classof missing new subclass?!");
   }
 
 public:
-  ValueOwnershipKind getOwnershipKind() const { return ownershipKind; }
-  void setOwnershipKind(ValueOwnershipKind newOwnershipKind) {
-    ownershipKind = newOwnershipKind;
-    assert(ownershipKind);
+  static bool classof(const SILNode *node) {
+    if (auto *i = dyn_cast<SILInstruction>(node))
+      return classof(i);
+    return false;
+  }
+
+  static bool classof(SILInstructionKind kind) {
+    switch (kind) {
+    case SILInstructionKind::MarkUninitializedInst:
+    case SILInstructionKind::StructInst:
+    case SILInstructionKind::ObjectInst:
+    case SILInstructionKind::TupleInst:
+    case SILInstructionKind::EnumInst:
+    case SILInstructionKind::UncheckedEnumDataInst:
+    case SILInstructionKind::SelectValueInst:
+    case SILInstructionKind::OpenExistentialRefInst:
+    case SILInstructionKind::InitExistentialRefInst:
+    case SILInstructionKind::MarkDependenceInst:
+    case SILInstructionKind::LinearFunctionInst:
+    case SILInstructionKind::DifferentiableFunctionInst:
+      return true;
+    default:
+      return false;
+    }
+  }
+
+  static bool classof(const SILInstruction *inst) {
+    return classof(inst->getKind());
   }
 };
 
@@ -4430,19 +4487,42 @@ class ConversionInst : public SingleValueInstruction {
 
 /// A conversion inst that produces a static OwnershipKind set upon the
 /// instruction's construction.
-class OwnershipForwardingConversionInst : public ConversionInst {
-  ValueOwnershipKind ownershipKind;
-
+class OwnershipForwardingConversionInst : public ConversionInst,
+                                          public OwnershipForwardingInst {
 protected:
   OwnershipForwardingConversionInst(SILInstructionKind kind,
                                     SILDebugLocation debugLoc, SILType ty,
                                     ValueOwnershipKind ownershipKind)
-      : ConversionInst(kind, debugLoc, ty), ownershipKind(ownershipKind) {}
+      : ConversionInst(kind, debugLoc, ty),
+        OwnershipForwardingInst(kind, ownershipKind) {
+    assert(classof(kind) && "classof missing subclass?!");
+  }
 
 public:
-  ValueOwnershipKind getOwnershipKind() const { return ownershipKind; }
-  void setOwnershipKind(ValueOwnershipKind newOwnershipKind) {
-    ownershipKind = newOwnershipKind;
+  static bool classof(const SILNode *node) {
+    if (auto *i = dyn_cast<SILInstruction>(node))
+      return classof(i);
+    return false;
+  }
+
+  static bool classof(const SILInstruction *inst) {
+    return classof(inst->getKind());
+  }
+
+  static bool classof(SILInstructionKind kind) {
+    switch (kind) {
+    case SILInstructionKind::ConvertFunctionInst:
+    case SILInstructionKind::UpcastInst:
+    case SILInstructionKind::UncheckedRefCastInst:
+    case SILInstructionKind::UncheckedValueCastInst:
+    case SILInstructionKind::RefToBridgeObjectInst:
+    case SILInstructionKind::BridgeObjectToRefInst:
+    case SILInstructionKind::ThinToThickFunctionInst:
+    case SILInstructionKind::UnconditionalCheckedCastInst:
+      return true;
+    default:
+      return false;
+    }
   }
 };
 
@@ -5610,22 +5690,35 @@ class SelectEnumInstBase
 };
 
 /// A select enum inst that produces a static OwnershipKind.
-class OwnershipForwardingSelectEnumInstBase : public SelectEnumInstBase {
-  ValueOwnershipKind ownershipKind;
-
+class OwnershipForwardingSelectEnumInstBase : public SelectEnumInstBase,
+                                              public OwnershipForwardingInst {
 protected:
   OwnershipForwardingSelectEnumInstBase(
       SILInstructionKind kind, SILDebugLocation debugLoc, SILType type,
       bool defaultValue, Optional<ArrayRef<ProfileCounter>> caseCounts,
       ProfileCounter defaultCount, ValueOwnershipKind ownershipKind)
       : SelectEnumInstBase(kind, debugLoc, type, defaultValue, caseCounts,
                            defaultCount),
-        ownershipKind(ownershipKind) {}
+        OwnershipForwardingInst(kind, ownershipKind) {
+    assert(classof(kind) && "classof missing subclass");
+  }
 
 public:
-  ValueOwnershipKind getOwnershipKind() const { return ownershipKind; }
-  void setOwnershipKind(ValueOwnershipKind newOwnershipKind) {
-    ownershipKind = newOwnershipKind;
+  static bool classof(const SILNode *node) {
+    if (auto *i = dyn_cast<SILInstruction>(node))
+      return classof(i);
+    return false;
+  }
+
+  static bool classof(const SILInstruction *i) { return classof(i->getKind()); }
+
+  static bool classof(SILInstructionKind kind) {
+    switch (kind) {
+    case SILInstructionKind::SelectEnumInst:
+      return true;
+    default:
+      return false;
+    }
   }
 };
 
@@ -7327,18 +7420,31 @@ class TermInst : public NonValueInstruction {
   }
 };
 
-class OwnershipForwardingTermInst : public TermInst {
-  ValueOwnershipKind ownershipKind;
-
+class OwnershipForwardingTermInst : public TermInst,
+                                    public OwnershipForwardingInst {
 protected:
   OwnershipForwardingTermInst(SILInstructionKind kind,
                               SILDebugLocation debugLoc,
                               ValueOwnershipKind ownershipKind)
-      : TermInst(kind, debugLoc), ownershipKind(ownershipKind) {}
+      : TermInst(kind, debugLoc), OwnershipForwardingInst(kind, ownershipKind) {
+    assert(classof(kind));
+  }
 
 public:
-  ValueOwnershipKind getOwnershipKind() const { return ownershipKind; }
-  void setOwnershipKind(ValueOwnershipKind newKind) { ownershipKind = newKind; }
+  static bool classof(const SILNode *node) {
+    if (auto *i = dyn_cast<SILInstruction>(node))
+      return classof(i);
+    return false;
+  }
+
+  static bool classof(const SILInstruction *inst) {
+    return classof(inst->getKind());
+  }
+
+  static bool classof(SILInstructionKind kind) {
+    return kind == SILInstructionKind::SwitchEnumInst ||
+           kind == SILInstructionKind::CheckedCastBranchInst;
+  }
 };
 
 /// UnreachableInst - Position in the code which would be undefined to reach.
@@ -8771,19 +8877,33 @@ SILFunction *ApplyInstBase<Impl, Base, false>::getCalleeFunction() const {
 }
 
 class OwnershipForwardingMultipleValueInstruction
-    : public MultipleValueInstruction {
-  ValueOwnershipKind ownershipKind;
-
+    : public MultipleValueInstruction,
+      public OwnershipForwardingInst {
 public:
   OwnershipForwardingMultipleValueInstruction(SILInstructionKind kind,
                                               SILDebugLocation loc,
                                               ValueOwnershipKind ownershipKind)
-      : MultipleValueInstruction(kind, loc), ownershipKind(ownershipKind) {}
+      : MultipleValueInstruction(kind, loc),
+        OwnershipForwardingInst(kind, ownershipKind) {
+    assert(classof(kind) && "Missing subclass from classof?!");
+  }
 
-  /// Returns the preferred ownership kind of this multiple value instruction.
-  ValueOwnershipKind getOwnershipKind() const { return ownershipKind; }
-  void setOwnershipKind(ValueOwnershipKind newOwnershipKind) {
-    ownershipKind = newOwnershipKind;
+  static bool classof(const SILNode *n) {
+    if (auto *i = dyn_cast<SILInstruction>(n))
+      return classof(i);
+    return false;
+  }
+
+  static bool classof(const SILInstruction *i) { return classof(i->getKind()); }
+
+  static bool classof(SILInstructionKind kind) {
+    switch (kind) {
+    case SILInstructionKind::DestructureTupleInst:
+    case SILInstructionKind::DestructureStructInst:
+      return true;
+    default:
+      return false;
+    }
   }
 };
 
@@ -8953,6 +9073,22 @@ inline bool Operand::isTypeDependent() const {
   return getUser()->isTypeDependentOperand(*this);
 }
 
+inline bool OwnershipForwardingInst::classof(const SILInstruction *inst) {
+  return OwnershipForwardingSingleValueInst::classof(inst) ||
+         OwnershipForwardingTermInst::classof(inst) ||
+         OwnershipForwardingConversionInst::classof(inst) ||
+         OwnershipForwardingSelectEnumInstBase::classof(inst) ||
+         OwnershipForwardingMultipleValueInstruction::classof(inst);
+}
+
+inline bool OwnershipForwardingInst::classof(SILInstructionKind kind) {
+  return OwnershipForwardingSingleValueInst::classof(kind) ||
+         OwnershipForwardingTermInst::classof(kind) ||
+         OwnershipForwardingConversionInst::classof(kind) ||
+         OwnershipForwardingSelectEnumInstBase::classof(kind) ||
+         OwnershipForwardingMultipleValueInstruction::classof(kind);
+}
+
 } // end swift namespace
 
 //===----------------------------------------------------------------------===//

lib/SIL/IR/OperandOwnership.cpp

@@ -319,7 +319,7 @@ FORWARD_ANY_OWNERSHIP_INST(DestructureTuple)
   OwnershipConstraint OwnershipConstraintClassifier::visit##INST##Inst(        \
       INST##Inst *i) {                                                         \
     assert(i->getNumOperands() && "Expected to have non-zero operands");       \
-    assert(isGuaranteedForwardingInst(i) &&                                    \
+    assert(isGuaranteedForwardingValueKind(SILNodeKind(i->getKind())) &&       \
            "Expected an ownership forwarding inst");                           \
     return {OwnershipKind::OWNERSHIP,                                          \
             UseLifetimeConstraint::USE_LIFETIME_CONSTRAINT};                   \