Skip to content

Commit c035022

Browse files
authored
Merge pull request swiftlang#38870 from ktoso/wip-dist-destructor-sil
2 parents 1cc1036 + 4480bb1 commit c035022

File tree

5 files changed

+200
-18
lines changed

5 files changed

+200
-18
lines changed

lib/SILGen/SILGenDestructor.cpp

Lines changed: 96 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,15 @@ void SILGenFunction::emitClassMemberDestruction(ManagedValue selfValue,
201201
ClassDecl *cd,
202202
CleanupLocation cleanupLoc) {
203203
assert(selfValue.getOwnershipKind() == OwnershipKind::Guaranteed);
204-
for (VarDecl *vd : cd->getStoredProperties()) {
204+
ASTContext &ctx = getASTContext();
205+
206+
auto loc = SILLocation(cd);
207+
loc.markAutoGenerated();
208+
209+
SILBasicBlock* normalMemberDestroyBB = nullptr;
210+
SILBasicBlock* returnBB = nullptr;
211+
212+
auto destroyVar = [&](VarDecl *vd) {
205213
const TypeLowering &ti = getTypeLowering(vd->getType());
206214
if (!ti.isTrivial()) {
207215
SILValue addr =
@@ -213,15 +221,96 @@ void SILGenFunction::emitClassMemberDestruction(ManagedValue selfValue,
213221
B.createDestroyAddr(cleanupLoc, addr);
214222
B.createEndAccess(cleanupLoc, addr, false /*is aborting*/);
215223
}
224+
};
225+
226+
if (cd->isDistributedActor()) {
227+
auto selfTy = cd->getDeclaredInterfaceType();
228+
229+
Scope scope(Cleanups, CleanupLocation(loc));
230+
231+
auto isLocalBB = createBasicBlock();
232+
normalMemberDestroyBB = createBasicBlock();
233+
auto remoteMemberDestroyBB = createBasicBlock();
234+
returnBB = createBasicBlock();
235+
236+
// TODO(distributed): de-duplicate with the thunk logic in SILGenDistributed
237+
// if __isRemoteActor(self) {
238+
// ...
239+
// } else {
240+
// ...
241+
// }
242+
{
243+
FuncDecl *isRemoteFn = ctx.getIsRemoteDistributedActor();
244+
assert(isRemoteFn && "Could not find 'is remote' function, is the "
245+
"'_Distributed' module available?");
246+
247+
ManagedValue selfAnyObject =
248+
B.createInitExistentialRef(loc, getLoweredType(ctx.getAnyObjectType()),
249+
CanType(selfTy), selfValue, {});
250+
auto result = emitApplyOfLibraryIntrinsic(
251+
loc, isRemoteFn, SubstitutionMap(), {selfAnyObject}, SGFContext());
252+
253+
SILValue isRemoteResult =
254+
std::move(result).forwardAsSingleValue(*this, loc);
255+
SILValue isRemoteResultUnwrapped =
256+
emitUnwrapIntegerResult(loc, isRemoteResult);
257+
258+
B.createCondBranch(loc, isRemoteResultUnwrapped, remoteMemberDestroyBB, isLocalBB);
259+
}
260+
261+
// // if __isRemoteActor(self)
262+
// {
263+
// // destroy only self.id and self.actorTransport
264+
// }
265+
{
266+
B.emitBlock(remoteMemberDestroyBB);
267+
268+
for (VarDecl *vd : cd->getStoredProperties()) {
269+
if (!vd->getAttrs().hasAttribute<DistributedActorIndependentAttr>())
270+
continue;
271+
272+
destroyVar(vd);
273+
}
274+
275+
B.createBranch(loc, returnBB);
276+
}
277+
278+
// // else (local distributed actor)
279+
// {
280+
// <continue normal deinit>
281+
// }
282+
{
283+
B.emitBlock(isLocalBB);
284+
285+
B.createBranch(loc, normalMemberDestroyBB);
286+
}
287+
}
288+
289+
{
290+
if (normalMemberDestroyBB)
291+
B.emitBlock(normalMemberDestroyBB);
292+
293+
for (VarDecl *vd : cd->getStoredProperties())
294+
destroyVar(vd);
295+
296+
if (returnBB)
297+
B.createBranch(loc, returnBB);
216298
}
217299

218-
if (cd->isRootDefaultActor()) {
219-
auto builtinName = getASTContext().getIdentifier(
220-
getBuiltinName(BuiltinValueKind::DestroyDefaultActor));
221-
auto resultTy = SGM.Types.getEmptyTupleType();
300+
{
301+
if (returnBB)
302+
B.emitBlock(returnBB);
303+
304+
305+
if (cd->isRootDefaultActor()) {
306+
// TODO(distributed): we may need to call the distributed destroy here instead?
307+
auto builtinName = getASTContext().getIdentifier(
308+
getBuiltinName(BuiltinValueKind::DestroyDefaultActor));
309+
auto resultTy = SGM.Types.getEmptyTupleType();
222310

223-
B.createBuiltin(cleanupLoc, builtinName, resultTy, /*subs*/{},
224-
{ selfValue.getValue() });
311+
B.createBuiltin(cleanupLoc, builtinName, resultTy, /*subs*/{},
312+
{ selfValue.getValue() });
313+
}
225314
}
226315
}
227316

lib/SILGen/SILGenDistributed.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ static void emitDistributedActorStore_transport(
102102
SILValue actorSelf, AbstractFunctionDecl *func,
103103
SILArgument *transportArg) {
104104
auto &B = SGF.B;
105-
auto &F = SGF.F;
106105
auto &SGM = SGF.SGM;
107106
SILGenFunctionBuilder builder(SGM);
108107

@@ -118,7 +117,6 @@ static void emitDistributedActorStore_transport(
118117
auto *var = dyn_cast<VarDecl>(vars.front());
119118

120119
// ----
121-
auto *selfTyDecl = func->getParent()->getSelfNominalTypeDecl();
122120
auto fieldAddr = B.createRefElementAddr(
123121
loc, actorSelf, var,
124122
SGF.getLoweredType(var->getInterfaceType()));
@@ -127,7 +125,7 @@ static void emitDistributedActorStore_transport(
127125
B.createCopyAddr(loc,
128126
/*src*/transportArg,
129127
/*dest*/fieldAddr,
130-
IsNotTake, IsInitialization); // TODO(distributed): should it be IsTake?
128+
IsNotTake, IsInitialization);
131129
}
132130

133131
// TODO(distributed): remove this store impl and reuse Store_transport
@@ -151,7 +149,6 @@ emitDistributedActor_init_transportStore(
151149
ManagedValue transportArgManaged = ManagedValue::forUnmanaged(transportArgValue);
152150

153151
// ----
154-
auto *selfTyDecl = ctor->getParent()->getSelfNominalTypeDecl();
155152
auto transportFieldAddr = B.createRefElementAddr(
156153
loc, borrowedSelfArg.getValue(), var,
157154
SGF.getLoweredType(var->getInterfaceType()));
@@ -197,7 +194,7 @@ static void emitDistributedActorStore_id(
197194
B.createCopyAddr(loc,
198195
/*src*/identityArg,
199196
/*dest*/fieldAddr,
200-
IsNotTake, IsInitialization); // TODO(distributed): should it be IsTake?
197+
IsNotTake, IsInitialization);
201198
}
202199

203200
/// Synthesize the distributed actor's identity (`id`) initialization:
@@ -819,7 +816,7 @@ void SILGenFunction::emitDistributedThunk(SILDeclRef thunk) {
819816
}
820817
}
821818

822-
// TODO: to use a emitAndBranch local function, since these four blocks are so similar
819+
// TODO(distributed): to use a emitAndBranch local function, since these four blocks are so similar
823820

824821
{
825822
B.emitBlock(remoteErrorBB);

test/Distributed/Runtime/distributed_actor_remote_fieldsDontCrashDeinit.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
// UNSUPPORTED: use_os_stdlib
99
// UNSUPPORTED: back_deployment_runtime
1010

11-
// REQUIRES: rdar78290608
1211

1312
import _Distributed
1413

test/SIL/Distributed/distributed_actor_default_deinit_sil.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -O -primary-file %s -emit-sil -enable-experimental-distributed | %FileCheck %s
1+
// RUN: %target-swift-frontend -O -primary-file %s -emit-sil -enable-experimental-distributed | %FileCheck %s --dump-input=fail
22

33
import _Distributed
44

@@ -38,7 +38,7 @@ distributed actor SimpleEmptyDistributedActor {
3838
// CHECK: bb3:
3939
// CHECK: destroy_addr [[ID_ADDR]] : $*AnyActorIdentity
4040
// CHECK: destroy_addr [[TRANSPORT_ADDR]] : $*ActorTransport
41-
// CHECK: %16 = builtin "destroyDefaultActor"(%0 : $SimpleEmptyDistributedActor) : $()
42-
// CHECK: %17 = unchecked_ref_cast %0 : $SimpleEmptyDistributedActor to $Builtin.NativeObject
43-
// CHECK: return %17 : $Builtin.NativeObject
41+
// CHECK: [[_:%[0-9]+]] = builtin "destroyDefaultActor"(%0 : $SimpleEmptyDistributedActor) : $()
42+
// CHECK: [[SELF:%[0-9]+]] = unchecked_ref_cast %0 : $SimpleEmptyDistributedActor to $Builtin.NativeObject
43+
// CHECK: return [[SELF]] : $Builtin.NativeObject
4444
// CHECK: } // end sil function '$s36distributed_actor_default_deinit_sil27SimpleEmptyDistributedActorCfd'
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// RUN: %target-swift-frontend -O -primary-file %s -emit-sil -enable-experimental-distributed | %FileCheck %s --dump-input=fail
2+
3+
import _Distributed
4+
5+
class SomeClass {}
6+
7+
@available(SwiftStdlib 5.5, *)
8+
actor SimpleActor {
9+
let someFieldInLocalActor: SomeClass
10+
init(field: SomeClass) {
11+
self.someFieldInLocalActor = field
12+
}
13+
}
14+
15+
16+
@available(SwiftStdlib 5.5, *)
17+
distributed actor SimpleEmptyDistributedActor {
18+
let localOnlyField: SomeClass
19+
20+
init(field: SomeClass, transport: ActorTransport) {
21+
self.localOnlyField = field
22+
}
23+
}
24+
25+
// ==== ------------------------------------------------------------------------
26+
// ==== Check that a normal local only actor is left unchanged
27+
28+
// CHECK: // SimpleActor.deinit
29+
// CHECK: sil hidden{{.*}} @$s35distributed_actor_remote_deinit_sil11SimpleActorCfd : $@convention(method) (@guaranteed SimpleActor) -> @owned Builtin.NativeObject {
30+
// CHECK: // %0 "self" // users: %6, %5, %2, %1
31+
// CHECK: bb0(%0 : $SimpleActor):
32+
// CHECK: debug_value %0 : $SimpleActor, let, name "self", argno 1, implicit // id: %1
33+
// CHECK: %2 = ref_element_addr %0 : $SimpleActor, #SimpleActor.someFieldInLocalActor // user: %3
34+
// CHECK: %3 = load %2 : $*SomeClass // user: %4
35+
// CHECK: strong_release %3 : $SomeClass // id: %4
36+
// CHECK: %5 = builtin "destroyDefaultActor"(%0 : $SimpleActor) : $()
37+
// CHECK: %6 = unchecked_ref_cast %0 : $SimpleActor to $Builtin.NativeObject // user: %7
38+
// CHECK: return %6 : $Builtin.NativeObject // id: %7
39+
// CHECK: } // end sil function '$s35distributed_actor_remote_deinit_sil11SimpleActorCfd'
40+
41+
42+
43+
44+
// ==== deinit must have the extra "if remote..." path emitted for the
45+
// distributed actor only. That path will not attempt deallocating the
46+
// `localOnly...` fields, since they were never initialized and have no storage.
47+
48+
// CHECK: // SimpleEmptyDistributedActor.deinit
49+
// sil hidden [available 12.0] @$s35distributed_actor_remote_deinit_sil27SimpleEmptyDistributedActorCfd : $@convention(method) (@guaranteed SimpleEmptyDistributedActor) -> @owned Builtin.NativeObject {
50+
// CHECK: // [[SELF:%[0-9]+]] "self"
51+
// CHECK: bb0(%0 : $SimpleEmptyDistributedActor):
52+
// CHECK: debug_value [[SELF]] : $SimpleEmptyDistributedActor, let, name "self", argno 1, implicit
53+
// CHECK: [[IDENTITY_ADDR:%[0-9]+]] = ref_element_addr %0 : $SimpleEmptyDistributedActor, #SimpleEmptyDistributedActor.id // users: %13, %24
54+
// CHECK: [[TRANSPORT_ADDR:%[0-9]+]] = ref_element_addr %0 : $SimpleEmptyDistributedActor, #SimpleEmptyDistributedActor.actorTransport
55+
// CHECK: [[SELF_1:%[0-9]+]] = init_existential_ref %0 : $SimpleEmptyDistributedActor : $SimpleEmptyDistributedActor, $AnyObject
56+
// CHECK: // function_ref swift_distributed_actor_is_remote
57+
// CHECK: [[IS_REMOTE_FN_1:%[0-9]+]] = function_ref @swift_distributed_actor_is_remote : $@convention(thin) (@guaranteed AnyObject) -> Bool
58+
// CHECK: [[IS_REMOTE_FN_RES_1:%[0-9]+]] = apply [[IS_REMOTE_FN_1]]([[SELF_1]]) : $@convention(thin) (@guaranteed AnyObject) -> Bool
59+
// CHECK: [[IS_REMOTE_BOOL_1:%[0-9]+]] = struct_extract [[IS_REMOTE_FN_RES_1]] : $Bool, #Bool._value
60+
// CHECK: cond_br [[IS_REMOTE_BOOL_1]], [[BB_CONT_1:bb[0-9]+]], [[BB_RESIGN_DIST_IDENTITY:bb[0-9]+]]
61+
62+
// CHECK: [[BB_CONT_1]]:
63+
// CHECK: br [[BB_CHECK_REMOTE_OR_LOCAL_MEMBER_DEINIT_TYPE:bb[0-9]+]]
64+
65+
// It was a local actor, so we resign the address:
66+
// CHECK: [[BB_RESIGN_DIST_IDENTITY]]:
67+
// %11 = open_existential_addr immutable_access %4 : $*ActorTransport to $*@opened({{.*}}) ActorTransport // users: %13, %13, %12
68+
// CHECK: [[RESIGN_FN:%[0-9]+]] = witness_method $@opened({{.*}}) ActorTransport, #ActorTransport.resignIdentity
69+
// CHECK: [[RESIGNED:%[0-9]+]] = apply [[RESIGN_FN]]<@opened({{.*}}) ActorTransport>(%3, %11) : $@convention(witness_method: ActorTransport) <τ_0_0 where τ_0_0 : ActorTransport> (@in_guaranteed AnyActorIdentity, @in_guaranteed τ_0_0) -> ()
70+
// CHECK: br [[BB_CHECK_REMOTE_OR_LOCAL_MEMBER_DEINIT_TYPE]]
71+
72+
// Check if we must skip destroying local storage because actor was remote
73+
// CHECK: [[BB_CHECK_REMOTE_OR_LOCAL_MEMBER_DEINIT_TYPE]]:
74+
// CHECK: %15 = init_existential_ref %0 : $SimpleEmptyDistributedActor : $SimpleEmptyDistributedActor, $AnyObject
75+
// CHECK: %16 = apply %6(%15) : $@convention(thin) (@guaranteed AnyObject) -> Bool
76+
// CHECK: %17 = struct_extract %16 : $Bool, #Bool._value // user: %18
77+
// CHECK: cond_br %17, [[BB_CONT_REMOTE_DONT_DESTROY_LOCAL_MEMBERS:bb[0-9]+]], [[BB_CONT_DESTROY_LOCAL_THEN_INDEPENDENT_MEMBERS:bb[0-9]+]]
78+
79+
// CHECK: [[BB_CONT_REMOTE_DONT_DESTROY_LOCAL_MEMBERS]]:
80+
// CHECK: br [[BB_DESTROY_INDEPENDENT_MEMBERS:bb[0-9]+]]
81+
82+
// We were a local instance after all, and thus must destroy local properties
83+
// CHECK: [[BB_CONT_DESTROY_LOCAL_THEN_INDEPENDENT_MEMBERS]]:
84+
// CHECK: [[FIELD_ADDR:%[0-9]+]] = ref_element_addr %0 : $SimpleEmptyDistributedActor, #SimpleEmptyDistributedActor.localOnlyField
85+
// CHECK: [[LOAD_FIELD_ADDR:%[0-9]+]] = load [[FIELD_ADDR]] : $*SomeClass
86+
// CHECK: strong_release [[LOAD_FIELD_ADDR]] : $SomeClass
87+
// CHECK: br [[BB_DESTROY_INDEPENDENT_MEMBERS]]
88+
89+
// Destroy "distributed nonisolated" fields and the actor itself
90+
// CHECK: [[BB_DESTROY_INDEPENDENT_MEMBERS]]:
91+
// CHECK: destroy_addr [[IDENTITY_ADDR]] : $*AnyActorIdentity
92+
// CHECK: destroy_addr [[TRANSPORT_ADDR]] : $*ActorTransport
93+
// CHECK: {{.*}} = builtin "destroyDefaultActor"(%0 : $SimpleEmptyDistributedActor) : $()
94+
// CHECK: dealloc_ref [[SELF]] : $SimpleEmptyDistributedActor
95+
// CHECK: [[EMPTY:%[0-9]+]] = tuple ()
96+
// CHECK: return [[EMPTY]] : $()
97+
// } // end sil function '$s35distributed_actor_remote_deinit_sil27SimpleEmptyDistributedActorCfD'

0 commit comments

Comments
 (0)