From 285c60d959cea30d9716373c8d7b7bb72fb73fdd Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Thu, 30 Nov 2023 10:10:07 +0100 Subject: [PATCH] deinit-devirtualization: correctly handle drop_deinit for address types Make it clear that drop_deinit cannot be used to prevent a deinit called from a destroy_addr. This is more a refactoring and clarification than a bug fix, because a destroy_addr cannot have a drop_deinit as operand, anyway. --- .../Optimizer/Utilities/Devirtualization.swift | 12 +++++++++--- test/SILOptimizer/devirt_deinits.sil | 17 ++++------------- .../moveonly_deinit_devirtualization.sil | 12 ------------ ...einit_devirtualization_library_evolution.sil | 12 ------------ 4 files changed, 13 insertions(+), 40 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/Devirtualization.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/Devirtualization.swift index 1067276a7db1a..3afed51fc0dcf 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Utilities/Devirtualization.swift +++ b/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) diff --git a/test/SILOptimizer/devirt_deinits.sil b/test/SILOptimizer/devirt_deinits.sil index b7eaefbfda28c..f58bde2c750fd 100644 --- a/test/SILOptimizer/devirt_deinits.sil +++ b/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 : $() } diff --git a/test/SILOptimizer/moveonly_deinit_devirtualization.sil b/test/SILOptimizer/moveonly_deinit_devirtualization.sil index 328385065d93e..87ded98ab9fbd 100644 --- a/test/SILOptimizer/moveonly_deinit_devirtualization.sil +++ b/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 diff --git a/test/SILOptimizer/moveonly_deinit_devirtualization_library_evolution.sil b/test/SILOptimizer/moveonly_deinit_devirtualization_library_evolution.sil index 7f65eb3bebadf..daab4312bbb88 100644 --- a/test/SILOptimizer/moveonly_deinit_devirtualization_library_evolution.sil +++ b/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