Merge pull request #30291 from gottesmm/pr-20d3a11a596c39f79b461a829ff03722b9755ec9

Author: swift-ci

include/swift/SIL/SILInstruction.h

@@ -6192,7 +6192,7 @@ class InitExistentialValueInst final
 class InitExistentialRefInst final
     : public UnaryInstructionWithTypeDependentOperandsBase<
           SILInstructionKind::InitExistentialRefInst, InitExistentialRefInst,
-          SingleValueInstruction> {
+          OwnershipForwardingSingleValueInst> {
   friend SILBuilder;
 
   CanType ConcreteType;
@@ -6203,7 +6203,8 @@ class InitExistentialRefInst final
                          ArrayRef<SILValue> TypeDependentOperands,
                          ArrayRef<ProtocolConformanceRef> Conformances)
       : UnaryInstructionWithTypeDependentOperandsBase(
-            DebugLoc, Instance, TypeDependentOperands, ExistentialType),
+          DebugLoc, Instance, TypeDependentOperands, ExistentialType,
+          Instance.getOwnershipKind()),
         ConcreteType(FormalConcreteType), Conformances(Conformances) {}
 
   static InitExistentialRefInst *

lib/SIL/OperandOwnership.cpp

@@ -173,7 +173,6 @@ CONSTANT_OWNERSHIP_INST(Owned, MustBeInvalidated, DeallocExistentialBox)
 CONSTANT_OWNERSHIP_INST(Owned, MustBeInvalidated, DeallocRef)
 CONSTANT_OWNERSHIP_INST(Owned, MustBeInvalidated, DestroyValue)
 CONSTANT_OWNERSHIP_INST(Owned, MustBeInvalidated, EndLifetime)
-CONSTANT_OWNERSHIP_INST(Owned, MustBeInvalidated, InitExistentialRef)
 CONSTANT_OWNERSHIP_INST(None, MustBeLive, AbortApply)
 CONSTANT_OWNERSHIP_INST(None, MustBeLive, AddressToPointer)
 CONSTANT_OWNERSHIP_INST(None, MustBeLive, BeginAccess)
@@ -348,6 +347,7 @@ FORWARD_ANY_OWNERSHIP_INST(UnconditionalCheckedCast)
 FORWARD_ANY_OWNERSHIP_INST(UncheckedEnumData)
 FORWARD_ANY_OWNERSHIP_INST(DestructureStruct)
 FORWARD_ANY_OWNERSHIP_INST(DestructureTuple)
+FORWARD_ANY_OWNERSHIP_INST(InitExistentialRef)
 #undef FORWARD_ANY_OWNERSHIP_INST
 
 // An instruction that forwards a constant ownership or trivial ownership.

lib/SIL/OwnershipUtils.cpp

@@ -46,6 +46,7 @@ bool swift::isOwnershipForwardingValueKind(SILNodeKind kind) {
   case SILNodeKind::DestructureStructInst:
   case SILNodeKind::DestructureTupleInst:
   case SILNodeKind::MarkDependenceInst:
+  case SILNodeKind::InitExistentialRefInst:
     return true;
   default:
     return false;

lib/SIL/ValueOwnership.cpp

@@ -78,13 +78,6 @@ CONSTANT_OWNERSHIP_INST(Owned, KeyPath)
 CONSTANT_OWNERSHIP_INST(Owned, InitExistentialValue)
 CONSTANT_OWNERSHIP_INST(Owned, GlobalValue) // TODO: is this correct?
 
-// NOTE: Even though init_existential_ref from a reference counting perspective
-// is not considered to be "owned" since it doesn't affect reference counts,
-// conceptually we want to treat it as an owned value that produces owned
-// things, rather than a forwarding thing since initialization is generally a
-// consuming operation.
-CONSTANT_OWNERSHIP_INST(Owned, InitExistentialRef)
-
 // One would think that these /should/ be unowned. In truth they are owned since
 // objc metatypes do not go through the retain/release fast path. In their
 // implementations of retain/release nothing happens, so this is safe.
@@ -260,6 +253,16 @@ FORWARDING_OWNERSHIP_INST(Upcast)
 FORWARDING_OWNERSHIP_INST(UncheckedEnumData)
 FORWARDING_OWNERSHIP_INST(SelectEnum)
 FORWARDING_OWNERSHIP_INST(Enum)
+// NOTE: init_existential_ref from a reference counting perspective is not
+// considered to be "owned" since it doesn't affect reference counts. That being
+// said in the past, we wanted to conceptually treat it as an owned value that
+// produces owned things, rather than a forwarding thing since initialization is
+// generally a consuming operation. That being said, there are often cases in
+// class based code where we are propagating around a plus zero version of a
+// value and need to wrap the class in an existential wrapper in an intermediate
+// frame from usage. In such cases, we have been creating unnecessary ref count
+// traffic in code.
+FORWARDING_OWNERSHIP_INST(InitExistentialRef)
 #undef FORWARDING_OWNERSHIP_INST
 
 ValueOwnershipKind

lib/SILOptimizer/Transforms/SemanticARCOpts.cpp

@@ -593,6 +593,7 @@ struct SemanticARCOptVisitor
   FORWARDING_INST(OpenExistentialValue)
   FORWARDING_INST(OpenExistentialBoxValue)
   FORWARDING_INST(MarkDependence)
+  FORWARDING_INST(InitExistentialRef)
 #undef FORWARDING_INST
 
 #define FORWARDING_TERM(NAME)                                                  \

test/SIL/ownership-verifier/over_consume.sil

@@ -460,32 +460,6 @@ bb0(%0 : @owned $Klass):
   return %9999 : $()
 }
 
-// CHECK-LABEL: Function: 'init_existential_ref_not_forwarding'
-// CHECK: Have operand with incompatible ownership?!
-// CHECK: Value: %0 = argument of bb0 : $ClassProtConformingRef
-// CHECK: User:   %2 = init_existential_ref %0 : $ClassProtConformingRef : $ClassProtConformingRef, $ClassProt
-// CHECK: Operand Number: 0
-// CHECK: Conv: guaranteed
-// CHECK: OwnershipMap:
-// CHECK: -- OperandOwnershipKindMap --
-// CHECK: unowned:  No.
-// CHECK: owned: Yes. Liveness: MustBeInvalidated
-// CHECK: guaranteed:  No.
-// CHECK: any: Yes. Liveness: MustBeLive
-// CHECK-NOT: init_existential_ref %1
-// CHECK: Function: 'init_existential_ref_not_forwarding'
-// CHECK: Error! Found a leaked owned value that was never consumed.
-// CHECK: Value:   %2 = init_existential_ref %0 : $ClassProtConformingRef : $ClassProtConformingRef, $ClassProt
-// CHECK-NOT: init_existential_ref %1
-sil [ossa] @init_existential_ref_not_forwarding : $@convention(thin) (@guaranteed ClassProtConformingRef, @owned ClassProtConformingRef) -> @owned (ClassProt, ClassProt) {
-bb0(%0 : @guaranteed $ClassProtConformingRef, %1 : @owned $ClassProtConformingRef):
-  %2 = init_existential_ref %0 : $ClassProtConformingRef : $ClassProtConformingRef, $ClassProt
-  %3 = copy_value %2 : $ClassProt
-  %4 = init_existential_ref %1 : $ClassProtConformingRef : $ClassProtConformingRef, $ClassProt
-  %5 = tuple(%3 : $ClassProt, %4 : $ClassProt)
-  return %5 : $(ClassProt, ClassProt)
-}
-
 sil [ossa] @eliminate_copy_try_apple_callee : $@convention(thin) (@owned Builtin.NativeObject) -> @error Error {
 entry(%0 : @owned $Builtin.NativeObject):
   %9999 = tuple()

test/SIL/ownership-verifier/use_verifier.sil

@@ -109,6 +109,10 @@ class UnsafeUnownedFieldKlass {
   unowned(safe) var x: @sil_unowned Builtin.NativeObject
 }
 
+class ClassProtConformingRef {}
+protocol ClassProt : AnyObject {}
+extension ClassProtConformingRef : ClassProt {}
+
 ////////////////
 // Test Cases //
 ////////////////
@@ -1292,3 +1296,12 @@ bb4:
   destroy_value %1 : $Builtin.NativeObject
   return %9999 : $()
 }
+
+sil [ossa] @init_existential_ref_forwarding : $@convention(thin) (@guaranteed ClassProtConformingRef, @owned ClassProtConformingRef) -> @owned (ClassProt, ClassProt) {
+bb0(%0 : @guaranteed $ClassProtConformingRef, %1 : @owned $ClassProtConformingRef):
+  %2 = init_existential_ref %0 : $ClassProtConformingRef : $ClassProtConformingRef, $ClassProt
+  %3 = copy_value %2 : $ClassProt
+  %4 = init_existential_ref %1 : $ClassProtConformingRef : $ClassProtConformingRef, $ClassProt
+  %5 = tuple(%3 : $ClassProt, %4 : $ClassProt)
+  return %5 : $(ClassProt, ClassProt)
+}

test/SILOptimizer/semantic-arc-opts.sil

@@ -8,6 +8,8 @@ import Builtin
 // Declarations //
 //////////////////
 
+typealias AnyObject = Builtin.AnyObject
+
 enum MyNever {}
 enum FakeOptional<T> {
 case none
@@ -26,7 +28,14 @@ struct NativeObjectPair {
 
 sil @get_nativeobject_pair : $@convention(thin) () -> @owned NativeObjectPair
 
-class Klass {}
+protocol MyFakeAnyObject : Klass {
+  func myFakeMethod()
+}
+
+final class Klass {}
+extension Klass : MyFakeAnyObject {
+  func myFakeMethod()
+}
 sil @guaranteed_klass_user : $@convention(thin) (@guaranteed Klass) -> ()
 sil @guaranteed_fakeoptional_klass_user : $@convention(thin) (@guaranteed FakeOptional<Klass>) -> ()
 sil @guaranteed_fakeoptional_classlet_user : $@convention(thin) (@guaranteed FakeOptional<ClassLet>) -> ()
@@ -1781,3 +1790,22 @@ bb0(%0 : @guaranteed $ClassLet):
   %9999 = tuple()
   return %9999 : $()
 }
+
+// Make sure we can chew through this and get rid of all ARC traffic.
+// CHECK-LABEL: sil [ossa] @init_existential_ref_forwarding_test : $@convention(thin) (@guaranteed Klass) -> () {
+// CHECK-NOT: copy_value
+// CHECK-NOT: begin_borrow
+// CHECK: } // end sil function 'init_existential_ref_forwarding_test'
+sil [ossa] @init_existential_ref_forwarding_test : $@convention(thin) (@guaranteed Klass) -> () {
+bb0(%0 : @guaranteed $Klass):
+  %0a = copy_value %0 : $Klass
+  %1 = init_existential_ref %0a : $Klass : $Klass, $MyFakeAnyObject
+  %1a = begin_borrow %1 : $MyFakeAnyObject
+  %2 = open_existential_ref %1a : $MyFakeAnyObject to $@opened("A2E21C52-6089-11E4-9866-3C0754723233") MyFakeAnyObject
+  %3 = witness_method $@opened("A2E21C52-6089-11E4-9866-3C0754723233") MyFakeAnyObject, #MyFakeAnyObject.myFakeMethod!1, %2 : $@opened("A2E21C52-6089-11E4-9866-3C0754723233") MyFakeAnyObject : $@convention(witness_method: MyFakeAnyObject) <τ_0_0 where τ_0_0 : MyFakeAnyObject> (@guaranteed τ_0_0) -> ()
+  apply %3<@opened("A2E21C52-6089-11E4-9866-3C0754723233") MyFakeAnyObject>(%2) : $@convention(witness_method: MyFakeAnyObject)  <τ_0_0 where τ_0_0 : MyFakeAnyObject> (@guaranteed τ_0_0) -> ()
+  end_borrow %1a : $MyFakeAnyObject
+  destroy_value %1 : $MyFakeAnyObject
+  %9999 = tuple()
+  return %9999 : $()
+}