Skip to content

Commit 43839ac

Browse files
ktosoktoso
authored
Merge pull request #76995 from nickolas-pohilets/mpokhylets/disable-stack-promotion
Disable stack promotion for classes with isolated deinit
2 parents 229ca24 + e201aa3 commit 43839ac

File tree

7 files changed

+113
-28
lines changed

7 files changed

+113
-28
lines changed

‎SwiftCompilerSources/Sources/AST/Declarations.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ final public class StructDecl: NominalTypeDecl {
5757

5858
final public class ClassDecl: NominalTypeDecl {
5959
public var superClass: Type? { Type(bridgedOrNil: bridged.Class_getSuperclass()) }
60+
61+
final public var destructor: DestructorDecl {
62+
bridged.Class_getDestructor().getAs(DestructorDecl.self)
63+
}
6064
}
6165

6266
final public class ProtocolDecl: NominalTypeDecl {}
@@ -85,7 +89,9 @@ public class AbstractFunctionDecl: ValueDecl {}
8589

8690
final public class ConstructorDecl: AbstractFunctionDecl {}
8791

88-
final public class DestructorDecl: AbstractFunctionDecl {}
92+
final public class DestructorDecl: AbstractFunctionDecl {
93+
final public var isIsolated: Bool { bridged.Destructor_isIsolated() }
94+
}
8995

9096
public class FuncDecl: AbstractFunctionDecl {}
9197

‎SwiftCompilerSources/Sources/Optimizer/FunctionPasses/StackPromotion.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
import AST
1314
import SIL
1415

1516
/// Promotes heap allocated objects to the stack.
@@ -74,6 +75,17 @@ private func tryPromoteAlloc(_ allocRef: AllocRefInstBase,
7475
return false
7576
}
7677

78+
if let dtor = (allocRef.type.nominal as? ClassDecl)?.destructor {
79+
if dtor.isIsolated {
80+
// Classes (including actors) with isolated deinit can escape implicitly.
81+
//
82+
// We could optimize this further and allow promotion if we can prove that
83+
// deinit will take fast path (i.e. it will not schedule a job).
84+
// But for now, let's keep things simple and disable promotion conservatively.
85+
return false
86+
}
87+
}
88+
7789
// The most important check: does the object escape the current function?
7890
if allocRef.isEscaping(context) {
7991
return false

‎include/swift/AST/ASTBridging.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,8 @@ struct BridgedDeclObj {
313313
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE OptionalBridgedDeclObj NominalType_getValueTypeDestructor() const;
314314
BRIDGED_INLINE bool Struct_hasUnreferenceableStorage() const;
315315
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedASTType Class_getSuperclass() const;
316+
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedDeclObj Class_getDestructor() const;
317+
BRIDGED_INLINE bool Destructor_isIsolated() const;
316318
};
317319

318320
struct BridgedASTNode {

‎include/swift/AST/ASTBridgingImpl.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,16 @@ BridgedASTType BridgedDeclObj::Class_getSuperclass() const {
142142
return {getAs<swift::ClassDecl>()->getSuperclass().getPointer()};
143143
}
144144

145+
BridgedDeclObj BridgedDeclObj::Class_getDestructor() const {
146+
return {getAs<swift::ClassDecl>()->getDestructor()};
147+
}
148+
149+
bool BridgedDeclObj::Destructor_isIsolated() const {
150+
auto dd = getAs<swift::DestructorDecl>();
151+
auto ai = swift::getActorIsolation(dd);
152+
return ai.isActorIsolated();
153+
}
154+
145155
//===----------------------------------------------------------------------===//
146156
// MARK: BridgedASTNode
147157
//===----------------------------------------------------------------------===//

‎test/Concurrency/Runtime/async_task_locals_isolated_deinit.swift

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,6 @@ class ClassNoOp: Probe {
127127

128128
let tests = TestSuite("Isolated Deinit")
129129

130-
// Dummy global variable to suppress stack propagation
131-
// TODO: Remove it after disabling allocation on stack for classes with isolated deinit
132-
var x: AnyObject? = nil
133-
func preventAllocationOnStack(_ object: AnyObject) {
134-
x = object
135-
x = nil
136-
}
137-
138130
if #available(SwiftStdlib 5.1, *) {
139131
tests.test("class sync fast path") {
140132
let group = DispatchGroup()
@@ -143,7 +135,7 @@ if #available(SwiftStdlib 5.1, *) {
143135
// FIXME: isolated deinit should be clearing task locals
144136
await TL.$number.withValue(42) {
145137
await AnotherActor.shared.performTesting {
146-
preventAllocationOnStack(ClassNoOp(expectedNumber: 0, group: group))
138+
_ = ClassNoOp(expectedNumber: 0, group: group)
147139
}
148140
}
149141
}
@@ -155,7 +147,7 @@ if #available(SwiftStdlib 5.1, *) {
155147
group.enter(1)
156148
Task {
157149
TL.$number.withValue(99) {
158-
preventAllocationOnStack(ClassNoOp(expectedNumber: 0, group: group))
150+
_ = ClassNoOp(expectedNumber: 0, group: group)
159151
}
160152
}
161153
group.wait()
@@ -169,7 +161,7 @@ if #available(SwiftStdlib 5.1, *) {
169161
TL.$number.withValue(99) {
170162
// Despite last release happening not on the actor itself,
171163
// this is still a fast path due to optimisation for deallocating actors.
172-
preventAllocationOnStack(ActorNoOp(expectedNumber: 0, group: group))
164+
_ = ActorNoOp(expectedNumber: 0, group: group)
173165
}
174166
}
175167
group.wait()
@@ -181,7 +173,7 @@ if #available(SwiftStdlib 5.1, *) {
181173
Task {
182174
TL.$number.withValue(99) {
183175
// Using ProxyActor breaks optimization
184-
preventAllocationOnStack(ProxyActor(expectedNumber: 0, group: group))
176+
_ = ProxyActor(expectedNumber: 0, group: group)
185177
}
186178
}
187179
group.wait()
@@ -191,8 +183,8 @@ if #available(SwiftStdlib 5.1, *) {
191183
let group = DispatchGroup()
192184
group.enter(2)
193185
Task {
194-
preventAllocationOnStack(ActorNoOp(expectedNumber: 0, group: group))
195-
preventAllocationOnStack(ClassNoOp(expectedNumber: 0, group: group))
186+
_ = ActorNoOp(expectedNumber: 0, group: group)
187+
_ = ClassNoOp(expectedNumber: 0, group: group)
196188
}
197189
group.wait()
198190
}

‎test/Concurrency/voucher_propagation.swift

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -267,14 +267,6 @@ func adopt(voucher: voucher_t?) {
267267
os_release(voucher_adopt(os_retain(voucher)))
268268
}
269269

270-
// Dummy global variable to suppress stack propagation
271-
// TODO: Remove it after disabling allocation on stack for classes with isolated deinit
272-
var x: AnyObject? = nil
273-
func preventAllocationOnStack(_ object: AnyObject) {
274-
x = object
275-
x = nil
276-
}
277-
278270
let tests = TestSuite("Voucher Propagation")
279271

280272
if #available(SwiftStdlib 5.1, *) {
@@ -445,15 +437,15 @@ if #available(SwiftStdlib 5.1, *) {
445437
Task {
446438
await AnotherActor.shared.performTesting {
447439
adopt(voucher: v1)
448-
preventAllocationOnStack(ClassWithIsolatedDeinit(expectedVoucher: v1, group: group))
440+
_ = ClassWithIsolatedDeinit(expectedVoucher: v1, group: group)
449441
}
450442
await AnotherActor.shared.performTesting {
451443
adopt(voucher: v2)
452-
preventAllocationOnStack(ActorWithSelfIsolatedDeinit(expectedVoucher: v2, group: group))
444+
_ = ActorWithSelfIsolatedDeinit(expectedVoucher: v2, group: group)
453445
}
454446
await AnotherActor.shared.performTesting {
455447
adopt(voucher: v3)
456-
preventAllocationOnStack(ActorWithDeinitIsolatedOnAnother(expectedVoucher: v3, group: group))
448+
_ = ActorWithDeinitIsolatedOnAnother(expectedVoucher: v3, group: group)
457449
}
458450
}
459451
group.wait()
@@ -468,11 +460,11 @@ if #available(SwiftStdlib 5.1, *) {
468460
Task {
469461
do {
470462
adopt(voucher: v1)
471-
preventAllocationOnStack(ActorWithDeinitIsolatedOnAnother(expectedVoucher: v1, group: group))
463+
_ = ActorWithDeinitIsolatedOnAnother(expectedVoucher: v1, group: group)
472464
}
473465
do {
474466
adopt(voucher: v2)
475-
preventAllocationOnStack(ClassWithIsolatedDeinit(expectedVoucher: v2, group: group))
467+
_ = ClassWithIsolatedDeinit(expectedVoucher: v2, group: group)
476468
}
477469
}
478470
group.wait()

‎test/SILOptimizer/stack_promotion_isolated_deinit.swift

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
// RUN: %target-swift-frontend -parse-as-library -O -module-name=test %s -emit-sil -enable-experimental-feature IsolatedDeinit | %FileCheck %s
2+
// REQUIRES: swift_in_compiler
3+
4+
@globalActor actor AnotherActor: GlobalActor {
5+
static let shared = AnotherActor()
6+
}
7+
8+
final class Inner {}
9+
10+
final class IsolatedDeinit {
11+
var inner: Inner?
12+
@AnotherActor deinit {}
13+
}
14+
15+
final class Container {
16+
var ref: IsolatedDeinit?
17+
}
18+
19+
// CHECK-LABEL: sil [noinline] {{.*}}@$s4test0A16ContainerOutsideyyF : $@convention(thin) () -> () {
20+
// CHECK: [[C:%.*]] = alloc_ref [bare] [stack] $Container
21+
// CHECK: [[ID:%.*]] = alloc_ref $IsolatedDeinit
22+
// CHECK: dealloc_stack_ref [[C]] : $Container
23+
// CHECK: return
24+
@inline(never)
25+
public func testContainerOutside() {
26+
// container can be promoted
27+
let container = Container()
28+
let obj = IsolatedDeinit()
29+
container.ref = obj
30+
}
31+
32+
// CHECK-LABEL: sil [noinline] @$s4test0A15ContainerInsideyyF : $@convention(thin) () -> () {
33+
// CHECK: [[D:%.*]] = alloc_ref $IsolatedDeinit
34+
// CHECK: [[C:%.*]] = alloc_ref [bare] [stack] $Container
35+
// CHECK: dealloc_stack_ref [[C]] : $Container
36+
// CHECK: return
37+
@inline(never)
38+
public func testContainerInside() {
39+
let obj = IsolatedDeinit()
40+
// container can be promoted
41+
let container = Container()
42+
container.ref = obj
43+
}
44+
45+
// CHECK-LABEL: sil [noinline] @$s4test0A12InnerOutsideyyF : $@convention(thin) () -> () {
46+
// CHECK: [[I:%.*]] = alloc_ref $Inner
47+
// CHECK: [[D:%.*]] = alloc_ref $IsolatedDeinit
48+
// CHECK: [[DI:%.*]] = end_init_let_ref [[D]] : $IsolatedDeinit
49+
// CHECK: strong_release [[DI]] : $IsolatedDeinit
50+
// CHECK: return
51+
@inline(never)
52+
public func testInnerOutside() {
53+
// inner cannot be promoted, because it escapes to isolated deinit
54+
let inner = Inner()
55+
let obj = IsolatedDeinit()
56+
obj.inner = inner
57+
}
58+
59+
// CHECK-LABEL: sil [noinline] @$s4test0A11InnerInsideyyF : $@convention(thin) () -> () {
60+
// CHECK: [[D:%.*]] = alloc_ref $IsolatedDeinit
61+
// CHECK: [[DI:%.*]] = end_init_let_ref [[D]] : $IsolatedDeinit
62+
// CHECK: [[I:%.*]] = alloc_ref $Inner
63+
// CHECK: strong_release [[DI]] : $IsolatedDeinit
64+
// CHECK: return
65+
@inline(never)
66+
public func testInnerInside() {
67+
let obj = IsolatedDeinit()
68+
// inner cannot be promoted, because it escapes to isolated deinit
69+
let inner = Inner()
70+
obj.inner = inner
71+
}

0 commit comments

Comments
 (0)