[ownership] Change init_existential_ref to be forwarding for both guaranteed and owned.

Otherwise in call frames like the one in the test in this commit get unneeded
ARC traffic. We should never pessimize read only code that doesnt need
side-effects with side-effects if we can avoid it.

I am seeing this a bunch when I look at SIL from projects that use a lot of
protocols. Specifically, one has a sort of trampoline code that wraps a ref
counted object in an existential ref container (which from an ARC perspective
doesn't imply ownership) and then calls a method on it or passes it off to some
other code.

Because of this requirement, there is a copy/destroy that can not be eliminated
unless we can devirt/inline/eliminate the init_existential_ref box, inline
enough that the low level ARC optimizer can hit it. We shouldn't rely on such
properties if we do not need to.

Author: gottesmm

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 : $()
+}