Skip to content

Commit 04a2dd9

Browse files
ktosoktoso
committed
[Distributed] only emit resignAddress for local actor
1 parent 3e77eac commit 04a2dd9

File tree

6 files changed

+123
-40
lines changed

6 files changed

+123
-40
lines changed

‎lib/AST/NameLookup.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1418,7 +1418,6 @@ NominalTypeDecl::lookupDirectRemoteFunc(AbstractFunctionDecl *func) {
14181418
if (remoteFuncDecls.empty())
14191419
return nullptr;
14201420

1421-
assert(remoteFuncDecls.size() > 1 && "at-most one _remote distributed function expected, found more.");
14221421
if (auto remoteDecl = dyn_cast<AbstractFunctionDecl>(remoteFuncDecls.front())) {
14231422
// TODO: implement more checks here, it has to be the exact right signature.
14241423
return remoteDecl;

‎lib/SILGen/SILGenDestructor.cpp

Lines changed: 69 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,11 @@ void SILGenFunction::emitDestroyingDestructor(DestructorDecl *dd) {
7676
resultSelfValue = selfValue;
7777
}
7878

79+
7980
if (cd->isDistributedActor()) {
80-
injectDistributedActorDestructorLifecycleCall(dd, selfValue);
81+
SILBasicBlock *continueBB = createBasicBlock();
82+
injectDistributedActorDestructorLifecycleCall(dd, selfValue, continueBB);
83+
B.emitBlock(continueBB);
8184
}
8285

8386
ArgumentScope S(*this, Loc);
@@ -106,19 +109,8 @@ void SILGenFunction::emitDestroyingDestructor(DestructorDecl *dd) {
106109
B.createReturn(returnLoc, resultSelfValue);
107110
}
108111

109-
static AccessorDecl *getActorTransportGetter(ASTContext &ctx,
110-
ProtocolDecl *distributedActorProtocol) {
111-
for (auto member: distributedActorProtocol->getAllMembers()) {
112-
if (auto var = dyn_cast<VarDecl>(member)) {
113-
if (var->getName() == ctx.Id_unownedExecutor)
114-
return var->getAccessor(AccessorKind::Get);
115-
}
116-
}
117-
return nullptr;
118-
}
119-
120112
void SILGenFunction::injectDistributedActorDestructorLifecycleCall(
121-
DestructorDecl *dd, SILValue selfValue) {
113+
DestructorDecl *dd, SILValue selfValue, SILBasicBlock *continueBB) {
122114
ASTContext &ctx = getASTContext();
123115

124116
auto cd = cast<ClassDecl>(dd->getDeclContext());
@@ -129,6 +121,10 @@ void SILGenFunction::injectDistributedActorDestructorLifecycleCall(
129121
if (dd->isImplicit())
130122
Loc.markAutoGenerated();
131123

124+
auto selfDecl = dd->getImplicitSelfDecl();
125+
auto selfManagedValue = ManagedValue::forUnmanaged(selfValue);
126+
auto selfType = selfDecl->getType();
127+
132128
// ==== locate: self.actorAddress
133129
auto addressVarDeclRefs = cd->lookupDirect(ctx.Id_actorAddress);
134130
assert(addressVarDeclRefs.size() == 1);
@@ -142,43 +138,82 @@ void SILGenFunction::injectDistributedActorDestructorLifecycleCall(
142138
auto transportVarDeclRefs = cd->lookupDirect(ctx.Id_actorTransport);
143139
assert(transportVarDeclRefs.size() == 1);
144140
auto *transportVarDeclRef = dyn_cast<VarDecl>(transportVarDeclRefs.front());
145-
assert(transportVarDeclRef);
146141
auto transportRef =
147142
B.createRefElementAddr(Loc, selfValue, transportVarDeclRef,
148143
getLoweredType(transportVarDeclRef->getType()));
149-
assert(transportRef);
150144

151145
// locate: self.transport.resignAddress(...)
152146
auto *transportDecl = ctx.getActorTransportDecl();
153147
auto resignFnDecls = transportDecl->lookupDirect(ctx.Id_resignAddress);
154148
assert(resignFnDecls.size() == 1);
155-
ValueDecl *resignFnDecl = resignFnDecls.front();
156-
assert(resignFnDecl && "Unable to find 'resignAddress' on ActorTransport");
149+
auto *resignFnDecl = resignFnDecls.front();
157150
auto resignFnRef = SILDeclRef(resignFnDecl);
158151

159-
auto openedTransport =
160-
OpenedArchetypeType::get(transportVarDeclRef->getType());
152+
// we only transport.resignAddress if we are a local actor,
153+
// and thus the address was created by transport.assignAddress.
154+
auto isRemoteBB = createBasicBlock();
155+
auto isLocalBB = createBasicBlock();
161156

162-
auto transportAddr = B.createOpenExistentialAddr(
163-
Loc, /*operand=*/transportRef, getLoweredType(openedTransport),
164-
OpenedExistentialAccess::Immutable);
157+
// if __isRemoteActor(self) {
158+
// ...
159+
// } else {
160+
// ...
161+
// }
162+
{
163+
FuncDecl *isRemoteFn = ctx.getIsRemoteDistributedActor();
164+
assert(isRemoteFn && "Could not find 'is remote' function, is the "
165+
"'_Distributed' module available?");
166+
167+
ManagedValue selfAnyObject =
168+
B.createInitExistentialRef(Loc, getLoweredType(ctx.getAnyObjectType()),
169+
CanType(selfType), selfManagedValue, {});
170+
auto result = emitApplyOfLibraryIntrinsic(
171+
Loc, isRemoteFn, SubstitutionMap(), {selfAnyObject}, SGFContext());
172+
173+
SILValue isRemoteResult =
174+
std::move(result).forwardAsSingleValue(*this, Loc);
175+
SILValue isRemoteResultUnwrapped =
176+
emitUnwrapIntegerResult(Loc, isRemoteResult);
177+
178+
B.createCondBranch(Loc, isRemoteResultUnwrapped, isRemoteBB, isLocalBB);
179+
}
165180

166-
auto resignFnType = SGM.M.Types.getConstantFunctionType(
167-
TypeExpansionContext::minimal(), resignFnRef);
181+
// if remote
182+
// === <noop>
183+
{
184+
B.emitBlock(isRemoteBB);
185+
// noop, remote actors don't do anything in their deinit
186+
B.createBranch(Loc, continueBB);
187+
}
168188

169-
auto witness = B.createWitnessMethod(
170-
Loc, openedTransport, ProtocolConformanceRef(transportDecl), resignFnRef,
171-
SILType::getPrimitiveObjectType(resignFnType));
189+
// if local
190+
// === self.transport.resignAddress(self.address)
191+
{
192+
B.emitBlock(isLocalBB);
172193

173-
auto subs = SubstitutionMap::getProtocolSubstitutions(
174-
transportDecl, openedTransport, ProtocolConformanceRef(transportDecl));
194+
auto openedTransport =
195+
OpenedArchetypeType::get(transportVarDeclRef->getType());
196+
auto transportAddr = B.createOpenExistentialAddr(
197+
Loc, /*operand=*/transportRef, getLoweredType(openedTransport),
198+
OpenedExistentialAccess::Immutable);
175199

176-
SmallVector<SILValue, 2> params;
177-
params.push_back(addressRef);
178-
params.push_back(transportAddr); // self for the call, as last param
200+
auto resignFnType = SGM.M.Types.getConstantFunctionType(
201+
TypeExpansionContext::minimal(), resignFnRef);
179202

180-
// === self.transport.resignAddress(self.address)
181-
B.createApply(Loc, witness, subs, params);
203+
auto witness = B.createWitnessMethod(
204+
Loc, openedTransport, ProtocolConformanceRef(transportDecl),
205+
resignFnRef, SILType::getPrimitiveObjectType(resignFnType));
206+
207+
auto subs = SubstitutionMap::getProtocolSubstitutions(
208+
transportDecl, openedTransport, ProtocolConformanceRef(transportDecl));
209+
210+
SmallVector<SILValue, 2> params;
211+
params.push_back(addressRef);
212+
params.push_back(transportAddr); // self for the call, as last param
213+
214+
B.createApply(Loc, witness, subs, params);
215+
B.createBranch(Loc, continueBB);
216+
}
182217
}
183218

184219
void SILGenFunction::emitDeallocatingDestructor(DestructorDecl *dd) {

‎lib/SILGen/SILGenFunction.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
620620

621621
/// Inject distributed actor and transport interaction code into the destructor.
622622
void injectDistributedActorDestructorLifecycleCall(
623-
DestructorDecl *dd, SILValue selfValue);
623+
DestructorDecl *dd, SILValue selfValue, SILBasicBlock *continueBB);
624624

625625
/// Generates code for an artificial top-level function that starts an
626626
/// application based on a main type and optionally a main type.

‎test.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ class ACTOR {
2020
}
2121

2222
deinit {
23-
self.transport.RESIGN(address: self.address);
23+
if ISREMOTE() {
24+
self.transport.RESIGN(address: self.address);
25+
}
2426
}
2527
}
28+
29+
func ISREMOTE() -> Bool {
30+
false
31+
}

‎test/Distributed/Runtime/distributed_actor_deinit.swift

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,16 @@
44
// REQUIRES: concurrency
55
// REQUIRES: distributed
66

7+
// UNSUPPORTED: use_os_stdlib
8+
// UNSUPPORTED: back_deployment_runtime
9+
10+
// REQUIRES: radar78290608
711

812
import _Distributed
913

14+
@available(SwiftStdlib 5.5, *)
15+
actor A {}
16+
1017
@available(SwiftStdlib 5.5, *)
1118
distributed actor DA {
1219
}
@@ -24,13 +31,25 @@ distributed actor DA_userDefined2 {
2431
}
2532
}
2633

34+
@available(SwiftStdlib 5.5, *)
35+
distributed actor DA_state {
36+
var name = "Hello"
37+
var age = 42
38+
39+
deinit {
40+
print("Deinitializing \(self.actorAddress)")
41+
return
42+
}
43+
}
44+
2745
// ==== Fake Transport ---------------------------------------------------------
2846

2947
@available(SwiftStdlib 5.5, *)
3048
struct FakeTransport: ActorTransport {
3149
func resolve<Act>(address: ActorAddress, as actorType: Act.Type)
3250
throws -> ActorResolved<Act> where Act: DistributedActor {
33-
fatalError()
51+
print("resolve type:\(actorType), address:\(address)")
52+
return .makeProxy
3453
}
3554

3655
func assignAddress<Act>(
@@ -59,6 +78,14 @@ struct FakeTransport: ActorTransport {
5978
@available(SwiftStdlib 5.5, *)
6079
func test() {
6180
let transport = FakeTransport()
81+
let address = ActorAddress(parse: "xxx")
82+
83+
// no lifecycle things make sense for a normal actor, double check we didn't emit them
84+
print("before A")
85+
_ = A()
86+
print("after A")
87+
// CHECK: before A
88+
// CHECK: after A
6289

6390
_ = DA(transport: transport)
6491
// CHECK: assign type:DA, address:[[ADDRESS:.*]]
@@ -76,6 +103,21 @@ func test() {
76103
// CHECK: ready actor:main.DA_userDefined2, address:[[ADDRESS]]
77104
// CHECK: Deinitializing [[ADDRESS]]
78105
// CHECK-NEXT: resign address:[[ADDRESS]]
106+
107+
// resign must happen as the _last thing_ after user-deinit completed
108+
_ = DA_state(transport: transport)
109+
// CHECK: assign type:DA_state, address:[[ADDRESS:.*]]
110+
// CHECK: ready actor:main.DA_state, address:[[ADDRESS]]
111+
// CHECK: Deinitializing [[ADDRESS]]
112+
// CHECK-NEXT: resign address:[[ADDRESS]]
113+
114+
// a remote actor should not resign it's address, it was never "assigned" it
115+
print("before")
116+
_ = try! DA_userDefined2(resolve: address, using: transport)
117+
print("done")
118+
// CHECK: before
119+
// CHECK-NEXT: Deinitializing
120+
// CHECK-NEXT: done
79121
}
80122

81123
@available(SwiftStdlib 5.5, *)

‎test/SIL/Distributed/distributed_actor_sil.swift

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

3+
// XFAIL: *
4+
// depends on distributed actors initializers not crashing the SIL verifier
5+
36
import _Distributed
47

58
@available(SwiftStdlib 5.5, *)
69
distributed actor DA_DefaultDeinit {}
710

8-
// CHECK-LABEL: sil {{.*}} @{{.*}}(@guaranteed DA_DefaultDeinit) -> @owned -> Builtin.NativeObject
911
// CHECK: %0 "self"
10-
// CHECK: [[RESIGN_REF:%.*]] = function_ref xxx

0 commit comments

Comments
 (0)