Merge pull request swiftlang#34183 from eeckstein/fix-rcidentity

SILOptimizer: fix ARC code motion problem for metatype casts.

Author: eeckstein

lib/SILOptimizer/Analysis/RCIdentityAnalysis.cpp

@@ -58,14 +58,31 @@ static bool isRCIdentityPreservingCast(ValueKind Kind) {
   }
 }
 
+/// Returns a null SILValue if V has a trivial type, otherwise returns V.
+///
+/// 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.
+/// Now, if the RCRoot of such an AnyObject would be a trivial type, ARC
+/// optimizations get confused and might eliminate a retain of such an object
+/// completely.
+static SILValue noTrivialType(SILValue V, SILFunction *F) {
+  if (V->getType().isTrivial(*F))
+    return SILValue();
+  return V;
+}
+
 //===----------------------------------------------------------------------===//
 //                    RC Identity Root Instruction Casting
 //===----------------------------------------------------------------------===//
 
 static SILValue stripRCIdentityPreservingInsts(SILValue V) {
   // First strip off RC identity preserving casts.
-  if (isRCIdentityPreservingCast(V->getKind()))
-    return cast<SingleValueInstruction>(V)->getOperand(0);
+  if (isRCIdentityPreservingCast(V->getKind())) {
+    auto *inst = cast<SingleValueInstruction>(V);
+    return noTrivialType(inst->getOperand(0), inst->getFunction());
+  }
 
   // Then if we have a struct_extract that is extracting a non-trivial member
   // from a struct with no other non-trivial members, a ref count operation on
@@ -118,7 +135,7 @@ static SILValue stripRCIdentityPreservingInsts(SILValue V) {
   // purposes.
   if (auto *A = dyn_cast<SILPhiArgument>(V))
     if (SILValue Result = A->getSingleTerminatorOperand())
-      return Result;
+      return noTrivialType(Result, A->getFunction());
 
   return SILValue();
 }
@@ -321,6 +338,9 @@ SILValue RCIdentityFunctionInfo::stripRCIdentityPreservingArgs(SILValue V,
   }
 
   unsigned IVListSize = IncomingValues.size();
+  if (IVListSize == 1 &&
+      !noTrivialType(IncomingValues[0].second, A->getFunction()))
+    return SILValue();
 
   assert(IVListSize != 1 && "Should have been handled in "
          "stripRCIdentityPreservingInsts");

test/SILOptimizer/retain_release_code_motion.sil

@@ -718,6 +718,65 @@ bb2:
   return %9999 : $()
 }
 
+// Don't remove a retain of an AnyObject which comes from a metatype.
+//
+// CHECK-LABEL: sil @conditional_metatype_cast
+// CHECK: bb2([[ARG:%[0-9]+]] : $AnyObject):
+// CHECK: strong_retain [[ARG]]
+// CHECK: checked_cast_addr_br
+// CHECK: } // end sil function 'conditional_metatype_cast'
+sil @conditional_metatype_cast : $@convention(thin) () -> AnyObject {
+bb0:
+  %0 = metatype $@thick Int.Type
+  checked_cast_br %0 : $@thick Int.Type to AnyObject, bb2, bb1
+
+bb1:
+  unreachable
+
+bb2(%6 : $AnyObject):
+  strong_retain %6 : $AnyObject
+  %9 = alloc_stack $AnyObject
+  store %6 to %9 : $*AnyObject
+  %11 = alloc_stack $@thick Int.Type
+  checked_cast_addr_br take_always AnyObject in %9 : $*AnyObject to Int.Type in %11 : $*@thick Int.Type, bb3, bb4
+
+bb3:
+  dealloc_stack %11 : $*@thick Int.Type
+  dealloc_stack %9 : $*AnyObject
+  return %6 : $AnyObject
+
+bb4:
+  unreachable
+}
+
+// Don't remove a retain of an AnyObject which comes from a metatype.
+//
+// CHECK-LABEL: sil @unconditional_metatype_cast
+// CHECK: [[O:%[0-9]+]] = unconditional_checked_cast
+// CHECK: strong_retain [[O]]
+// CHECK: checked_cast_addr_br
+// CHECK: } // end sil function 'unconditional_metatype_cast'
+sil @unconditional_metatype_cast : $@convention(thin) () -> AnyObject {
+bb0:
+  %0 = metatype $@thick Int.Type
+  %6 = unconditional_checked_cast %0 : $@thick Int.Type to AnyObject
+  strong_retain %6 : $AnyObject
+  %9 = alloc_stack $AnyObject
+  store %6 to %9 : $*AnyObject
+  %11 = alloc_stack $@thick Int.Type
+  checked_cast_addr_br take_always AnyObject in %9 : $*AnyObject to Int.Type in %11 : $*@thick Int.Type, bb3, bb4
+
+bb3:
+  dealloc_stack %11 : $*@thick Int.Type
+  dealloc_stack %9 : $*AnyObject
+  return %6 : $AnyObject
+
+bb4:
+  unreachable
+}
+
+
+
 // Hoist releases above dealloc_stack
 // CHECK-LABEL: sil @testReleaseHoistDeallocStack : $@convention(thin) (AnyObject) -> () {
 // CHECK: bb0(%0 : $AnyObject):