Merge pull request swiftlang#77390 from eeckstein/fix-rc-identity-analysis

RCIdentityAnalysis: don't let a non-copyable value be the RC root of a copyable value

Author: eeckstein

lib/SIL/Verifier/SILVerifier.cpp

@@ -3391,6 +3391,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
   void checkRetainValueInst(RetainValueInst *I) {
     require(I->getOperand()->getType().isObject(),
             "Source value should be an object value");
+    require(!I->getOperand()->getType().isMoveOnly(),
+            "retain value operand type must be copyable");
     require(!F.hasOwnership(),
             "retain_value is only in functions with unqualified ownership");
   }

lib/SILOptimizer/Analysis/RCIdentityAnalysis.cpp

@@ -81,8 +81,10 @@ static SILValue stripRCIdentityPreservingInsts(SILValue V) {
   // the struct is equivalent to a ref count operation on the extracted
   // member. Strip off the extract.
   if (auto *SEI = dyn_cast<StructExtractInst>(V))
-    if (SEI->isFieldOnlyNonTrivialField() && !hasValueDeinit(SEI->getOperand()))
+    if (SEI->isFieldOnlyNonTrivialField() && !SEI->getOperand()->getType().isMoveOnly()) {
+      assert(!hasValueDeinit(SEI->getOperand()));
       return SEI->getOperand();
+    }
 
   // If we have a struct instruction with only one non-trivial stored field, the
   // only reference count that can be modified is the non-trivial field. Return

test/SILOptimizer/earlycodemotion_moveonly.sil

@@ -23,6 +23,13 @@ enum MaybeFileDescriptor: ~Copyable {
   deinit
 }
 
+struct CS {
+  let c: AnyObject
+}
+
+public struct S : ~Copyable {
+  var a: CS
+}
 
 // Test that a release_value is not replaced for a enum-with-deinit.
 // Doing so would forget the deinit.
@@ -39,3 +46,21 @@ bb0:
   %42 = tuple ()
   return %42 : $()
 }
+
+sil @useCS : $@convention(thin) (@guaranteed CS) -> ()
+
+// CHECK-LABEL: sil @testRCroot :
+// CHECK-NOT:     retain_value %{{[0-9]*}} : $S 
+// CHECK:       } // end sil function 'testRCroot'
+sil @testRCroot : $@convention(method) (@guaranteed S) -> () {
+bb0(%0 : $S):
+  %2 = struct_extract %0 : $S, #S.a
+  %3 = struct_extract %2 : $CS, #CS.c
+  strong_retain %3 : $AnyObject
+  %5 = function_ref @useCS : $@convention(thin) (@guaranteed CS) -> ()
+  %6 = apply %5(%2) : $@convention(thin) (@guaranteed CS) -> ()
+  release_value %0 : $S
+  %9 = tuple ()
+  return %9 : $()
+}
+

test/SILOptimizer/simplify_retain_value.sil

@@ -19,17 +19,6 @@ struct WithDeinit : ~Copyable {
   deinit
 }
 
-struct Outer : ~Copyable {
-  var wd: WithDeinit
-}
-
-enum EnumWithDeinit: ~Copyable {
-  case A(Int64)
-  case B
-
-  deinit
-}
-
 struct TwoEnums {
   var u1: U
   var u2: U
@@ -62,16 +51,6 @@ bb0(%0 : $Builtin.Int8, %1 : $Builtin.NativeObject):
   return %4 : $(Builtin.Int8, Builtin.NativeObject)
 }
 
-// CHECK-LABEL: sil @dont_remove_struct_with_deinit_field
-// CHECK:         retain_value %0
-// CHECK:       } // end sil function 'dont_remove_struct_with_deinit_field'
-sil @dont_remove_struct_with_deinit_field : $@convention(thin) (Outer) -> () {
-bb0(%0 : $Outer):
-  retain_value %0 : $Outer
-  %2 = tuple ()
-  return %2 : $()
-}
-
 // CHECK-LABEL: sil @struct_with_two_trivial_enums
 // CHECK-NOT:     retain_value
 // CHECK:       } // end sil function 'struct_with_two_trivial_enums'
@@ -138,18 +117,6 @@ bb0(%0 : $@thin U.Type, %1 : $C):
   return %6 : $(U, U, U)
 }
 
-// CHECK-LABEL: sil @dont_replace_payload_of_enum_with_deinit
-// CHECK:         %1 = enum
-// CHECK:         retain_value %1
-// CHECK:       } // end sil function 'dont_replace_payload_of_enum_with_deinit'
-sil @dont_replace_payload_of_enum_with_deinit : $@convention(thin) (Int64) -> () {
-bb0(%0 : $Int64):
-  %1 = enum $EnumWithDeinit, #EnumWithDeinit.A!enumelt, %0 : $Int64
-  retain_value %1 : $EnumWithDeinit
-  %3 = tuple ()
-  return %3 : $()
-}
-
 // CHECK-LABEL: sil @release_then_retain_peephole
 // CHECK:       bb0
 // CHECK-NOT:     retain_value