Skip to content

Commit 02385c8

Browse files
jckarterjckarter
committed
AllocBoxToStack: Don't try to destroy promoted stack closure captures.
If the partial_apply is already [on_stack], then ClosureLifetimeFixup would have already turned the captures into borrows and established their final lifetimes.
1 parent 69e4b95 commit 02385c8

File tree

2 files changed

+130
-14
lines changed

2 files changed

+130
-14
lines changed

‎lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -991,21 +991,25 @@ specializeApplySite(SILOptFunctionBuilder &FuncBuilder, ApplySite Apply,
991991
// the stack. The partial_apply had ownership of this box so we must now
992992
// release it explicitly when the partial_apply is released.
993993
if (Apply.getKind() == ApplySiteKind::PartialApplyInst) {
994-
if (PAFrontier.empty()) {
995-
auto *PAI = cast<PartialApplyInst>(Apply);
996-
ValueLifetimeAnalysis VLA(PAI, PAI->getUses());
997-
pass.CFGChanged |= !VLA.computeFrontier(
998-
PAFrontier, ValueLifetimeAnalysis::AllowToModifyCFG);
999-
assert(!PAFrontier.empty() &&
1000-
"partial_apply must have at least one use "
1001-
"to release the returned function");
1002-
}
994+
auto *PAI = cast<PartialApplyInst>(Apply);
995+
// If it's already been stack promoted, then the stack closure only
996+
// borrows its captures, and we don't need to adjust capture lifetimes.
997+
if (!PAI->isOnStack()) {
998+
if (PAFrontier.empty()) {
999+
ValueLifetimeAnalysis VLA(PAI, PAI->getUses());
1000+
pass.CFGChanged |= !VLA.computeFrontier(
1001+
PAFrontier, ValueLifetimeAnalysis::AllowToModifyCFG);
1002+
assert(!PAFrontier.empty() &&
1003+
"partial_apply must have at least one use "
1004+
"to release the returned function");
1005+
}
10031006

1004-
// Insert destroys of the box at each point where the partial_apply
1005-
// becomes dead.
1006-
for (SILInstruction *FrontierInst : PAFrontier) {
1007-
SILBuilderWithScope Builder(FrontierInst);
1008-
Builder.emitDestroyValueOperation(Apply.getLoc(), Box);
1007+
// Insert destroys of the box at each point where the partial_apply
1008+
// becomes dead.
1009+
for (SILInstruction *FrontierInst : PAFrontier) {
1010+
SILBuilderWithScope Builder(FrontierInst);
1011+
Builder.emitDestroyValueOperation(Apply.getLoc(), Box);
1012+
}
10091013
}
10101014
}
10111015
}

‎test/SILOptimizer/allocboxtostack_after_closure_lifetime_fixup.sil

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -allocbox-to-stack | %FileCheck %s
2+
3+
// Verify that AllocBoxToStack can properly rewrite stack closures when
4+
// promoting captures after ClosureLifetimeFixup has established stack closures.
5+
6+
sil_stage canonical
7+
8+
import Swift
9+
import Builtin
10+
11+
class Fooer {
12+
func foo1()
13+
func foo2()
14+
}
15+
16+
// closure #1 in Fooer.foo()
17+
sil private [ossa] @$s4main5FooerC3fooyyFyyXEfU_ : $@convention(thin) (@guaranteed { var @sil_weak Optional<Fooer> }) -> () {
18+
// %0 "weakSelf" // user: %1
19+
bb0(%0 : @closureCapture @guaranteed ${ var @sil_weak Optional<Fooer> }):
20+
%1 = project_box %0 : ${ var @sil_weak Optional<Fooer> }, 0 // users: %28, %3, %2
21+
debug_value %1 : $*@sil_weak Optional<Fooer>, var, name "weakSelf", argno 1, expr op_deref // id: %2
22+
%3 = begin_access [read] [dynamic] %1 : $*@sil_weak Optional<Fooer> // users: %19, %13, %5
23+
%4 = alloc_stack $Optional<Fooer> // users: %23, %18, %16, %12, %11, %9, %6
24+
%5 = load_weak %3 : $*@sil_weak Optional<Fooer> // user: %6
25+
store %5 to [init] %4 : $*Optional<Fooer> // id: %6
26+
%7 = integer_literal $Builtin.Int1, -1 // user: %9
27+
%8 = integer_literal $Builtin.Int1, 0 // user: %9
28+
%9 = select_enum_addr %4 : $*Optional<Fooer>, case #Optional.some!enumelt: %7, default %8 : $Builtin.Int1 // user: %10
29+
cond_br %9, bb2, bb1 // id: %10
30+
31+
bb1: // Preds: bb0
32+
destroy_addr %4 : $*Optional<Fooer> // id: %11
33+
dealloc_stack %4 : $*Optional<Fooer> // id: %12
34+
end_access %3 : $*@sil_weak Optional<Fooer> // id: %13
35+
%14 = enum $Optional<()>, #Optional.none!enumelt // user: %15
36+
br bb3(%14 : $Optional<()>) // id: %15
37+
38+
bb2: // Preds: bb0
39+
%16 = unchecked_take_enum_data_addr %4 : $*Optional<Fooer>, #Optional.some!enumelt // user: %17
40+
%17 = load [copy] %16 : $*Fooer // users: %22, %21, %20
41+
destroy_addr %4 : $*Optional<Fooer> // id: %18
42+
end_access %3 : $*@sil_weak Optional<Fooer> // id: %19
43+
%20 = class_method %17 : $Fooer, #Fooer.foo1 : (Fooer) -> () -> (), $@convention(method) (@guaranteed Fooer) -> () // user: %21
44+
%21 = apply %20(%17) : $@convention(method) (@guaranteed Fooer) -> ()
45+
destroy_value %17 : $Fooer // id: %22
46+
dealloc_stack %4 : $*Optional<Fooer> // id: %23
47+
%24 = tuple () // user: %25
48+
%25 = enum $Optional<()>, #Optional.some!enumelt, %24 : $() // user: %26
49+
br bb3(%25 : $Optional<()>) // id: %26
50+
51+
bb3(%27 : $Optional<()>): // Preds: bb1 bb2
52+
%28 = begin_access [read] [dynamic] %1 : $*@sil_weak Optional<Fooer> // users: %44, %38, %30
53+
%29 = alloc_stack $Optional<Fooer> // users: %48, %43, %41, %37, %36, %34, %31
54+
%30 = load_weak %28 : $*@sil_weak Optional<Fooer> // user: %31
55+
store %30 to [init] %29 : $*Optional<Fooer> // id: %31
56+
%32 = integer_literal $Builtin.Int1, -1 // user: %34
57+
%33 = integer_literal $Builtin.Int1, 0 // user: %34
58+
%34 = select_enum_addr %29 : $*Optional<Fooer>, case #Optional.some!enumelt: %32, default %33 : $Builtin.Int1 // user: %35
59+
cond_br %34, bb5, bb4 // id: %35
60+
61+
bb4: // Preds: bb3
62+
destroy_addr %29 : $*Optional<Fooer> // id: %36
63+
dealloc_stack %29 : $*Optional<Fooer> // id: %37
64+
end_access %28 : $*@sil_weak Optional<Fooer> // id: %38
65+
%39 = enum $Optional<()>, #Optional.none!enumelt // user: %40
66+
br bb6(%39 : $Optional<()>) // id: %40
67+
68+
bb5: // Preds: bb3
69+
%41 = unchecked_take_enum_data_addr %29 : $*Optional<Fooer>, #Optional.some!enumelt // user: %42
70+
%42 = load [copy] %41 : $*Fooer // users: %47, %46, %45
71+
destroy_addr %29 : $*Optional<Fooer> // id: %43
72+
end_access %28 : $*@sil_weak Optional<Fooer> // id: %44
73+
%45 = class_method %42 : $Fooer, #Fooer.foo2 : (Fooer) -> () -> (), $@convention(method) (@guaranteed Fooer) -> () // user: %46
74+
%46 = apply %45(%42) : $@convention(method) (@guaranteed Fooer) -> ()
75+
destroy_value %42 : $Fooer // id: %47
76+
dealloc_stack %29 : $*Optional<Fooer> // id: %48
77+
%49 = tuple () // user: %50
78+
%50 = enum $Optional<()>, #Optional.some!enumelt, %49 : $() // user: %51
79+
br bb6(%50 : $Optional<()>) // id: %51
80+
81+
bb6(%52 : $Optional<()>): // Preds: bb4 bb5
82+
%53 = tuple () // user: %54
83+
return %53 : $() // id: %54
84+
} // end sil function '$s4main5FooerC3fooyyFyyXEfU_'
85+
sil @$s4main6do_fooyyyyXEF : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> ()
86+
87+
// CHECK-LABEL: sil hidden [ossa] @$s4main5FooerC3fooyyF
88+
// Fooer.foo()
89+
sil hidden [ossa] @$s4main5FooerC3fooyyF : $@convention(method) (@owned Fooer) -> () {
90+
// %0 "self" // users: %13, %5, %1
91+
bb0(%0 : @owned $Fooer):
92+
debug_value %0 : $Fooer, let, name "self", argno 1, implicit // id: %1
93+
%2 = alloc_box ${ var @sil_weak Optional<Fooer> }, var, name "weakSelf" // users: %16, %3
94+
%3 = begin_borrow [lexical] %2 : ${ var @sil_weak Optional<Fooer> } // users: %10, %15, %4
95+
%4 = project_box %3 : ${ var @sil_weak Optional<Fooer> }, 0 // user: %7
96+
%5 = copy_value %0 : $Fooer // user: %6
97+
%6 = enum $Optional<Fooer>, #Optional.some!enumelt, %5 : $Fooer // users: %8, %7
98+
store_weak %6 to [init] %4 : $*@sil_weak Optional<Fooer> // id: %7
99+
destroy_value %6 : $Optional<Fooer> // id: %8
100+
// function_ref closure #1 in Fooer.foo()
101+
%9 = function_ref @$s4main5FooerC3fooyyFyyXEfU_ : $@convention(thin) (@guaranteed { var @sil_weak Optional<Fooer> }) -> () // user: %10
102+
%10 = partial_apply [callee_guaranteed] [on_stack] %9(%3) : $@convention(thin) (@guaranteed { var @sil_weak Optional<Fooer> }) -> () // users: %12, %14
103+
// function_ref do_foo(_:)
104+
%11 = function_ref @$s4main6do_fooyyyyXEF : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> () // user: %12
105+
%12 = apply %11(%10) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> ()
106+
destroy_value %0 : $Fooer // id: %13
107+
destroy_value %10 : $@noescape @callee_guaranteed () -> () // id: %14
108+
end_borrow %3 : ${ var @sil_weak Optional<Fooer> } // id: %15
109+
destroy_value %2 : ${ var @sil_weak Optional<Fooer> } // id: %16
110+
%17 = tuple () // user: %18
111+
return %17 : $() // id: %18
112+
} // end sil function '$s4main5FooerC3fooyyF'

0 commit comments

Comments
 (0)