Skip to content

Commit 7ecfc82

Browse files
committed
[Distributed] correct take semantics for synthesized ID assignments
cleanup: no need to dump input always
1 parent 7af52aa commit 7ecfc82

5 files changed

+153
-16
lines changed

lib/SILGen/SILGenDistributed.cpp

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,19 @@ static SILValue emitActorPropertyReference(
5454
/// \param value the value to use when initializing the property.
5555
static void initializeProperty(SILGenFunction &SGF, SILLocation loc,
5656
SILValue actorSelf,
57-
VarDecl* prop, SILValue value) {
57+
VarDecl* prop, SILValue value,
58+
IsTake_t isTake) {
5859
Type formalType = SGF.F.mapTypeIntoContext(prop->getInterfaceType());
5960
SILType loweredType = SGF.getLoweredType(formalType);
6061

6162
auto fieldAddr = emitActorPropertyReference(SGF, loc, actorSelf, prop);
6263

6364
if (loweredType.isAddressOnly(SGF.F)) {
64-
SGF.B.createCopyAddr(loc, value, fieldAddr, IsNotTake, IsInitialization);
65+
SGF.B.createCopyAddr(loc, value, fieldAddr, isTake, IsInitialization);
6566
} else {
6667
if (value->getType().isAddress()) {
6768
SGF.emitSemanticLoadInto(loc, value, SGF.F.getTypeLowering(value->getType()),
68-
fieldAddr, SGF.getTypeLowering(loweredType), IsTake, IsInitialization);
69+
fieldAddr, SGF.getTypeLowering(loweredType), isTake, IsInitialization);
6970
} else {
7071
value = SGF.B.emitCopyValueOperation(loc, value);
7172
SGF.B.emitStoreValueOperation(
@@ -152,10 +153,10 @@ static SILArgument *findFirstDistributedActorSystemArg(SILFunction &F) {
152153
/// For the initialization of a local distributed actor instance, emits code to
153154
/// initialize the instance's stored property corresponding to the system.
154155
static void emitActorSystemInit(SILGenFunction &SGF,
155-
ConstructorDecl *ctor,
156-
SILLocation loc,
157-
ManagedValue actorSelf,
158-
SILValue systemValue) {
156+
ConstructorDecl *ctor,
157+
SILLocation loc,
158+
ManagedValue actorSelf,
159+
SILValue systemValue) {
159160
assert(ctor->isImplicit() && "unexpected explicit dist actor init");
160161
assert(ctor->isDesignatedInit());
161162

@@ -166,9 +167,8 @@ static void emitActorSystemInit(SILGenFunction &SGF,
166167
// exactly one ActorSystem-conforming argument to the constructor,
167168
// so we grab the first one from the params.
168169
VarDecl *var = classDecl->getDistributedActorSystemProperty();
169-
assert(var);
170-
171-
initializeProperty(SGF, loc, actorSelf.getValue(), var, systemValue);
170+
171+
initializeProperty(SGF, loc, actorSelf.getValue(), var, systemValue, IsNotTake);
172172
}
173173

174174
/// Emits the distributed actor's identity (`id`) initialization.
@@ -209,7 +209,7 @@ void SILGenFunction::emitDistActorIdentityInit(ConstructorDecl *ctor,
209209
{ temp, selfMetatypeValue });
210210

211211
// --- initialize the property.
212-
initializeProperty(*this, loc, borrowedSelfArg, var, temp);
212+
initializeProperty(*this, loc, borrowedSelfArg, var, temp, IsTake);
213213
}
214214

215215
// TODO(distributed): rename to DistributedActorID
@@ -443,14 +443,16 @@ void SILGenFunction::emitDistributedActorFactory(FuncDecl *fd) { // TODO(distrib
443443
loc.markAutoGenerated();
444444
auto *dc = fd->getDeclContext();
445445
auto classDecl = dc->getSelfClassDecl();
446-
446+
447447
initializeProperty(*this, loc, remote,
448448
classDecl->getDistributedActorIDProperty(),
449-
idArg);
449+
idArg,
450+
IsNotTake);
450451

451452
initializeProperty(*this, loc, remote,
452453
classDecl->getDistributedActorSystemProperty(),
453-
actorSystemArg);
454+
actorSystemArg,
455+
IsNotTake);
454456

455457
// ==== Branch to return the fully initialized remote instance
456458
B.createBranch(loc, returnBB, {remote});
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/../Inputs/FakeDistributedActorSystems.swift
3+
// RUN: %target-swift-frontend -module-name default_deinit -primary-file %s -emit-sil -verify -disable-availability-checking -I %t | %FileCheck %s --enable-var-scope --dump-input=fail
4+
// REQUIRES: concurrency
5+
// REQUIRES: distributed
6+
7+
/// The convention in this test is that the Swift declaration comes before its FileCheck lines.
8+
9+
import Distributed
10+
import FakeDistributedActorSystems
11+
12+
/// This actor system is a class, therefore SIL is slightly different
13+
typealias DefaultDistributedActorSystem = FakeRoundtripActorSystem
14+
15+
// ==== ----------------------------------------------------------------------------------------------------------------
16+
17+
class SomeClass {}
18+
19+
enum Err : Error {
20+
case blah
21+
}
22+
23+
func getSomeClass() throws -> SomeClass { throw Err.blah }
24+
func getSystem() throws -> FakeRoundtripActorSystem { throw Err.blah }
25+
26+
distributed actor MyDistActor {
27+
var someField: SomeClass
28+
29+
init() throws {
30+
do {
31+
actorSystem = try getSystem()
32+
} catch {
33+
actorSystem = FakeRoundtripActorSystem()
34+
}
35+
someField = try getSomeClass()
36+
}
37+
38+
// CHECK: sil hidden @$s14default_deinit11MyDistActorCACyKcfc : $@convention(method) (@owned MyDistActor) -> (@owned MyDistActor, @error any Error) {
39+
// CHECK: bb0([[SELF:%[0-9]+]] : $MyDistActor):
40+
// CHECK: builtin "initializeDefaultActor"([[SELF]] : $MyDistActor)
41+
// CHECK: try_apply {{%[0-9]+}}() : $@convention(thin) () -> (@owned FakeRoundtripActorSystem, @error any Error), normal [[SYSTEM_SUCCESS_BB:bb[0-9]+]], error [[SYSTEM_ERROR_BB:bb[0-9]+]]
42+
43+
// CHECK: [[SYSTEM_SUCCESS_BB]]([[SYSTEM_VAL:%[0-9]+]] : $FakeRoundtripActorSystem):
44+
// *** save system ***
45+
// CHECK: [[TP_FIELD1:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
46+
// CHECK: store [[SYSTEM_VAL]] to [[TP_FIELD1]] : $*FakeRoundtripActorSystem
47+
// *** obtain an identity ***
48+
// CHECK: [[TP_FIELD2:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
49+
// CHECK: [[RELOADED_SYS1:%[0-9]+]] = load [[TP_FIELD2]] : $*FakeRoundtripActorSystem
50+
// CHECK: [[SELF_METATYPE:%[0-9]+]] = metatype $@thick MyDistActor.Type
51+
// CHECK: strong_retain [[RELOADED_SYS1]] : $FakeRoundtripActorSystem
52+
// CHECK: [[ASSIGN_ID_FN:%[0-9]+]] = function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemC8assignIDyAA0C7AddressVxm0B00bC0RzlF
53+
// CHECK: [[ID:%[0-9]+]] = apply [[ASSIGN_ID_FN]]<MyDistActor>([[SELF_METATYPE]], [[RELOADED_SYS1]])
54+
// CHECK: strong_release [[RELOADED_SYS1]] : $FakeRoundtripActorSystem
55+
// *** save identity ***
56+
// CHECK: [[ID_FIELD:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
57+
// CHECK: store [[ID]] to [[ID_FIELD]] : $*ActorAddress
58+
// CHECK-NOT: apply
59+
// CHECK: br [[JOIN_PT:bb[0-9]+]]
60+
61+
// CHECK: [[JOIN_PT]]:
62+
// CHECK: try_apply {{.*}}() : $@convention(thin) () -> (@owned SomeClass, @error any Error), normal [[CLASS_SUCCESS_BB:bb[0-9]+]], error [[CLASS_ERROR_BB:bb[0-9]+]]
63+
64+
// CHECK: [[CLASS_SUCCESS_BB]]{{.*}}:
65+
// CHECK: store {{.*}} to {{.*}} : $*SomeClass
66+
// *** invoke actorReady ***
67+
// CHECK: [[TP_FIELD3:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
68+
// CHECK: [[RELOADED_SYS2:%[0-9]+]] = load [[TP_FIELD3]] : $*FakeRoundtripActorSystem
69+
// CHECK: [[READY_FN:%[0-9]+]] = function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemC10actorReadyyyx0B00bC0RzAA0C7AddressV2IDRtzlF
70+
// CHECK: = apply [[READY_FN]]<MyDistActor>([[SELF]], [[RELOADED_SYS2]])
71+
// CHECK: return [[SELF]] : $MyDistActor
72+
73+
// CHECK: [[SYSTEM_ERROR_BB]]([[ERROR_ARG:%[0-9]+]] : $any Error):
74+
// CHECK: strong_retain [[ERROR_ARG]] : $any Error
75+
// CHECK: function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemCACycfC
76+
// CHECK: store {{.*}} to {{.*}} : $*FakeRoundtripActorSystem
77+
// CHECK: store {{.*}} to {{.*}} : $*ActorAddress
78+
// CHECK: br [[JOIN_PT]]
79+
80+
// CHECK: [[CLASS_ERROR_BB]]{{.*}}:
81+
// ** deinit the id **
82+
// CHECK: [[REF_ID_D:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
83+
// CHECK: [[ID_D:%[0-9]+]] = begin_access [deinit] [static] [[REF_ID_D]] : $*ActorAddress
84+
// CHECK: [[REF_SYS_D:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
85+
// CHECK: [[ID:%[0-9]+]] = load [[ID_D]] : $*ActorAddress
86+
// CHECK: [[SYS:%[0-9]+]] = load [[REF_SYS_D]] : $*FakeRoundtripActorSystem
87+
// CHECK: [[RESIGN_FN:%[0-9]+]] = function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemC8resignIDyyAA0C7AddressVF
88+
// CHECK: = apply [[RESIGN_FN]]([[ID]], [[SYS]]) : $@convention(method) (@guaranteed ActorAddress, @guaranteed FakeRoundtripActorSystem) -> ()
89+
// CHECK: destroy_addr [[ID_D]] : $*ActorAddress
90+
// CHECK: end_access [[ID_D]] : $*ActorAddress
91+
// ** deinit the system **
92+
// CHECK: [[REF_SYS_D2:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
93+
// CHECK: [[SYSTEM_ACC:%[0-9]+]] = begin_access [deinit] [static] [[REF_SYS_D2]] : $*FakeRoundtripActorSystem
94+
// CHECK: destroy_addr [[SYSTEM_ACC]] : $*FakeRoundtripActorSystem
95+
// CHECK: end_access [[SYSTEM_ACC]] : $*FakeRoundtripActorSystem
96+
// CHECK: builtin "destroyDefaultActor"([[SELF]] : $MyDistActor) : $()
97+
// CHECK: dealloc_partial_ref [[SELF]]
98+
// CHECK: throw
99+
100+
101+
}
102+
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/../Inputs/FakeDistributedActorSystems.swift
3+
// RUN: %target-swift-frontend -module-name default_deinit -primary-file %s -emit-sil -verify -disable-availability-checking -I %t | %FileCheck %s --enable-var-scope --dump-input=fail
4+
// REQUIRES: concurrency
5+
// REQUIRES: distributed
6+
7+
/// The convention in this test is that the Swift declaration comes before its FileCheck lines.
8+
9+
import Distributed
10+
import FakeDistributedActorSystems
11+
12+
typealias DefaultDistributedActorSystem = FakeRoundtripActorSystem
13+
14+
// ==== ----------------------------------------------------------------------------------------------------------------
15+
16+
distributed actor MyDistActor {
17+
18+
// protocol witness for DistributedActorSystem.resolve<A>(id:as:) in conformance FakeRoundtripActorSystem
19+
// CHECK: sil hidden @$s14default_deinit11MyDistActorC7resolve2id5usingAC015FakeDistributedE7Systems0E7AddressV_AG0i9RoundtripE6SystemCtKFZ
20+
// CHECK: bb0([[ACTOR_ID_ARG:%[0-9]+]] : $ActorAddress, [[SYSTEM_ARG:%[0-9]+]] : $FakeRoundtripActorSystem, [[TYPE_ARG:%[0-9]+]] : $@thick MyDistActor.Type):
21+
// CHECK: [[SYS_RESOLVE_RESULT:%[0-9]+]] = function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemC7resolve2id2asxSgAA0C7AddressV_xmtK0B00bC0RzlF
22+
23+
// CHECK: // makeProxyBB
24+
// CHECK: [[ACTOR_INSTANCE:%[0-9]+]] = builtin "initializeDistributedRemoteActor"(%7 : $@thick MyDistActor.Type) : $MyDistActor
25+
// CHECK: [[ID_PROPERTY:%[0-9]+]] = ref_element_addr [[ACTOR_INSTANCE]] : $MyDistActor, #MyDistActor.id
26+
// CHECK: retain_value [[ACTOR_ID_ARG]] : $ActorAddress
27+
// CHECK: store [[ACTOR_ID_ARG]] to [[ID_PROPERTY]] : $*ActorAddress
28+
// CHECK: [[SYSTEM_PROPERTY:%[0-9]+]] = ref_element_addr [[ACTOR_INSTANCE]] : $MyDistActor, #MyDistActor.actorSystem
29+
// CHECK: strong_retain [[SYSTEM_ARG]] : $FakeRoundtripActorSystem
30+
// CHECK: store [[SYSTEM_ARG]] to [[SYSTEM_PROPERTY]] : $*FakeRoundtripActorSystem
31+
// CHECK: br bb5([[ACTOR_INSTANCE]] : $MyDistActor)
32+
}
33+

test/Distributed/distributed_actor_accessor_section_elf.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift
3-
// RUN: %target-swift-frontend -emit-irgen -module-name distributed_actor_accessors -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s --dump-input=always
3+
// RUN: %target-swift-frontend -emit-irgen -module-name distributed_actor_accessors -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s
44

55
// UNSUPPORTED: back_deploy_concurrency
66
// REQUIRES: concurrency

test/Distributed/distributed_actor_accessor_thunks_64bit.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift
3-
// RUN: %target-swift-frontend -module-name distributed_actor_accessors -emit-irgen -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s -check-prefix CHECK-%target-import-type --dump-input=always
3+
// RUN: %target-swift-frontend -module-name distributed_actor_accessors -emit-irgen -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s -check-prefix CHECK-%target-import-type
44

55
// UNSUPPORTED: back_deploy_concurrency
66
// REQUIRES: concurrency

0 commit comments

Comments
 (0)