Skip to content

Commit eb675c2

Browse files
ktosoktoso
authored
[Distributed] Correct tbd handling for distributed thunks (#74935)
1 parent 2f1f3ab commit eb675c2

File tree

12 files changed

+264
-60
lines changed

12 files changed

+264
-60
lines changed

‎include/swift/IRGen/Linking.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,6 +1608,7 @@ class LinkEntity {
16081608
bool isTypeMetadataAccessFunction() const {
16091609
return getKind() == Kind::TypeMetadataAccessFunction;
16101610
}
1611+
bool isDistributedThunk() const;
16111612
bool isDispatchThunk() const {
16121613
return getKind() == Kind::DispatchThunk ||
16131614
getKind() == Kind::DispatchThunkInitializer ||

‎include/swift/SIL/SILDeclRef.h

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ struct SILDeclRef {
187187
/// True if this references a foreign entry point for the referenced decl.
188188
unsigned isForeign : 1;
189189
/// True if this references a distributed function.
190-
unsigned isDistributed : 1;
190+
unsigned distributedThunk : 1;
191191
/// True if this references a distributed function, but it is known to be local
192192
unsigned isKnownToBeLocal : 1;
193193
/// True is this reference to function that could be looked up via a special
@@ -231,7 +231,7 @@ struct SILDeclRef {
231231

232232
/// Produces a null SILDeclRef.
233233
SILDeclRef()
234-
: loc(), kind(Kind::Func), isForeign(0), isDistributed(0),
234+
: loc(), kind(Kind::Func), isForeign(0), distributedThunk(0),
235235
isKnownToBeLocal(0), isRuntimeAccessible(0),
236236
backDeploymentKind(BackDeploymentKind::None), defaultArgIndex(0),
237237
isAsyncLetClosure(0) {}
@@ -406,13 +406,13 @@ struct SILDeclRef {
406406
friend llvm::hash_code hash_value(const SILDeclRef &ref) {
407407
return llvm::hash_combine(
408408
ref.loc.getOpaqueValue(), static_cast<int>(ref.kind), ref.isForeign,
409-
ref.isDistributed, ref.defaultArgIndex, ref.isAsyncLetClosure);
409+
ref.distributedThunk, ref.defaultArgIndex, ref.isAsyncLetClosure);
410410
}
411411

412412
bool operator==(SILDeclRef rhs) const {
413413
return loc.getOpaqueValue() == rhs.loc.getOpaqueValue() &&
414414
kind == rhs.kind && isForeign == rhs.isForeign &&
415-
isDistributed == rhs.isDistributed &&
415+
distributedThunk == rhs.distributedThunk &&
416416
backDeploymentKind == rhs.backDeploymentKind &&
417417
defaultArgIndex == rhs.defaultArgIndex && pointer == rhs.pointer &&
418418
isAsyncLetClosure == rhs.isAsyncLetClosure;
@@ -468,7 +468,7 @@ struct SILDeclRef {
468468

469469
/// Returns a copy of the decl with the given back deployment kind.
470470
SILDeclRef asBackDeploymentKind(BackDeploymentKind backDeploymentKind) const {
471-
return SILDeclRef(loc.getOpaqueValue(), kind, isForeign, isDistributed,
471+
return SILDeclRef(loc.getOpaqueValue(), kind, isForeign, distributedThunk,
472472
isKnownToBeLocal, isRuntimeAccessible, backDeploymentKind,
473473
defaultArgIndex, isAsyncLetClosure,
474474
pointer.get<AutoDiffDerivativeFunctionIdentifier *>());
@@ -511,6 +511,9 @@ struct SILDeclRef {
511511
/// True if the decl ref references a thunk handling potentially distributed actor functions
512512
bool isDistributedThunk() const;
513513

514+
/// True if the decl references a 'distributed' function.
515+
bool isDistributed() const;
516+
514517
/// True if the decl ref references a thunk handling a call to a function that
515518
/// supports back deployment.
516519
bool isBackDeploymentThunk() const;
@@ -605,13 +608,13 @@ struct SILDeclRef {
605608
friend struct llvm::DenseMapInfo<swift::SILDeclRef>;
606609
/// Produces a SILDeclRef from an opaque value.
607610
explicit SILDeclRef(void *opaqueLoc, Kind kind, bool isForeign,
608-
bool isDistributed, bool isKnownToBeLocal,
611+
bool isDistributedThunk, bool isKnownToBeLocal,
609612
bool isRuntimeAccessible,
610613
BackDeploymentKind backDeploymentKind,
611614
unsigned defaultArgIndex, bool isAsyncLetClosure,
612615
AutoDiffDerivativeFunctionIdentifier *derivativeId)
613616
: loc(Loc::getFromOpaqueValue(opaqueLoc)), kind(kind),
614-
isForeign(isForeign), isDistributed(isDistributed),
617+
isForeign(isForeign), distributedThunk(isDistributedThunk),
615618
isKnownToBeLocal(isKnownToBeLocal),
616619
isRuntimeAccessible(isRuntimeAccessible),
617620
backDeploymentKind(backDeploymentKind),
@@ -655,7 +658,7 @@ template<> struct DenseMapInfo<swift::SILDeclRef> {
655658
: 0;
656659
unsigned h4 = UnsignedInfo::getHashValue(Val.isForeign);
657660
unsigned h5 = PointerInfo::getHashValue(Val.pointer.getOpaqueValue());
658-
unsigned h6 = UnsignedInfo::getHashValue(Val.isDistributed);
661+
unsigned h6 = UnsignedInfo::getHashValue(Val.distributedThunk);
659662
unsigned h7 = UnsignedInfo::getHashValue(unsigned(Val.backDeploymentKind));
660663
unsigned h8 = UnsignedInfo::getHashValue(Val.isKnownToBeLocal);
661664
unsigned h9 = UnsignedInfo::getHashValue(Val.isRuntimeAccessible);

‎lib/IRGen/GenMeta.cpp

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -942,8 +942,12 @@ namespace {
942942
SILDeclRef func(entry.getFunction());
943943

944944
// Emit the dispatch thunk.
945-
if (Resilient || IGM.getOptions().WitnessMethodElimination)
945+
auto shouldEmitDispatchThunk =
946+
(Resilient || IGM.getOptions().WitnessMethodElimination) &&
947+
!func.isDistributed();
948+
if (shouldEmitDispatchThunk) {
946949
IGM.emitDispatchThunk(func);
950+
}
947951

948952
{
949953
auto *requirement = cast<AbstractFunctionDecl>(func.getDecl());
@@ -1009,10 +1013,23 @@ namespace {
10091013
}
10101014

10111015
for (auto &entry : pi.getWitnessEntries()) {
1016+
if (entry.isFunction() &&
1017+
entry.getFunction().getDecl()->isDistributedGetAccessor()) {
1018+
// We avoid emitting _distributed_get accessors, as they cannot be
1019+
// referred to anyway
1020+
continue;
1021+
}
1022+
10121023
if (Resilient) {
10131024
if (entry.isFunction()) {
10141025
// Define the method descriptor.
10151026
SILDeclRef func(entry.getFunction());
1027+
1028+
/// Distributed thunks don't need resilience.
1029+
if (func.isDistributedThunk()) {
1030+
continue;
1031+
}
1032+
10161033
auto *descriptor =
10171034
B.getAddrOfCurrentPosition(
10181035
IGM.ProtocolRequirementStructTy);
@@ -1021,13 +1038,6 @@ namespace {
10211038
}
10221039
}
10231040

1024-
if (entry.isFunction() &&
1025-
entry.getFunction().getDecl()->isDistributedGetAccessor()) {
1026-
// We avoid emitting _distributed_get accessors, as they cannot be
1027-
// referred to anyway
1028-
continue;
1029-
}
1030-
10311041
if (entry.isAssociatedType()) {
10321042
auto assocType = entry.getAssociatedType();
10331043
// Define the associated type descriptor to point to the current

‎lib/IRGen/GenProto.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2215,6 +2215,12 @@ namespace {
22152215
LinkEntity::forBaseConformanceDescriptor(requirement));
22162216
B.addRelativeAddress(baseConformanceDescriptor);
22172217
} else if (entry.getKind() == SILWitnessTable::Method) {
2218+
// distributed thunks don't need resilience
2219+
if (entry.getMethodWitness().Requirement.isDistributedThunk()) {
2220+
witnesses = witnesses.drop_back();
2221+
continue;
2222+
}
2223+
22182224
// Method descriptor.
22192225
auto declRef = entry.getMethodWitness().Requirement;
22202226
auto requirement =

‎lib/IRGen/IRSymbolVisitor.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,15 @@ class IRSymbolVisitorImpl : public SILSymbolVisitor {
103103

104104
void addDispatchThunk(SILDeclRef declRef) override {
105105
auto entity = LinkEntity::forDispatchThunk(declRef);
106+
107+
// TODO: explain why
108+
if (declRef.isDistributedThunk()) {
109+
auto afd = declRef.getAbstractFunctionDecl();
110+
if (afd && isa<ProtocolDecl>(afd->getDeclContext())) {
111+
return;
112+
}
113+
}
114+
106115
addLinkEntity(entity);
107116

108117
if (declRef.getAbstractFunctionDecl()->hasAsync())
@@ -147,6 +156,14 @@ class IRSymbolVisitorImpl : public SILSymbolVisitor {
147156
}
148157

149158
void addMethodDescriptor(SILDeclRef declRef) override {
159+
if (declRef.isDistributedThunk()) {
160+
auto afd = declRef.getAbstractFunctionDecl();
161+
auto DC = afd->getDeclContext();
162+
if (isa<ProtocolDecl>(DC)) {
163+
return;
164+
}
165+
}
166+
150167
addLinkEntity(LinkEntity::forMethodDescriptor(declRef));
151168
}
152169

‎lib/IRGen/Linking.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,18 @@ bool LinkEntity::isText() const {
13041304
}
13051305
}
13061306

1307+
bool LinkEntity::isDistributedThunk() const {
1308+
if (!hasDecl())
1309+
return false;
1310+
1311+
auto value = getDecl();
1312+
if (auto afd = dyn_cast<AbstractFunctionDecl>(value)) {
1313+
return afd->isDistributedThunk();
1314+
}
1315+
1316+
return false;
1317+
}
1318+
13071319
bool LinkEntity::isWeakImported(ModuleDecl *module) const {
13081320
switch (getKind()) {
13091321
case Kind::SILGlobalVariable:

‎lib/SIL/IR/SILDeclRef.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,11 @@ bool swift::requiresForeignEntryPoint(ValueDecl *vd) {
124124
}
125125

126126
SILDeclRef::SILDeclRef(ValueDecl *vd, SILDeclRef::Kind kind, bool isForeign,
127-
bool isDistributed, bool isKnownToBeLocal,
127+
bool isDistributedThunk, bool isKnownToBeLocal,
128128
bool isRuntimeAccessible,
129129
SILDeclRef::BackDeploymentKind backDeploymentKind,
130130
AutoDiffDerivativeFunctionIdentifier *derivativeId)
131-
: loc(vd), kind(kind), isForeign(isForeign), isDistributed(isDistributed),
131+
: loc(vd), kind(kind), isForeign(isForeign), distributedThunk(isDistributedThunk),
132132
isKnownToBeLocal(isKnownToBeLocal),
133133
isRuntimeAccessible(isRuntimeAccessible),
134134
backDeploymentKind(backDeploymentKind), defaultArgIndex(0),
@@ -186,7 +186,7 @@ SILDeclRef::SILDeclRef(SILDeclRef::Loc baseLoc, bool asForeign,
186186
}
187187

188188
isForeign = asForeign;
189-
isDistributed = asDistributed;
189+
distributedThunk = asDistributed;
190190
isKnownToBeLocal = asDistributedKnownToBeLocal;
191191
}
192192

@@ -1072,10 +1072,20 @@ bool SILDeclRef::isNativeToForeignThunk() const {
10721072
}
10731073

10741074
bool SILDeclRef::isDistributedThunk() const {
1075-
if (!isDistributed)
1075+
if (!distributedThunk)
10761076
return false;
10771077
return kind == Kind::Func;
10781078
}
1079+
bool SILDeclRef::isDistributed() const {
1080+
if (!hasFuncDecl())
1081+
return false;
1082+
1083+
if (auto decl = getFuncDecl()) {
1084+
return decl->isDistributed();
1085+
}
1086+
1087+
return false;
1088+
}
10791089

10801090
bool SILDeclRef::isBackDeploymentFallback() const {
10811091
if (backDeploymentKind != BackDeploymentKind::Fallback)

‎lib/SIL/IR/SILSymbolVisitor.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,7 @@ class SILSymbolVisitorImpl : public ASTVisitor<SILSymbolVisitorImpl> {
811811
V.Ctx.getOpts().WitnessMethodElimination} {}
812812

813813
void addMethod(SILDeclRef declRef) {
814+
// TODO: alternatively maybe prevent adding distributed thunk here rather than inside those?
814815
if (Resilient || WitnessMethodElimination) {
815816
Visitor.addDispatchThunk(declRef);
816817
Visitor.addMethodDescriptor(declRef);

‎lib/SILGen/SILGenType.cpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -401,14 +401,6 @@ template<typename T> class SILGenWitnessTable : public SILWitnessVisitor<T> {
401401

402402
public:
403403
void addMethod(SILDeclRef requirementRef) {
404-
// TODO: here the requirement is thunk_decl of the protocol; it is a FUNC
405-
// detect here that it is a func dec + thunk.
406-
// walk up to DC, and find storage.
407-
// e requirementRef->getDecl()->dump()
408-
//(func_decl implicit "distributedVariable()" interface type="<Self where Self : WorkerProtocol> (Self) -> () async throws -> String" access=internal nonisolated distributed_thunk
409-
// (parameter "self")
410-
// (parameter_list))
411-
412404
auto reqDecl = requirementRef.getDecl();
413405

414406
// Static functions can be witnessed by enum cases with payload
@@ -473,7 +465,6 @@ template<typename T> class SILGenWitnessTable : public SILWitnessVisitor<T> {
473465
// Here we notice a `distributed var` thunk requirement,
474466
// and witness it with the distributed thunk -- the "getter thunk".
475467
if (requirementRef.isDistributedThunk()) {
476-
477468
return addMethodImplementation(
478469
requirementRef, getWitnessRef(requirementRef, witnessStorage->getDistributedThunk()),
479470
witness);

‎test/Distributed/Runtime/distributed_actor_protocol_call_resilient_lib.swift

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// RUN: %empty-directory(%t/src)
2+
// RUN: split-file %s %t/src
3+
4+
/// Build the fake actor systems lib
5+
// RUN: %target-build-swift \
6+
// RUN: -Xfrontend -disable-availability-checking \
7+
// RUN: -parse-as-library -emit-library \
8+
// RUN: -emit-module-path %t/FakeDistributedActorSystems.swiftmodule \
9+
// RUN: -module-name FakeDistributedActorSystems \
10+
// RUN: %S/../Inputs/FakeDistributedActorSystems.swift \
11+
// RUN: -enable-library-evolution \
12+
// RUN: -o %t/%target-library-name(FakeDistributedActorSystems)
13+
14+
/// Build the Lib
15+
// RUN: %target-build-swift \
16+
// RUN: -Xfrontend -disable-availability-checking \
17+
// RUN: -parse-as-library -emit-library \
18+
// RUN: -emit-module-path %t/ResilientLib.swiftmodule \
19+
// RUN: -module-name ResilientLib \
20+
// RUN: -I %t \
21+
// RUN: -L %t \
22+
// RUN: %t/src/ResilientLib.swift \
23+
// RUN: -enable-library-evolution \
24+
// RUN: -o %t/%target-library-name(ResilientLib)
25+
26+
/// Build the ActorLib
27+
// RUN: %target-build-swift \
28+
// RUN: -Xfrontend -disable-availability-checking \
29+
// RUN: -parse-as-library -emit-library \
30+
// RUN: -emit-module-path %t/ResilientActorLib.swiftmodule \
31+
// RUN: -module-name ResilientActorLib \
32+
// RUN: -I %t \
33+
// RUN: -L %t \
34+
// RUN: %t/src/ResilientActorLib.swift \
35+
// RUN: -lFakeDistributedActorSystems \
36+
// RUN: -lResilientLib \
37+
// RUN: -enable-library-evolution \
38+
// RUN: -o %t/%target-library-name(ResilientActorLib)
39+
40+
/// Build the client
41+
// RUN: %target-build-swift \
42+
// RUN: -Xfrontend -disable-availability-checking \
43+
// RUN: -parse-as-library \
44+
// RUN: -lFakeDistributedActorSystems \
45+
// RUN: -lResilientLib \
46+
// RUN: -lResilientActorLib \
47+
// RUN: -module-name main \
48+
// RUN: -I %t \
49+
// RUN: -L %t \
50+
// RUN: %s \
51+
// RUN: -enable-library-evolution \
52+
// RUN: -o %t/a.out
53+
54+
// RUN: %target-codesign %t/a.out
55+
// RUN: %target-run %t/a.out | %FileCheck %s
56+
57+
// REQUIRES: executable_test
58+
// REQUIRES: concurrency
59+
// REQUIRES: distributed
60+
61+
// Locating the built libraries failed on Linux (construction of test case),
62+
// but we primarily care about macOS in this test
63+
// UNSUPPORTED: OS=linux-gnu
64+
65+
// UNSUPPORTED: use_os_stdlib
66+
// UNSUPPORTED: back_deployment_runtime
67+
68+
//--- ResilientLib.swift
69+
70+
import Distributed
71+
72+
public protocol SomeProtocol {
73+
func function() async throws -> String
74+
}
75+
76+
//--- ResilientActorLib.swift
77+
78+
import ResilientLib
79+
80+
import Distributed
81+
import FakeDistributedActorSystems
82+
83+
public distributed actor Impl: SomeProtocol {
84+
public typealias ActorSystem = FakeRoundtripActorSystem
85+
86+
public distributed func function() async throws -> String {
87+
"Success!"
88+
}
89+
}
90+
91+
//--- Main.swift
92+
93+
import ResilientLib
94+
import ResilientActorLib
95+
96+
import Distributed
97+
import FakeDistributedActorSystems
98+
99+
@main struct Main {
100+
static func main() async {
101+
let system = FakeRoundtripActorSystem()
102+
103+
print("start")
104+
105+
let impl = Impl(actorSystem: system)
106+
107+
let anyAct: any SomeProtocol = impl
108+
let anyReply = try! await anyAct.function()
109+
print("any reply = \(anyReply)") // CHECK: any reply = Success!
110+
111+
let proxy: any SomeProtocol = try! Impl.resolve(id: impl.id, using: system)
112+
let proxyReply = try! await proxy.function()
113+
print("proxy reply = \(proxyReply)") // CHECK: proxy reply = Success!
114+
115+
print("done")
116+
}
117+
}

0 commit comments

Comments
 (0)