Merge pull request #34383 from eeckstein/fix_bridging_casts

Author: swift-ci

include/swift/SIL/DynamicCasts.h

@@ -433,6 +433,9 @@ struct SILDynamicCastInst {
     return TargetIsBridgeable != SourceIsBridgeable;
   }
 
+  /// Returns true if this dynamic cast can release its source operand.
+  bool isRCIdentityPreserving() const;
+
   /// If getSourceType() is a Swift type that can bridge to an ObjC type, return
   /// the ObjC type it bridges to. If the source type is an objc type, an empty
   /// CanType() is returned.

lib/SIL/IR/SILInstruction.cpp

@@ -22,6 +22,7 @@
 #include "swift/SIL/SILCloner.h"
 #include "swift/SIL/SILDebugScope.h"
 #include "swift/SIL/SILVisitor.h"
+#include "swift/SIL/DynamicCasts.h"
 #include "swift/Basic/AssertImplements.h"
 #include "swift/ClangImporter/ClangModule.h"
 #include "swift/SIL/SILModule.h"
@@ -1059,6 +1060,13 @@ bool SILInstruction::mayHaveSideEffects() const {
 }
 
 bool SILInstruction::mayRelease() const {
+  // Overrule a "DoesNotRelease" of dynamic casts. If a dynamic cast is not
+  // RC identity preserving it can release it's source (in some cases - we are
+  // conservative here).
+  auto dynCast = SILDynamicCastInst::getAs(const_cast<SILInstruction *>(this));
+  if (dynCast && !dynCast.isRCIdentityPreserving())
+    return true;
+
   if (getReleasingBehavior() ==
       SILInstruction::ReleasingBehavior::DoesNotRelease)
     return false;

lib/SIL/Utils/DynamicCasts.cpp

@@ -208,6 +208,50 @@ static CanType getHashableExistentialType(ModuleDecl *M) {
   return hashable->getDeclaredInterfaceType()->getCanonicalType();
 }
 
+static bool canBeExistential(CanType ty) {
+  // If ty is an archetype, conservatively assume it's an existential.
+  return ty.isAnyExistentialType() || ty->is<ArchetypeType>();
+}
+
+static bool canBeClass(CanType ty) {
+  // If ty is an archetype, conservatively assume it's an existential.
+  return ty.getClassOrBoundGenericClass() || ty->is<ArchetypeType>();
+}
+
+bool SILDynamicCastInst::isRCIdentityPreserving() const {
+  // Casts which cast from a trivial type, like a metatype, to something which
+  // is retainable (or vice versa), like an AnyObject, are not RC identity
+  // preserving.
+  // On some platforms such casts dynamically allocate a ref-counted box for the
+  // metatype. Naturally that is the place where a new rc-identity begins.
+  // Therefore such a cast is introducing a new rc identical object.
+  //
+  // If RCIdentityAnalysis would look through such a cast, ARC optimizations
+  // would get confused and might eliminate a retain of such an object
+  // completely.
+  SILFunction &f = *getFunction();
+  if (getSourceLoweredType().isTrivial(f) != getTargetLoweredType().isTrivial(f))
+    return false;
+
+  CanType source = getSourceFormalType();
+  CanType target = getTargetFormalType();
+
+  // An existential may be holding a reference to a bridgeable struct.
+  // In this case, ARC on the existential affects the refcount of the container
+  // holding the struct, not the class to which the struct is bridged.
+  // Therefore, don't assume RC identity when casting between existentials and
+  // classes (and also between two existentials).
+  if (canBeExistential(source) &&
+      (canBeClass(target) || canBeExistential(target)))
+    return false;
+
+  // And vice versa.
+  if (canBeClass(source) && canBeExistential(target))
+    return false;
+
+  return true;
+}
+
 /// Check if a given type conforms to _BridgedToObjectiveC protocol.
 bool swift::isObjectiveCBridgeable(ModuleDecl *M, CanType Ty) {
   // Retrieve the _BridgedToObjectiveC protocol.

lib/SIL/Utils/MemAccessUtils.cpp

@@ -15,6 +15,7 @@
 #include "swift/SIL/MemAccessUtils.h"
 #include "swift/SIL/SILModule.h"
 #include "swift/SIL/SILUndef.h"
+#include "swift/SIL/DynamicCasts.h"
 #include "llvm/Support/Debug.h"
 
 using namespace swift;
@@ -388,9 +389,7 @@ bool swift::isRCIdentityPreservingCast(SingleValueInstruction *svi) {
     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());
+    return SILDynamicCastInst(svi).isRCIdentityPreserving();
   }
 }
 

lib/SILOptimizer/Analysis/RCIdentityAnalysis.cpp

@@ -13,6 +13,7 @@
 #include "swift/SILOptimizer/Analysis/RCIdentityAnalysis.h"
 #include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
 #include "swift/SIL/SILInstruction.h"
+#include "swift/SIL/DynamicCasts.h"
 #include "llvm/Support/CommandLine.h"
 
 using namespace swift;
@@ -34,21 +35,6 @@ static bool isNoPayloadEnum(SILValue V) {
   return !EI->hasOperand();
 }
 
-/// Returns true if V is a valid operand of a cast which preserves RC identity.
-///
-/// V is a valid operand if it has a non-trivial type.
-/// RCIdentityAnalysis must not look through casts which cast from a trivial
-/// type, like a metatype, to something which is retainable, like an AnyObject.
-/// On some platforms such casts dynamically allocate a ref-counted box for the
-/// metatype. Naturally that is the place where a new rc-identity begins.
-/// Therefore such a cast is introducing a new rc identical object.
-///
-/// If we would look through such a cast, ARC optimizations would get confused
-/// and might eliminate a retain of such an object completely.
-static bool isValidRCIdentityCastOperand(SILValue V, SILFunction *F) {
-  return !V->getType().isTrivial(*F);
-}
-
 /// RC identity is more than a guarantee that references refer to the same
 /// object. It also means that reference counting operations on those references
 /// have the same semantics. If the types on either side of a cast do not have
@@ -61,16 +47,16 @@ static SILValue getRCIdentityPreservingCastOperand(SILValue V) {
   switch (V->getKind()) {
   case ValueKind::UpcastInst:
   case ValueKind::UncheckedRefCastInst:
-  case ValueKind::UnconditionalCheckedCastInst:
   case ValueKind::InitExistentialRefInst:
   case ValueKind::OpenExistentialRefInst:
   case ValueKind::RefToBridgeObjectInst:
   case ValueKind::BridgeObjectToRefInst:
-  case ValueKind::ConvertFunctionInst: {
-    auto *inst = cast<SingleValueInstruction>(V);
-    SILValue castOp = inst->getOperand(0);
-    if (isValidRCIdentityCastOperand(castOp, inst->getFunction()))
-      return castOp;
+  case ValueKind::ConvertFunctionInst:
+    return cast<SingleValueInstruction>(V)->getOperand(0);
+  case ValueKind::UnconditionalCheckedCastInst: {
+    auto *castInst = cast<UnconditionalCheckedCastInst>(V);
+    if (SILDynamicCastInst(castInst).isRCIdentityPreserving())
+      return castInst->getOperand();
     break;
   }
   default:
@@ -141,8 +127,10 @@ static SILValue stripRCIdentityPreservingInsts(SILValue V) {
     if (SILValue Result = A->getSingleTerminatorOperand()) {
       // In case the terminator is a conditional cast, Result is the source of
       // the cast.
-      if (isValidRCIdentityCastOperand(Result, A->getFunction()))
-        return Result;
+      auto dynCast = SILDynamicCastInst::getAs(A->getSingleTerminator());
+      if (dynCast && !dynCast.isRCIdentityPreserving())
+        return SILValue();
+      return Result;
     }
   }
 
@@ -347,12 +335,15 @@ SILValue RCIdentityFunctionInfo::stripRCIdentityPreservingArgs(SILValue V,
   }
 
   unsigned IVListSize = IncomingValues.size();
-  if (IVListSize == 1 &&
-      !isValidRCIdentityCastOperand(IncomingValues[0].second, A->getFunction()))
-    return SILValue();
-
-  assert(IVListSize != 1 && "Should have been handled in "
-         "stripRCIdentityPreservingInsts");
+  if (IVListSize == 1) {
+#ifndef NDEBUG
+      auto dynCast = SILDynamicCastInst::getAs(A->getSingleTerminator());
+      assert((dynCast && !dynCast.isRCIdentityPreserving())
+             && "Should have been handled in stripRCIdentityPreservingInsts");
+#endif
+      return SILValue();
+    
+  }
 
   // Ok, we have multiple predecessors. See if all of them are the same
   // value. If so, just return that value.

test/SILOptimizer/retain_release_code_motion.sil

@@ -31,6 +31,8 @@ struct A {
 
 class fuzz { }
 
+protocol P : class { }
+
 enum Boo {
   case one
   case two
@@ -817,3 +819,69 @@ bb0(%0 : $_ContiguousArrayBuffer<AnyObject>, %1 : $Builtin.Word, %2 : $Builtin.W
   release_value %0 : $_ContiguousArrayBuffer<AnyObject>
   return %newptr : $Builtin.RawPointer
 }
+
+// CHECK-LABEL: sil @dontMoveOverExistentialToClassCast : $@convention(thin) (@guaranteed AnyObject) -> Optional<fuzz>
+// CHECK:    strong_retain %0
+// CHECK:    checked_cast_br %0
+// CHECK:  } // end sil function 'dontMoveOverExistentialToClassCast'
+sil @dontMoveOverExistentialToClassCast : $@convention(thin) (@guaranteed AnyObject) -> Optional<fuzz> {
+bb0(%0 : $AnyObject):
+  strong_retain %0 : $AnyObject
+  checked_cast_br %0 : $AnyObject to fuzz, bb1, bb2
+
+bb1(%18 : $fuzz):
+  %19 = enum $Optional<fuzz>, #Optional.some!enumelt, %18 : $fuzz
+  br bb3(%19 : $Optional<fuzz>)
+
+bb2:
+  strong_release %0 : $AnyObject
+  %22 = enum $Optional<fuzz>, #Optional.none!enumelt
+  br bb3(%22 : $Optional<fuzz>)
+
+bb3(%24 : $Optional<fuzz>):
+  return %24 : $Optional<fuzz>
+}
+
+// CHECK-LABEL: sil @dontMoveOverClassToExistentialCast : $@convention(thin) (@guaranteed fuzz) -> Optional<P>
+// CHECK:    strong_retain %0
+// CHECK:    checked_cast_br %0
+// CHECK:  } // end sil function 'dontMoveOverClassToExistentialCast'
+sil @dontMoveOverClassToExistentialCast : $@convention(thin) (@guaranteed fuzz) -> Optional<P> {
+bb0(%0 : $fuzz):
+  strong_retain %0 : $fuzz
+  checked_cast_br %0 : $fuzz to P, bb1, bb2
+
+bb1(%18 : $P):
+  %19 = enum $Optional<P>, #Optional.some!enumelt, %18 : $P
+  br bb3(%19 : $Optional<P>)
+
+bb2:
+  strong_release %0 : $fuzz
+  %22 = enum $Optional<P>, #Optional.none!enumelt
+  br bb3(%22 : $Optional<P>)
+
+bb3(%24 : $Optional<P>):
+  return %24 : $Optional<P>
+}
+
+// CHECK-LABEL: sil @dontMoveOverExistentialToExistentialCast : $@convention(thin) (@guaranteed AnyObject) -> Optional<P>
+// CHECK:    strong_retain %0
+// CHECK:    checked_cast_br %0
+// CHECK:  } // end sil function 'dontMoveOverExistentialToExistentialCast'
+sil @dontMoveOverExistentialToExistentialCast : $@convention(thin) (@guaranteed AnyObject) -> Optional<P> {
+bb0(%0 : $AnyObject):
+  strong_retain %0 : $AnyObject
+  checked_cast_br %0 : $AnyObject to P, bb1, bb2
+
+bb1(%18 : $P):
+  %19 = enum $Optional<P>, #Optional.some!enumelt, %18 : $P
+  br bb3(%19 : $Optional<P>)
+
+bb2:
+  strong_release %0 : $AnyObject
+  %22 = enum $Optional<P>, #Optional.none!enumelt
+  br bb3(%22 : $Optional<P>)
+
+bb3(%24 : $Optional<P>):
+  return %24 : $Optional<P>
+}