averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-14 20:36:38 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix InteriorUseWalker: consider partial_apply [on_stack] an escape

Browse Source 
Fixes a bug in MandatoryDestroyHoisting where a captured value is destroyed
before a copy of the closure.

On-stack closures can be copied, and all copied uses must be within the borrow
scope of the captured operand. This is just like any other non-Escapable value,
so treat it as such by checking `Value.mayEscape` rather than `Type.Escapable`.

Originally, I wanted to make it illegal to copy of partial_apply [on_stack], but
it looks like we still allow it.

I would rather not complicate any logic yet with special handling for this
case. To fix any performance concerns, we might be able to simplify the
representation instead by banning copy_value on on-stack closures.

Fixes rdar://165850554 swift-frontend crash: While running "CopyPropagation" -
Invalid SIL provided to OSSACompleteLifetime?!)

(cherry picked from commit f7c3c6f437)
This commit is contained in:
Andrew Trick
2025-12-09 14:31:54 -08:00
parent 83b7095c4d
commit 68bfbfa476
2 changed files with 37 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift
View File

@@ -626,9 +626,13 @@ extension InteriorUseWalker: OwnershipUseVisitor {
if value.type.isTrivial(in: function) {
return .continueWalk
}
guard value.type.isEscapable(in: function) else {
guard value.mayEscape else {
// Non-escapable dependent values can be lifetime-extended by copying, which is not handled by
// InteriorUseWalker. LifetimeDependenceDefUseWalker does this.
// InteriorUseWalker. LifetimeDependenceDefUseWalker does this. Alternatively, we could continue to `walkDownUses`
// but later recognize a copy of a `mayEscape` value to be a pointer escape at that point.
//
// This includes partial_apply [on_stack] which can currently be copied. Although a better solution would be to
// make copying an on-stack closure illegal SIL.
return pointerEscapingUse(of: operand)
}
if useVisitor(operand) == .abortWalk {

31
test/SILOptimizer/mandatory-destroy-hoisting.sil
View File

@@ -319,8 +319,11 @@ bb0(%0 : @owned $String):
return %r
}
// TODO: If InteriorLiveness handles `partial_apply [on_stack]`, then the destroy can be hoisted.
//
// CHECK-LABEL: sil [ossa] @partial_apply :
// CHECK: destroy_value %1
// CHECK-NEXT: debug_step
// CHECK-NEXT: destroy_value %0
// CHECK: } // end sil function 'partial_apply'
sil [ossa] @partial_apply : $@convention(thin) (@owned String) -> () {
@@ -445,3 +448,31 @@ bb0:
%r = tuple ()
return %r
}
sil @closure1 : $@convention(thin) (@guaranteed C) -> ()
// rdar165850554: InteriorUseDefWalker must consider copies of an on-stack partial apply when computing liveness of the
// borrowed operand.
// CHECK-LABEL: sil private [ossa] @test_closure_copy :
// CHECK: %1 = copy_value %0
// CHECK: debug_step
// CHECK-NEXT: destroy_value %1
// CHECK-LABEL: } // end sil function 'test_closure_copy'
sil private [ossa] @test_closure_copy : $@convention(thin) (@owned C) -> () {
bb0(%0 : @owned $C):
// create a non-lexical value to trigger mandatory destroy hoisting
%1 = copy_value %0
%2 = function_ref @closure1 : $@convention(thin) (@guaranteed C) -> ()
%3 = partial_apply [callee_guaranteed] [on_stack] %2(%1) : $@convention(thin) (@guaranteed C) -> ()
%4 = copy_value %3
// force a pointer-escape
%5 = unchecked_bitwise_cast %4 to $@noescape @callee_guaranteed () -> ()
destroy_value %3
destroy_value %0
destroy_value %4
debug_step
destroy_value %1
%10 = tuple ()
return %10
}