CheckedBranchInst now uses doesCastPreserveOwnership.

Use the new API that determines whether a cast preserves
ownership. Remove an old hack.

Author: atrick

include/swift/SIL/SILInstruction.h

@@ -1129,24 +1129,14 @@ class OwnershipForwardingMixin {
   }
 
 public:
-  /// If an instruction is directly forwarding, then any operand op whose
-  /// ownership it forwards into a result r must have the property that op and r
-  /// are "rc identical". This means that they are representing the same set of
-  /// underlying lifetimes (plural b/c of aggregates).
+  /// A forwarding instruction preserved ownership if it has a
+  /// dynamically non-trivial result in which all references are forwarded from
+  /// the operand.
   ///
-  /// An instruction that is not directly forwarding, can not have guaranteed
-  /// ownership since without direct forwarding, there isn't necessarily any
-  /// connection in between the operand's lifetime and the value's lifetime.
-  ///
-  /// An example of this is checked_cast_br where when performing the following:
-  ///
-  ///   __SwiftValue(AnyHashable(Klass())) to OtherKlass()
-  ///
-  /// we will look through the __SwiftValue(AnyHashable(X)) any just cast Klass
-  /// to OtherKlass. This means that the result argument would no longer be
-  /// rc-identical to the operand and default case and thus we can not propagate
-  /// forward any form of guaranteed ownership.
-  bool isDirectlyForwarding() const { return directlyForwards; }
+  /// A cast can only forward guaranteed values if it preserves ownership. Such
+  /// casts cannot release any references within their operand's value and
+  /// cannot retain any references owned by their result.
+  bool preservesOwnership() const { return preservesOwnershipFlag; }
 
   /// Forwarding ownership is determined by the forwarding instruction's
   /// constant ownership attribute. If forwarding ownership is owned, then the
@@ -9064,16 +9054,12 @@ class CheckedCastBranchInst final
                         SILBasicBlock *SuccessBB, SILBasicBlock *FailureBB,
                         ProfileCounter Target1Count,
                         ProfileCounter Target2Count,
-                        ValueOwnershipKind forwardingOwnershipKind)
+                        ValueOwnershipKind forwardingOwnershipKind,
+                        bool preservesOwnership)
       : UnaryInstructionWithTypeDependentOperandsBase(
             DebugLoc, Operand, TypeDependentOperands, SuccessBB, FailureBB,
             Target1Count, Target2Count, forwardingOwnershipKind,
-            // We are always directly forwarding unless we are casting an
-            // AnyObject. This is b/c an AnyObject could contain a boxed
-            // AnyObject(Class()) that we unwrap as part of the cast. In such a
-            // case, we would return a different value and potentially end the
-            // lifetime of the operand value.
-            !Operand->getType().isAnyObject()),
+            preservesOwnership),
         DestLoweredTy(DestLoweredTy), DestFormalTy(DestFormalTy),
         IsExact(IsExact) {}
 

lib/SIL/IR/SILInstructions.cpp

@@ -19,6 +19,7 @@
 #include "swift/Basic/AssertImplements.h"
 #include "swift/Basic/Unicode.h"
 #include "swift/Basic/type_traits.h"
+#include "swift/SIL/DynamicCasts.h"
 #include "swift/SIL/FormalLinkage.h"
 #include "swift/SIL/Projection.h"
 #include "swift/SIL/SILBuilder.h"
@@ -2283,16 +2284,18 @@ CheckedCastBranchInst *CheckedCastBranchInst::create(
     SILBasicBlock *FailureBB, SILFunction &F,
     ProfileCounter Target1Count, ProfileCounter Target2Count,
     ValueOwnershipKind forwardingOwnershipKind) {
-  SILModule &Mod = F.getModule();
+  SILModule &module = F.getModule();
+  bool preservesOwnership = doesCastPreserveOwnershipForTypes(
+    module, Operand->getType().getASTType(), DestFormalTy);
   SmallVector<SILValue, 8> TypeDependentOperands;
   collectTypeDependentOperands(TypeDependentOperands, F, DestFormalTy);
   unsigned size =
       totalSizeToAlloc<swift::Operand>(1 + TypeDependentOperands.size());
-  void *Buffer = Mod.allocateInst(size, alignof(CheckedCastBranchInst));
+  void *Buffer = module.allocateInst(size, alignof(CheckedCastBranchInst));
   return ::new (Buffer) CheckedCastBranchInst(
       DebugLoc, IsExact, Operand, TypeDependentOperands, DestLoweredTy,
       DestFormalTy, SuccessBB, FailureBB, Target1Count, Target2Count,
-      forwardingOwnershipKind);
+      forwardingOwnershipKind, preservesOwnership);
 }
 
 MetatypeInst *MetatypeInst::create(SILDebugLocation Loc, SILType Ty,

lib/SILGen/SILGenBuilder.cpp

@@ -18,6 +18,7 @@
 #include "SwitchEnumBuilder.h"
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/SubstitutionMap.h"
+#include "swift/SIL/DynamicCasts.h"
 
 using namespace swift;
 using namespace Lowering;
@@ -527,10 +528,9 @@ void SILGenBuilder::createCheckedCastBranch(SILLocation loc, bool isExact,
                                             SILBasicBlock *falseBlock,
                                             ProfileCounter Target1Count,
                                             ProfileCounter Target2Count) {
-  // Check if our source type is AnyObject. In such a case, we need to ensure
-  // plus one our operand since SIL does not support guaranteed casts from an
-  // AnyObject.
-  if (op.getType().isAnyObject()) {
+  // Casting a guaranteed value requires ownership preservation.
+  if (!doesCastPreserveOwnershipForTypes(SGF.SGM.M, op.getType().getASTType(),
+                                         destFormalTy)) {
     op = op.ensurePlusOne(SGF, loc);
   }
   createCheckedCastBranch(loc, isExact, op.forward(SGF),

test/SILGen/opaque_values_silgen.swift

@@ -4,6 +4,8 @@
 
 // FIXME: "HECK" lines all need to be updated for OSSA.
 
+class C {}
+
 func genericInout<T>(_: inout T) {}
 
 func hasVarArg(_ args: Any...) {}
@@ -384,3 +386,50 @@ func testEmptyReturnClosure() {
   let b = nil ?? { bar() }
 }
 
+// Test that PatternMatchEmission::emitIsDispatch can emit a
+// class-to-AnyObject cast as a guaranteed scalar cast
+// (doesCastPreserveOwnershipForTypes returns true).
+//
+// CHECK-LABEL: sil hidden [ossa] @$s20opaque_values_silgen24testCastClassToAnyObjectyyXlAA1CCF : $@convention(thin) (@guaranteed C) -> @owned AnyObject {
+// CHECK: bb0(%0 : @guaranteed $C):
+// CHECK:   checked_cast_br %0 : $C to AnyObject, bb2, bb1
+// CHECK: bb1(%{{.*}} : @guaranteed $C):
+// CHECK: bb2(%{{.*}} : @guaranteed $AnyObject):
+// CHECK-LABEL: } // end sil function
+func testCastClassToAnyObject(_ c: C) -> AnyObject {
+  switch (c) {
+  case let x as AnyObject:
+    _ = x
+    break
+  }
+}
+
+// CHECK-LABEL: sil hidden [ossa] @$s20opaque_values_silgen24testCastAnyObjectToClassyAA1CCyXlF : $@convention(thin) (@guaranteed AnyObject) -> @owned C {
+// CHECK: bb0(%0 : @guaranteed $AnyObject):
+// CHECK:   [[CP:%.*]] = copy_value %0 : $AnyObject
+// CHECK:   checked_cast_br [[CP]] : $AnyObject to C, bb1, bb2
+// CHECK-LABEL: } // end sil function '$s20opaque_values_silgen24testCastAnyObjectToClassyAA1CCyXlF'
+func testCastAnyObjectToClass(_ o: AnyObject) -> C {
+  switch (o) {
+  case let x as C:
+    _ = x
+    break
+  default:
+    break
+  }
+}
+
+// CHECK-LABEL: sil hidden [ossa] @$s20opaque_values_silgen024testCastClassArchetypeToF0yAA1CCxRlzClF : $@convention(thin) <T where T : AnyObject> (@guaranteed T) -> @owned C {
+// CHECK: bb0(%0 : @guaranteed $T):
+// CHECK:   [[CP:%.*]] = copy_value %0 : $T
+// CHECK:   checked_cast_br [[CP]] : $T to C, bb1, bb2
+// CHECK-LABEL: } // end sil function '$s20opaque_values_silgen024testCastClassArchetypeToF0yAA1CCxRlzClF'
+func testCastClassArchetypeToClass<T : AnyObject>(_ o: T) -> C {
+  switch (o) {
+  case let x as C:
+    _ = x
+    break
+  default:
+    break
+  }
+}

test/SILOptimizer/rcidentity_opaque.sil

@@ -10,12 +10,24 @@ class Base {}
 
 class Sub : Base {}
 
+struct Bool {
+  var value : Builtin.Int1
+}
+
 protocol P {}
 
 protocol PClass : AnyObject {}
 
 protocol PBase : Base {}
 
+public protocol Hashable {}
+
+public struct AnyHashable: Hashable {
+  internal var base: Any
+
+  public init<H: Hashable>(_ base: H)
+}
+
 // Test dynamic casts that preserve ownership. See DynamicCasts.cpp
 // swift::doesCastPreserveOwnershipForTypes.
 //
@@ -24,6 +36,7 @@ protocol PBase : Base {}
 //   (A) one type is a bridged value and the other is an object:
 //      (A1) Boxing: <trivial> as! Object
 //      (A2) Unboxing: Object as! <trivial>
+//      (A3) Class-bridging: Error as! NSError
 //
 //   (B) one type is transparently wrapped in __SwiftValue, while the other is
 //       unwrapped. Given:
@@ -270,33 +283,46 @@ bb0(%0 : @owned $T):
 }
 
 // =============================================================================
-// (B1) Wrapped casts - may forward through a wrapper object
-//
-// Note: the AnyHashable-to-AnyObject cases are currently covered
-// by the potentially bridged casts.
+// (A3) Class bridging
 
-// TODO: The compiler could recognize this case as forwarded. Casting
-// *from* a class to AnyObject will not wrap the class.
+// Casting to NSError is indirect since it may conform to Error and
+// require Error-to-NSError bridging, unless we can statically see
+// that the source type inherits NSError.
+//
+// A class-constrained archetype may be bound to NSError, unless it has a
+// non-NSError superclass constraint. Casts to archetypes thus must always be
+// indirect.
 //
-// CHECK-LABEL: @testClassToClassArchetypeIsWrapped
+// CHECK-LABEL: @testClassToClassArchetypeIsBridged
 // CHECK: RESULT #0: 0 = 0
 // CHECK: RESULT #1: 1 = 1
-sil [ossa] @testClassToClassArchetypeIsWrapped : $@convention(thin) <U : AnyObject>(@owned Base) -> @owned U {
+sil [ossa] @testClassToClassArchetypeIsBridged : $@convention(thin) <U : AnyObject>(@owned Base) -> @owned U {
 bb0(%0 : @owned $Base):
   %1 = unconditional_checked_cast %0 : $Base to U
   return %1 : $U
 }
 
 // =============================================================================
-// (B2) Wrapped casts - may forward through a wrapper object
+// (B1) __SwiftValue Wrapping
 //
-// Note: (B1) the AnyHashable-to-AnyObject cases are currently covered
-// by the potentially bridged casts.
+// Note: the AnyHashable-to-AnyObject cases are currently covered by
+// the same logic that checks for potentially bridged casts. We
+// include it here for completeness.
+
+// CHECK-LABEL: @testNonClassToClassArchetypeIsWrapped
+// CHECK: RESULT #0: 0 = 0
+// CHECK: RESULT #1: 1 = 1
+sil [ossa] @testNonClassToClassArchetypeIsWrapped : $@convention(thin) <U : AnyObject>(@owned AnyHashable) -> @owned U {
+bb0(%0 : @owned $AnyHashable):
+  %1 = unconditional_checked_cast %0 : $AnyHashable to U
+  return %1 : $U
+}
+
+// =============================================================================
+// (B2) __SwiftValue Unwrapping
 //
 // TODO: In the future, when the runtime stops wrapping nontrivial types inside
-// __SwiftValue, cases (B1) and (B2) above will no longer apply. At that time,
-// expand ownership preserving cast types to AnyObject. Then remove the
-// isPotentiallyAnyObject() check.
+// __SwiftValue, cases (B1) and (B2) will no longer apply.
 
 // CHECK-LABEL: @testAnyObjectToClassIsWrapped
 // CHECK: RESULT #0: 0 = 0
@@ -310,9 +336,9 @@ bb0(%0 : @owned $AnyObject):
 // CHECK-LABEL: @testClassArchetypeToClassIsWrapped
 // CHECK: RESULT #0: 0 = 0
 // CHECK: RESULT #1: 1 = 1
-sil [ossa] @testClassArchetypeToClassIsWrapped : $@convention(thin) <U : AnyObject>(@owned U) -> @owned Sub {
-bb0(%0 : @owned $U):
-  %1 = unconditional_checked_cast %0 : $U to Sub
+sil [ossa] @testClassArchetypeToClassIsWrapped : $@convention(thin) <T : AnyObject>(@owned T) -> @owned Sub {
+bb0(%0 : @owned $T):
+  %1 = unconditional_checked_cast %0 : $T to Sub
   return %1 : $Sub
 }
 
@@ -402,3 +428,12 @@ bb0(%0 : @owned $Base):
   %1 = unconditional_checked_cast %0 : $Base to Sub
   return %1 : $Sub
 }
+
+// CHECK-LABEL: @testClassToAnyObjectIsForwarded
+// CHECK: RESULT #0: 0 = 0
+// CHECK: RESULT #1: 1 = 0
+sil [ossa] @testClassToAnyObjectIsForwarded : $@convention(thin) (@owned Base) -> @owned AnyObject {
+bb0(%0 : @owned $Base):
+  %1 = unconditional_checked_cast %0 : $Base to AnyObject
+  return %1 : $AnyObject
+}