Merge pull request #70117 from eeckstein/fix-deinit-devirtualization

deinit-devirtualization: correctly handle drop_deinit for address types

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/Utilities/Devirtualization.swift

@@ -36,7 +36,7 @@ private func devirtualize(destroy: some DevirtualizableDestroy, _ context: some
   precondition(type.isNominal, "non-copyable non-nominal types not supported, yet")
 
   let result: Bool
-  if type.nominal.hasValueDeinit && !destroy.hasDropDeinit {
+  if type.nominal.hasValueDeinit && !destroy.shouldDropDeinit {
     guard let deinitFunc = context.lookupDeinit(ofNominal: type.nominal) else {
       return false
     }
@@ -58,6 +58,7 @@ private func devirtualize(destroy: some DevirtualizableDestroy, _ context: some
 
 // Used to dispatch devirtualization tasks to `destroy_value` and `destroy_addr`.
 private protocol DevirtualizableDestroy : UnaryInstruction {
+  var shouldDropDeinit: Bool { get }
   func createDeinitCall(to deinitializer: Function, _ context: some MutatingContext)
   func devirtualizeStructFields(_ context: some MutatingContext) -> Bool
   func devirtualizeEnumPayload(enumCase: EnumCase, in block: BasicBlock, _ context: some MutatingContext) -> Bool
@@ -67,8 +68,6 @@ private protocol DevirtualizableDestroy : UnaryInstruction {
 private extension DevirtualizableDestroy {
   var type: Type { operand.value.type }
 
-  var hasDropDeinit: Bool { operand.value.lookThoughOwnershipInstructions is DropDeinitInst }
-
   func devirtualizeEnumPayloads(_ context: some MutatingContext) -> Bool {
     guard let cases = type.getEnumCases(in: parentFunction) else {
       return false
@@ -99,6 +98,8 @@ private extension DevirtualizableDestroy {
 }
 
 extension DestroyValueInst : DevirtualizableDestroy {
+  fileprivate var shouldDropDeinit: Bool { operand.value.lookThoughOwnershipInstructions is DropDeinitInst }
+
   fileprivate func createDeinitCall(to deinitializer: Function, _ context: some MutatingContext) {
     let builder = Builder(before: self, context)
     let subs = context.getContextSubstitutionMap(for: type)
@@ -162,6 +163,11 @@ extension DestroyValueInst : DevirtualizableDestroy {
 }
 
 extension DestroyAddrInst : DevirtualizableDestroy {
+  fileprivate var shouldDropDeinit: Bool {
+    // The deinit is always called by a destroy_addr. There must not be a `drop_deinit` as operand.
+    false
+  }
+
   fileprivate func createDeinitCall(to deinitializer: Function, _ context: some MutatingContext) {
     let builder = Builder(before: self, context)
     let subs = context.getContextSubstitutionMap(for: destroyedAddress.type)

test/SILOptimizer/devirt_deinits.sil

@@ -235,18 +235,6 @@ bb0(%0 : $*E2):
   return %r : $()
 }
 
-// CHECK-LABEL: sil [ossa] @test_drop_deinit_addr :
-// CHECK:         %1 = drop_deinit %0
-// CHECK:         end_lifetime %1
-// CHECK:       } // end sil function 'test_drop_deinit_addr'
-sil [ossa] @test_drop_deinit_addr : $@convention(thin) (@in S1) -> () {
-bb0(%0 : $*S1):
-  %1 = drop_deinit %0 : $*S1
-  destroy_addr %1 : $*S1
-  %r = tuple()
-  return %r : $()
-}
-
 // CHECK-LABEL: sil [ossa] @test_two_field_drop_deinit_addr :
 // CHECK:         %1 = drop_deinit %0
 // CHECK:         [[A1:%.*]] = struct_element_addr %1 : $*S4, #S4.a
@@ -261,7 +249,10 @@ bb0(%0 : $*S1):
 sil [ossa] @test_two_field_drop_deinit_addr : $@convention(thin) (@in S4) -> () {
 bb0(%0 : $*S4):
   %1 = drop_deinit %0 : $*S4
-  destroy_addr %1 : $*S4
+  %2 = struct_element_addr %1 : $*S4, #S4.a
+  destroy_addr %2 : $*S1
+  %4 = struct_element_addr %1 : $*S4, #S4.b
+  destroy_addr %4 : $*S2
   %r = tuple()
   return %r : $()
 }

test/SILOptimizer/moveonly_deinit_devirtualization.sil

@@ -329,18 +329,6 @@ bb0(%0 : @owned $StructDeinit):
   return %9999 : $()
 }
 
-// CHECK-LABEL: sil [ossa] @dropDeinitOnIndirectStruct : $@convention(thin) (@in StructDeinit) -> () {
-// CHECK:         %1 = drop_deinit %0
-// CHECK-NEXT:    end_lifetime %1
-// CHECK:       } // end sil function 'dropDeinitOnIndirectStruct'
-sil [ossa] @dropDeinitOnIndirectStruct : $@convention(thin) (@in StructDeinit) -> () {
-bb0(%0 : $*StructDeinit):
-  %1 = drop_deinit %0 : $*StructDeinit
-  destroy_addr %1 : $*StructDeinit
-  %9999 = tuple()
-  return %9999 : $()
-}
-
 sil @$s4main5KlassCfD : $@convention(method) (@owned Klass) -> ()
 sil @$s4main5KlassCACycfc : $@convention(method) (@owned Klass) -> @owned Klass
 sil @$s4main5KlassCfd : $@convention(method) (@guaranteed Klass) -> @owned Builtin.NativeObject

test/SILOptimizer/moveonly_deinit_devirtualization_library_evolution.sil

@@ -331,18 +331,6 @@ bb0(%0 : @owned $StructDeinit):
   return %9999 : $()
 }
 
-// CHECK-LABEL: sil [ossa] @dropDeinitOnIndirectStruct : $@convention(thin) (@in StructDeinit) -> () {
-// CHECK:         %1 = drop_deinit %0
-// CHECK-NEXT:    end_lifetime %1
-// CHECK:       } // end sil function 'dropDeinitOnIndirectStruct'
-sil [ossa] @dropDeinitOnIndirectStruct : $@convention(thin) (@in StructDeinit) -> () {
-bb0(%0 : $*StructDeinit):
-  %1 = drop_deinit %0 : $*StructDeinit
-  destroy_addr %1 : $*StructDeinit
-  %9999 = tuple()
-  return %9999 : $()
-}
-
 sil @$s4main5KlassCfD : $@convention(method) (@owned Klass) -> ()
 sil @$s4main5KlassCACycfc : $@convention(method) (@owned Klass) -> @owned Klass
 sil @$s4main5KlassCfd : $@convention(method) (@guaranteed Klass) -> @owned Builtin.NativeObject