[5.0] Fix SILCombine metatype cast optimization to avoid extending lifetimes.

include/swift/SILOptimizer/Utils/CastOptimizer.h

@@ -129,6 +129,10 @@ class CastOptimizer {
                                        SILValue Dest, CanType Source,
                                        CanType Target, SILBasicBlock *SuccessBB,
                                        SILBasicBlock *FailureBB);
+
+  SILInstruction *
+  optimizeMetatypeConversion(ConversionInst *MCI,
+                             MetatypeRepresentation Representation);
 };
 
 } // namespace swift

lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp

@@ -289,16 +289,19 @@ SILCombiner::visitUncheckedRefCastAddrInst(UncheckedRefCastAddrInst *URCI) {
 SILInstruction *
 SILCombiner::
 visitUnconditionalCheckedCastAddrInst(UnconditionalCheckedCastAddrInst *UCCAI) {
-  CastOpt.optimizeUnconditionalCheckedCastAddrInst(UCCAI);
+  if (CastOpt.optimizeUnconditionalCheckedCastAddrInst(UCCAI))
+    MadeChange = true;
+
   return nullptr;
 }
 
 SILInstruction *
 SILCombiner::
 visitUnconditionalCheckedCastInst(UnconditionalCheckedCastInst *UCCI) {
-  if (CastOpt.optimizeUnconditionalCheckedCastInst(UCCI))
+  if (CastOpt.optimizeUnconditionalCheckedCastInst(UCCI)) {
+    MadeChange = true;
     return nullptr;
-
+  }
   // FIXME: rename from RemoveCondFails to RemoveRuntimeAsserts.
   if (RemoveCondFails) {
     auto LoweredTargetType = UCCI->getType();
@@ -388,32 +391,6 @@ visitUncheckedBitwiseCastInst(UncheckedBitwiseCastInst *UBCI) {
   return nullptr;
 }
 
-/// Helper function for simplifying conversions between
-/// thick and objc metatypes.
-static SILInstruction *
-visitMetatypeConversionInst(SILBuilder &Builder, ConversionInst *MCI,
-                            MetatypeRepresentation Representation) {
-  SILValue Op = MCI->getOperand(0);
-  // Instruction has a proper target type already.
-  SILType Ty = MCI->getType();
-  auto MetatypeTy = Op->getType().getAs<AnyMetatypeType>();
-
-  if (MetatypeTy->getRepresentation() != Representation)
-    return nullptr;
-
-  if (isa<MetatypeInst>(Op))
-    return Builder.createMetatype(MCI->getLoc(), Ty);
-
-  if (auto *VMI = dyn_cast<ValueMetatypeInst>(Op))
-    return Builder.createValueMetatype(MCI->getLoc(), Ty, VMI->getOperand());
-
-  if (auto *EMI = dyn_cast<ExistentialMetatypeInst>(Op))
-    return Builder.createExistentialMetatype(MCI->getLoc(), Ty,
-                                             EMI->getOperand());
-
-  return nullptr;
-}
-
 SILInstruction *
 SILCombiner::visitThickToObjCMetatypeInst(ThickToObjCMetatypeInst *TTOCMI) {
   // Perform the following transformations:
@@ -425,8 +402,10 @@ SILCombiner::visitThickToObjCMetatypeInst(ThickToObjCMetatypeInst *TTOCMI) {
   //
   // (thick_to_objc_metatype (existential_metatype @thick)) ->
   // (existential_metatype @objc_metatype)
-  return visitMetatypeConversionInst(Builder, TTOCMI,
-                                     MetatypeRepresentation::Thick);
+  if (CastOpt.optimizeMetatypeConversion(TTOCMI, MetatypeRepresentation::Thick))
+    MadeChange = true;
+
+  return nullptr;
 }
 
 SILInstruction *
@@ -440,20 +419,26 @@ SILCombiner::visitObjCToThickMetatypeInst(ObjCToThickMetatypeInst *OCTTMI) {
   //
   // (objc_to_thick_metatype (existential_metatype @objc_metatype)) ->
   // (existential_metatype @thick)
-  return visitMetatypeConversionInst(Builder, OCTTMI,
-                                     MetatypeRepresentation::ObjC);
+  if (CastOpt.optimizeMetatypeConversion(OCTTMI, MetatypeRepresentation::ObjC))
+    MadeChange = true;
+
+  return nullptr;
 }
 
 SILInstruction *
 SILCombiner::visitCheckedCastBranchInst(CheckedCastBranchInst *CBI) {
-  CastOpt.optimizeCheckedCastBranchInst(CBI);
+  if (CastOpt.optimizeCheckedCastBranchInst(CBI))
+    MadeChange = true;
+
   return nullptr;
 }
 
 SILInstruction *
 SILCombiner::
 visitCheckedCastAddrBranchInst(CheckedCastAddrBranchInst *CCABI) {
-  CastOpt.optimizeCheckedCastAddrBranchInst(CCABI);
+  if (CastOpt.optimizeCheckedCastAddrBranchInst(CCABI))
+    MadeChange = true;
+
   return nullptr;
 }
 

lib/SILOptimizer/Utils/CastOptimizer.cpp

@@ -1576,3 +1576,40 @@ SILInstruction *CastOptimizer::optimizeUnconditionalCheckedCastAddrInst(
 
   return nullptr;
 }
+
+/// Simplify conversions between thick and objc metatypes.
+SILInstruction *CastOptimizer::optimizeMetatypeConversion(
+    ConversionInst *MCI, MetatypeRepresentation Representation) {
+  SILValue Op = MCI->getOperand(0);
+  // Instruction has a proper target type already.
+  SILType Ty = MCI->getType();
+  auto MetatypeTy = Op->getType().getAs<AnyMetatypeType>();
+
+  if (MetatypeTy->getRepresentation() != Representation)
+    return nullptr;
+
+  // Rematerialize the incoming metatype instruction with the outgoing type.
+  auto replaceCast = [&](SingleValueInstruction *NewCast) {
+    assert(Ty.getAs<AnyMetatypeType>()->getRepresentation()
+           == NewCast->getType().getAs<AnyMetatypeType>()->getRepresentation());
+    MCI->replaceAllUsesWith(NewCast);
+    EraseInstAction(MCI);
+    return NewCast;
+  };
+  if (auto *MI = dyn_cast<MetatypeInst>(Op)) {
+    return replaceCast(
+        SILBuilderWithScope(MCI).createMetatype(MCI->getLoc(), Ty));
+  }
+  // For metatype instructions that require an operand, generate the new
+  // metatype at the same position as the original to avoid extending the
+  // lifetime of `Op` past its destroy.
+  if (auto *VMI = dyn_cast<ValueMetatypeInst>(Op)) {
+    return replaceCast(SILBuilderWithScope(VMI).createValueMetatype(
+        MCI->getLoc(), Ty, VMI->getOperand()));
+  }
+  if (auto *EMI = dyn_cast<ExistentialMetatypeInst>(Op)) {
+    return replaceCast(SILBuilderWithScope(EMI).createExistentialMetatype(
+        MCI->getLoc(), Ty, EMI->getOperand()));
+  }
+  return nullptr;
+}

test/SILOptimizer/sil_combine.sil

@@ -3673,3 +3673,42 @@ bb0(%0 : $Builtin.Int64):
   return %4 : $Builtin.Int64
 }
 
+// <rdar://46746188> crash in swift_getObjCClassFromObject
+// class TestDocument: NSPersistentDocument {
+//   override init() {
+//     super.init()
+//   }
+//   convenience init(type: String) throws {
+//     self.init()
+//   }
+// }
+// After inlining the __allocating_init, we have two uses of the value_metatype.
+// The second use goes through a thick_to_objc_metatype. When SILCombine replaces 
+// thick_to_objc_metatype with value_metatype, it cannot extend the liverange
+// of value_metatype's operand.
+@objc class TestObjCInit {
+  init()
+
+  convenience init(type: String) throws
+}
+// CHECK-LABEL: sil hidden [thunk] @objc_init_partial_dealloc : $@convention(objc_method) (@owned TestObjCInit) -> Optional<TestObjCInit> {
+// CHECK: bb0(%0 : $TestObjCInit):
+// CHECK:   [[VMT2:%.*]] = value_metatype $@objc_metatype TestObjCInit.Type, %0 : $TestObjCInit
+// CHECK:   [[VMT:%.*]] = value_metatype $@thick TestObjCInit.Type, %0 : $TestObjCInit
+// CHECK:   dealloc_partial_ref %0 : $TestObjCInit, [[VMT]] : $@thick TestObjCInit.Type
+// CHECK-NOT: value_metatype
+// CHECK:   [[O:%.*]] = alloc_ref_dynamic [objc] [[VMT2]] : $@objc_metatype TestObjCInit.Type, $TestObjCInit
+// CHECK:   [[M:%.*]] = objc_method [[O]] : $TestObjCInit, #TestObjCInit.init!initializer.1.foreign : (TestObjCInit.Type) -> () -> TestObjCInit, $@convention(objc_method) (@owned TestObjCInit) -> @owned TestObjCInit
+// CHECK:   apply [[M]]([[O]]) : $@convention(objc_method) (@owned TestObjCInit) -> @owned TestObjCInit
+// CHECK-LABEL: } // end sil function 'objc_init_partial_dealloc'
+sil hidden [thunk] @objc_init_partial_dealloc : $@convention(objc_method) (@owned TestObjCInit) -> Optional<TestObjCInit> {
+bb0(%2 : $TestObjCInit):
+  %8 = value_metatype $@thick TestObjCInit.Type, %2 : $TestObjCInit
+  dealloc_partial_ref %2 : $TestObjCInit, %8 : $@thick TestObjCInit.Type
+  %11 = thick_to_objc_metatype %8 : $@thick TestObjCInit.Type to $@objc_metatype TestObjCInit.Type
+  %12 = alloc_ref_dynamic [objc] %11 : $@objc_metatype TestObjCInit.Type, $TestObjCInit
+  %13 = objc_method %12 : $TestObjCInit, #TestObjCInit.init!initializer.1.foreign : (TestObjCInit.Type) -> () -> TestObjCInit, $@convention(objc_method) (@owned TestObjCInit) -> @owned TestObjCInit
+  %14 = apply %13(%12) : $@convention(objc_method) (@owned TestObjCInit) -> @owned TestObjCInit
+  %19 = enum $Optional<TestObjCInit>, #Optional.some!enumelt.1, %14 : $TestObjCInit
+  return %19 : $Optional<TestObjCInit>
+}