Skip to content

deinit-devirtualization: correctly handle drop_deinit for address types #70117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 30, 2023
Merged

deinit-devirtualization: correctly handle drop_deinit for address types #70117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions SwiftCompilerSources/Sources/Optimizer/Utilities/Devirtualization.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
17 changes: 4 additions & 13 deletions test/SILOptimizer/devirt_deinits.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 : $()
}
Expand Down
12 changes: 0 additions & 12 deletions test/SILOptimizer/moveonly_deinit_devirtualization.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 0 additions & 12 deletions test/SILOptimizer/moveonly_deinit_devirtualization_library_evolution.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down