Skip to content

Commit 4c40c12

Browse files
ktosoktoso
committed
[Distributed] Correct tbd handling for distributed thunks
This avoids duplicate definitions and crashes when distributed thunks are used with protocols, and library evolution mode. Still problems remain with pedantic tbd validation. Resolves: edar://128310903 Still needs work for: rdar://128284016 TAPI validation Still needs work to address the new test, which fails now: <unknown>:0: error: IR generation failure: program too clever: function collides with existing symbol $s4test15GreeterProtocolP5hello4nameS2S_tYaKFTj
1 parent 517667e commit 4c40c12

File tree

9 files changed

+129
-22
lines changed

9 files changed

+129
-22
lines changed

‎include/swift/IRGen/Linking.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1448,7 +1448,17 @@ class LinkEntity {
14481448

14491449
const ValueDecl *getDecl() const {
14501450
assert(isDeclKind(getKind()));
1451-
return reinterpret_cast<ValueDecl*>(Pointer);
1451+
auto decl = reinterpret_cast<ValueDecl*>(Pointer);
1452+
1453+
// FIXME: determine if this matters after all or not
1454+
// if it is a distributed thunk, find the thunk decl and mangled based on it
1455+
if (auto afd = dyn_cast<AbstractFunctionDecl>(decl)) {
1456+
if (afd->isDistributed()) {
1457+
decl = afd->getDistributedThunk();
1458+
}
1459+
}
1460+
1461+
return decl;
14521462
}
14531463

14541464
const ExtensionDecl *getExtension() const {
@@ -1608,6 +1618,7 @@ class LinkEntity {
16081618
bool isTypeMetadataAccessFunction() const {
16091619
return getKind() == Kind::TypeMetadataAccessFunction;
16101620
}
1621+
bool isDistributedThunk() const;
16111622
bool isDispatchThunk() const {
16121623
return getKind() == Kind::DispatchThunk ||
16131624
getKind() == Kind::DispatchThunkInitializer ||

‎include/swift/SIL/SILDeclRef.h

Lines changed: 8 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 *>());
@@ -605,13 +605,13 @@ struct SILDeclRef {
605605
friend struct llvm::DenseMapInfo<swift::SILDeclRef>;
606606
/// Produces a SILDeclRef from an opaque value.
607607
explicit SILDeclRef(void *opaqueLoc, Kind kind, bool isForeign,
608-
bool isDistributed, bool isKnownToBeLocal,
608+
bool isDistributedThunk, bool isKnownToBeLocal,
609609
bool isRuntimeAccessible,
610610
BackDeploymentKind backDeploymentKind,
611611
unsigned defaultArgIndex, bool isAsyncLetClosure,
612612
AutoDiffDerivativeFunctionIdentifier *derivativeId)
613613
: loc(Loc::getFromOpaqueValue(opaqueLoc)), kind(kind),
614-
isForeign(isForeign), isDistributed(isDistributed),
614+
isForeign(isForeign), distributedThunk(isDistributedThunk),
615615
isKnownToBeLocal(isKnownToBeLocal),
616616
isRuntimeAccessible(isRuntimeAccessible),
617617
backDeploymentKind(backDeploymentKind),
@@ -655,7 +655,7 @@ template<> struct DenseMapInfo<swift::SILDeclRef> {
655655
: 0;
656656
unsigned h4 = UnsignedInfo::getHashValue(Val.isForeign);
657657
unsigned h5 = PointerInfo::getHashValue(Val.pointer.getOpaqueValue());
658-
unsigned h6 = UnsignedInfo::getHashValue(Val.isDistributed);
658+
unsigned h6 = UnsignedInfo::getHashValue(Val.distributedThunk);
659659
unsigned h7 = UnsignedInfo::getHashValue(unsigned(Val.backDeploymentKind));
660660
unsigned h8 = UnsignedInfo::getHashValue(Val.isKnownToBeLocal);
661661
unsigned h9 = UnsignedInfo::getHashValue(Val.isRuntimeAccessible);

‎lib/IRGen/GenProto.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2213,6 +2213,12 @@ namespace {
22132213
LinkEntity::forBaseConformanceDescriptor(requirement));
22142214
B.addRelativeAddress(baseConformanceDescriptor);
22152215
} else if (entry.getKind() == SILWitnessTable::Method) {
2216+
// distributed thunks don't need resilience
2217+
if (entry.getMethodWitness().Requirement.isDistributedThunk()) {
2218+
witnesses = witnesses.drop_back();
2219+
continue;
2220+
}
2221+
22162222
// Method descriptor.
22172223
auto declRef = entry.getMethodWitness().Requirement;
22182224
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: 4 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

@@ -1062,7 +1062,7 @@ bool SILDeclRef::isNativeToForeignThunk() const {
10621062
}
10631063

10641064
bool SILDeclRef::isDistributedThunk() const {
1065-
if (!isDistributed)
1065+
if (!distributedThunk)
10661066
return false;
10671067
return kind == Kind::Func;
10681068
}

‎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/TBD/distributed_library_evolution.swift

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// REQUIRES: VENDOR=apple
2+
// REQUIRES: concurrency
3+
// REQUIRES: distributed
4+
5+
// RUN: %empty-directory(%t)
6+
<<<<<<< Updated upstream
7+
// RUN: split-file %s %t
8+
9+
// RUN: %target-swift-frontend %t/library.swift \
10+
// RUN: -enable-library-evolution \
11+
// RUN: -disable-availability-checking \
12+
// RUN: -emit-ir -o %t/test.ll -emit-tbd \
13+
// RUN: -emit-tbd-path %t/library.tbd -I %t -tbd-install_name protocol
14+
15+
// RUN: %target-swift-frontend %t/library.swift \
16+
// RUN: -enable-library-evolution \
17+
// RUN: -disable-availability-checking \
18+
// RUN: -emit-module \
19+
// RUN: -package-name Package \
20+
// RUN: -enable-library-evolution \
21+
// RUN: -module-name Library \
22+
// RUN: -emit-module-path %t/Library.swiftmodule \
23+
// RUN: -emit-module-interface-path %t/Library.swiftinterface
24+
25+
// RUN: %target-swift-frontend %t/actor.swift -enable-library-evolution \
26+
// RUN: -disable-availability-checking -emit-ir -o %t/test.ll -emit-tbd \
27+
// RUN: -emit-tbd-path %t/actor.tbd -I %t -tbd-install_name actor
28+
29+
// RUN: %target-swift-frontend %t/actor.swift \
30+
// RUN: -I %t \
31+
// RUN: -disable-availability-checking \
32+
// RUN: -emit-module \
33+
// RUN: -package-name Package \
34+
// RUN: -enable-library-evolution \
35+
// RUN: -module-name Client \
36+
// RUN: -emit-module-path %t/Client.swiftmodule \
37+
// RUN: -emit-module-interface-path %t/Client.swiftinterface
38+
39+
40+
// RUN %llvm-nm -g %t/library.tbd | %FileCheck %s --dump-input=always
41+
// RUN %llvm-nm -g %t/actor.tbd | %FileCheck %s --dump-input=always
42+
43+
//--- library.swift
44+
import Distributed
45+
46+
// CHECK: @"$s4test1AC13_remote_helloyyYaKFTE" = hidden global %swift.async_func_pointer
47+
// CHECK: @"$s4test1AC13_remote_helloyyYaKFTETu" = hidden global %swift.async_func_pointer
48+
public protocol GreeterProtocol: DistributedActor where ActorSystem == LocalTestingDistributedActorSystem {
49+
distributed func hello(name: String) -> String
50+
}
51+
52+
//--- actor.swift
53+
import Distributed
54+
import Library
55+
56+
public distributed actor SomeDistributedActor: GreeterProtocol {
57+
public distributed func hello(name: String) -> String {
58+
"Hello, \(name)!"
59+
}
60+
}
61+
62+
// function:
63+
// IR unmangledName = $s4test20SomeDistributedActorC5hello4nameS2S_tF
64+
// function method descriptor
65+
// IR unmangledName = $s4test20SomeDistributedActorC5hello4nameS2S_tFTq
66+
// thunk, method reference
67+
// IR unmangledName = $s4test20SomeDistributedActorC5hello4nameS2S_tFTE
68+
// thunk, method reference + async function pointer
69+
// IR unmangledName = $s4test20SomeDistributedActorC5hello4nameS2S_tFTETu

0 commit comments

Comments
 (0)