Fix isRCIdentityPreservingCast to handle trivial-to-reference casts

And add assertions.

Author: atrick

include/swift/SIL/MemAccessUtils.h

@@ -1201,8 +1201,8 @@ SILBasicBlock::iterator removeBeginAccess(BeginAccessInst *beginAccess);
 
 namespace swift {
 
-/// Return true if \p value is a cast that preserves the identity of the
-/// reference at operand zero.
+/// Return true if \p value is a cast that preserves the identity and
+/// reference-counting equivalence of the reference at operand zero.
 bool isRCIdentityPreservingCast(SingleValueInstruction *svi);
 
 /// If \p svi is an access projection, return an address-type operand for the

lib/SIL/IR/SILType.cpp

@@ -129,6 +129,9 @@ bool SILType::isPointerSizeAndAligned() {
 //
 // TODO: handle casting to a loadable existential by generating
 // init_existential_ref. Until then, only promote to a heap object dest.
+//
+// This cannot allow trivial-to-reference casts, as required by
+// isRCIdentityPreservingCast.
 bool SILType::canRefCast(SILType operTy, SILType resultTy, SILModule &M) {
   auto fromTy = operTy.unwrapOptionalType();
   auto toTy = resultTy.unwrapOptionalType();

lib/SIL/Utils/InstructionUtils.cpp

@@ -12,6 +12,7 @@
 
 #define DEBUG_TYPE "sil-inst-utils"
 #include "swift/SIL/InstructionUtils.h"
+#include "swift/SIL/MemAccessUtils.h"
 #include "swift/AST/SubstitutionMap.h"
 #include "swift/Basic/NullablePtr.h"
 #include "swift/Basic/STLExtras.h"
@@ -64,20 +65,6 @@ SILValue swift::getUnderlyingObjectStopAtMarkDependence(SILValue v) {
   }
 }
 
-static bool isRCIdentityPreservingCast(ValueKind Kind) {
-  switch (Kind) {
-  case ValueKind::UpcastInst:
-  case ValueKind::UncheckedRefCastInst:
-  case ValueKind::UnconditionalCheckedCastInst:
-  case ValueKind::UnconditionalCheckedCastValueInst:
-  case ValueKind::RefToBridgeObjectInst:
-  case ValueKind::BridgeObjectToRefInst:
-    return true;
-  default:
-    return false;
-  }
-}
-
 /// Return the underlying SILValue after stripping off identity SILArguments if
 /// we belong to a BB with one predecessor.
 SILValue swift::stripSinglePredecessorArgs(SILValue V) {
@@ -122,42 +109,39 @@ SILValue swift::stripSinglePredecessorArgs(SILValue V) {
   }
 }
 
-SILValue swift::stripCastsWithoutMarkDependence(SILValue V) {
+SILValue swift::stripCastsWithoutMarkDependence(SILValue v) {
   while (true) {
-    V = stripSinglePredecessorArgs(V);
-
-    auto K = V->getKind();
-    if (isRCIdentityPreservingCast(K)
-        || K == ValueKind::UncheckedTrivialBitCastInst
-        || K == ValueKind::BeginAccessInst
-        || K == ValueKind::EndCOWMutationInst) {
-      V = cast<SingleValueInstruction>(V)->getOperand(0);
-      continue;
+    v = stripSinglePredecessorArgs(v);
+    if (auto *svi = dyn_cast<SingleValueInstruction>(v)) {
+      if (isRCIdentityPreservingCast(svi)
+          || isa<UncheckedTrivialBitCastInst>(v)
+          || isa<BeginAccessInst>(v)
+          || isa<EndCOWMutationInst>(v)) {
+        v = svi->getOperand(0);
+        continue;
+      }
     }
-
-    return V;
+    return v;
   }
 }
 
 SILValue swift::stripCasts(SILValue v) {
   while (true) {
     v = stripSinglePredecessorArgs(v);
-    
-    auto k = v->getKind();
-    if (isRCIdentityPreservingCast(k)
-        || k == ValueKind::UncheckedTrivialBitCastInst
-        || k == ValueKind::MarkDependenceInst
-        || k == ValueKind::BeginAccessInst) {
-      v = cast<SingleValueInstruction>(v)->getOperand(0);
-      continue;
+    if (auto *svi = dyn_cast<SingleValueInstruction>(v)) {
+      if (isRCIdentityPreservingCast(svi)
+          || isa<UncheckedTrivialBitCastInst>(v)
+          || isa<MarkDependenceInst>(v)
+          || isa<BeginAccessInst>(v)) {
+        v = cast<SingleValueInstruction>(v)->getOperand(0);
+        continue;
+      }
     }
-
     SILValue v2 = stripOwnershipInsts(v);
     if (v2 != v) {
       v = v2;
       continue;
     }
-
     return v;
   }
 }

lib/SIL/Utils/MemAccessUtils.cpp

@@ -365,6 +365,14 @@ bool swift::isLetAddress(SILValue address) {
 //                          MARK: FindReferenceRoot
 //===----------------------------------------------------------------------===//
 
+// On some platforms, casting from a metatype to a reference type dynamically
+// allocates a ref-counted box for the metatype. Naturally that is the place
+// where RC-identity begins. Considering the source of such a casts to be
+// RC-identical would confuse ARC optimization, which might eliminate a retain
+// of such an object completely.
+//
+// The SILVerifier checks that none of these operations cast a nontrivial value
+// to a reference except unconditional_checked_cast[_value].
 bool swift::isRCIdentityPreservingCast(SingleValueInstruction *svi) {
   switch (svi->getKind()) {
   default:
@@ -375,11 +383,14 @@ bool swift::isRCIdentityPreservingCast(SingleValueInstruction *svi) {
   // Ignore class type casts
   case SILInstructionKind::UpcastInst:
   case SILInstructionKind::UncheckedRefCastInst:
-  case SILInstructionKind::UnconditionalCheckedCastInst:
-  case SILInstructionKind::UnconditionalCheckedCastValueInst:
   case SILInstructionKind::RefToBridgeObjectInst:
   case SILInstructionKind::BridgeObjectToRefInst:
     return true;
+  case SILInstructionKind::UnconditionalCheckedCastInst:
+  case SILInstructionKind::UnconditionalCheckedCastValueInst:
+    // If the source is nontrivial, then this checked cast may actually create a
+    // new object, so its source is not ref-count equivalent.
+    return !svi->getOperand(0)->getType().isTrivial(*svi->getFunction());
   }
 }
 

lib/SIL/Verifier/SILVerifier.cpp

@@ -3591,6 +3591,15 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     verifyOpenedArchetype(CI, CI->getType().getASTType());
   }
 
+  // Make sure that opcodes handled by isRCIdentityPreservingCast cannot cast
+  // from a trivial to a reference type. Such a cast may dynamically
+  // instantiate a new reference-counted object.
+  void checkNoTrivialToReferenceCast(SingleValueInstruction *svi) {
+    require(!svi->getOperand(0)->getType().isTrivial(*svi->getFunction())
+            || svi->getType().isTrivial(*svi->getFunction()),
+            "Unexpected trivial-to-reference conversion: ");
+  }
+
   /// Verify if a given type is or contains an opened archetype or dynamic self.
   /// If this is the case, verify that the provided instruction has a type
   /// dependent operand for it.
@@ -3753,6 +3762,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
   void checkUpcastInst(UpcastInst *UI) {
     require(UI->getType() != UI->getOperand()->getType(),
             "can't upcast to same type");
+    checkNoTrivialToReferenceCast(UI);
     if (UI->getType().is<MetatypeType>()) {
       CanType instTy(UI->getType().castTo<MetatypeType>()->getInstanceType());
       require(UI->getOperand()->getType().is<MetatypeType>(),