Skip to content

Commit d609889

Browse files
authored
Merge pull request #85934 from atrick/fix-closure-liveness
Fix InteriorUseWalker: consider partial_apply [on_stack] an escape
2 parents 9dd9e96 + f7c3c6f commit d609889

File tree

2 files changed

+37
-2
lines changed

2 files changed

+37
-2
lines changed

SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -626,9 +626,13 @@ extension InteriorUseWalker: OwnershipUseVisitor {
626626
if value.type.isTrivial(in: function) {
627627
return .continueWalk
628628
}
629-
guard value.type.isEscapable(in: function) else {
629+
guard value.mayEscape else {
630630
// Non-escapable dependent values can be lifetime-extended by copying, which is not handled by
631-
// InteriorUseWalker. LifetimeDependenceDefUseWalker does this.
631+
// InteriorUseWalker. LifetimeDependenceDefUseWalker does this. Alternatively, we could continue to `walkDownUses`
632+
// but later recognize a copy of a `mayEscape` value to be a pointer escape at that point.
633+
//
634+
// This includes partial_apply [on_stack] which can currently be copied. Although a better solution would be to
635+
// make copying an on-stack closure illegal SIL.
632636
return pointerEscapingUse(of: operand)
633637
}
634638
if useVisitor(operand) == .abortWalk {

test/SILOptimizer/mandatory-destroy-hoisting.sil

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,11 @@ bb0(%0 : @owned $String):
319319
return %r
320320
}
321321

322+
// TODO: If InteriorLiveness handles `partial_apply [on_stack]`, then the destroy can be hoisted.
323+
//
322324
// CHECK-LABEL: sil [ossa] @partial_apply :
323325
// CHECK: destroy_value %1
326+
// CHECK-NEXT: debug_step
324327
// CHECK-NEXT: destroy_value %0
325328
// CHECK: } // end sil function 'partial_apply'
326329
sil [ossa] @partial_apply : $@convention(thin) (@owned String) -> () {
@@ -445,3 +448,31 @@ bb0:
445448
%r = tuple ()
446449
return %r
447450
}
451+
452+
sil @closure1 : $@convention(thin) (@guaranteed C) -> ()
453+
454+
// rdar165850554: InteriorUseDefWalker must consider copies of an on-stack partial apply when computing liveness of the
455+
// borrowed operand.
456+
457+
// CHECK-LABEL: sil private [ossa] @test_closure_copy :
458+
// CHECK: %1 = copy_value %0
459+
// CHECK: debug_step
460+
// CHECK-NEXT: destroy_value %1
461+
// CHECK-LABEL: } // end sil function 'test_closure_copy'
462+
sil private [ossa] @test_closure_copy : $@convention(thin) (@owned C) -> () {
463+
bb0(%0 : @owned $C):
464+
// create a non-lexical value to trigger mandatory destroy hoisting
465+
%1 = copy_value %0
466+
%2 = function_ref @closure1 : $@convention(thin) (@guaranteed C) -> ()
467+
%3 = partial_apply [callee_guaranteed] [on_stack] %2(%1) : $@convention(thin) (@guaranteed C) -> ()
468+
%4 = copy_value %3
469+
// force a pointer-escape
470+
%5 = unchecked_bitwise_cast %4 to $@noescape @callee_guaranteed () -> ()
471+
destroy_value %3
472+
destroy_value %0
473+
destroy_value %4
474+
debug_step
475+
destroy_value %1
476+
%10 = tuple ()
477+
return %10
478+
}

0 commit comments

Comments
 (0)