Skip to content

Commit d8f39f7

Browse files
committed
[region-isolation] Begin tracking in SILIsolationInfo the actorInstance that a value is isolated to if we are dealing with an actor instance.
This will let us distinguish in between values derived from two actor instances of the same type and to emit better errors.
1 parent eed51e7 commit d8f39f7

File tree

3 files changed

+79
-38
lines changed

3 files changed

+79
-38
lines changed

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,24 @@ class SILIsolationInfo {
118118

119119
/// This is the value that we got isolation from if we were able to find
120120
/// one. Used for isolation history.
121-
SILValue isolationSource;
121+
SILValue isolatedValue;
122122

123-
SILIsolationInfo(ActorIsolation actorIsolation, SILValue isolationSource)
123+
/// If set this is the SILValue that represents the actor instance that we
124+
/// derived isolatedValue from.
125+
SILValue actorInstance;
126+
127+
SILIsolationInfo(ActorIsolation actorIsolation, SILValue isolatedValue,
128+
SILValue actorInstance)
124129
: kind(Actor), actorIsolation(actorIsolation),
125-
isolationSource(isolationSource) {}
130+
isolatedValue(isolatedValue), actorInstance(actorInstance) {
131+
assert((!actorInstance ||
132+
(actorIsolation.getKind() == ActorIsolation::ActorInstance &&
133+
actorInstance->getType().isActor())) &&
134+
"actorInstance must be an actor if it is non-empty");
135+
}
126136

127-
SILIsolationInfo(Kind kind, SILValue isolationSource)
128-
: kind(kind), actorIsolation(), isolationSource(isolationSource) {}
137+
SILIsolationInfo(Kind kind, SILValue isolatedValue)
138+
: kind(kind), actorIsolation(), isolatedValue(isolatedValue) {}
129139

130140
SILIsolationInfo(Kind kind) : kind(kind), actorIsolation() {}
131141

@@ -156,46 +166,58 @@ class SILIsolationInfo {
156166
return actorIsolation;
157167
}
158168

159-
// If we are actor or task isolated and could find a specific value that
160-
// caused the isolation, put it here. Used for isolation history.
169+
/// If we are actor or task isolated and could find a specific value that
170+
/// caused the isolation, put it here. Used for isolation history.
161171
SILValue getIsolatedValue() const {
162172
assert(kind == Task || kind == Actor);
163-
return isolationSource;
173+
return isolatedValue;
174+
}
175+
176+
/// Return the specific SILValue for the actor that our isolated value is
177+
/// isolated to if one exists.
178+
SILValue getActorInstance() const {
179+
assert(kind == Actor);
180+
return actorInstance;
164181
}
165182

166183
bool hasActorIsolation() const { return kind == Actor; }
167184

168185
bool hasIsolatedValue() const {
169-
return (kind == Task || kind == Actor) && bool(isolationSource);
186+
return (kind == Task || kind == Actor) && bool(isolatedValue);
170187
}
171188

172189
[[nodiscard]] SILIsolationInfo merge(SILIsolationInfo other) const;
173190

174191
SILIsolationInfo withActorIsolated(SILValue isolatedValue,
192+
SILValue actorInstance,
175193
ActorIsolation isolation) {
176-
return SILIsolationInfo::getActorIsolated(isolatedValue, isolation);
194+
return SILIsolationInfo::getActorIsolated(isolatedValue, actorInstance,
195+
isolation);
177196
}
178197

179198
static SILIsolationInfo getDisconnected() { return {Kind::Disconnected}; }
180199

181200
static SILIsolationInfo getActorIsolated(SILValue isolatedValue,
201+
SILValue actorInstance,
182202
ActorIsolation actorIsolation) {
183-
return {actorIsolation, isolatedValue};
203+
return {actorIsolation, isolatedValue, actorInstance};
184204
}
185205

186206
static SILIsolationInfo getActorIsolated(SILValue isolatedValue,
207+
SILValue actorInstance,
187208
NominalTypeDecl *typeDecl) {
188209
if (typeDecl->isActor())
189-
return {ActorIsolation::forActorInstanceSelf(typeDecl), isolatedValue};
210+
return {ActorIsolation::forActorInstanceSelf(typeDecl), isolatedValue,
211+
actorInstance};
190212
auto isolation = swift::getActorIsolation(typeDecl);
191213
if (isolation.isGlobalActor())
192-
return {isolation, isolatedValue};
214+
return {isolation, isolatedValue, actorInstance};
193215
return {};
194216
}
195217

196218
static SILIsolationInfo getGlobalActorIsolated(SILValue value,
197219
Type globalActorType) {
198-
return getActorIsolated(value,
220+
return getActorIsolated(value, SILValue() /*no actor instance*/,
199221
ActorIsolation::forGlobalActor(globalActorType));
200222
}
201223

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3281,8 +3281,11 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
32813281
// If we were able to find this was actor isolated from finding our
32823282
// underlying object, use that. It is never wrong.
32833283
if (info.actorIsolation) {
3284+
SILValue actorInstance =
3285+
info.value->getType().isActor() ? info.value : SILValue();
32843286
iter.first->getSecond().mergeIsolationRegionInfo(
3285-
SILIsolationInfo::getActorIsolated(value, *info.actorIsolation));
3287+
SILIsolationInfo::getActorIsolated(value, actorInstance,
3288+
*info.actorIsolation));
32863289
}
32873290

32883291
auto storage = AccessStorageWithBase::compute(value);
@@ -3332,7 +3335,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
33323335
auto parentAddrInfo = getUnderlyingTrackedValue(svi);
33333336
if (parentAddrInfo.actorIsolation) {
33343337
iter.first->getSecond().mergeIsolationRegionInfo(
3335-
SILIsolationInfo::getActorIsolated(svi,
3338+
SILIsolationInfo::getActorIsolated(svi, parentAddrInfo.value,
33363339
*parentAddrInfo.actorIsolation));
33373340
}
33383341

lib/SILOptimizer/Utils/PartitionUtils.cpp

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,16 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
7272
if (auto crossing = apply->getIsolationCrossing()) {
7373
if (crossing->getCalleeIsolation().isActorIsolated())
7474
return SILIsolationInfo::getActorIsolated(
75-
SILValue(), crossing->getCalleeIsolation());
75+
SILValue(), SILValue(), crossing->getCalleeIsolation());
7676
}
7777
}
7878

7979
if (auto fas = FullApplySite::isa(inst)) {
8080
if (auto crossing = fas.getIsolationCrossing()) {
8181
if (crossing->getCalleeIsolation().isActorIsolated()) {
82+
// SIL level, just let it through
8283
return SILIsolationInfo::getActorIsolated(
83-
SILValue(), crossing->getCalleeIsolation());
84+
SILValue(), SILValue(), crossing->getCalleeIsolation());
8485
}
8586
}
8687

@@ -90,9 +91,8 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
9091
SILParameterInfo::Isolated)) {
9192
if (auto *nomDecl =
9293
self.get()->getType().getNominalOrBoundGenericNominal()) {
93-
// TODO: We should be doing this off of the instance... what if we
94-
// have two instances of the same class?
95-
return SILIsolationInfo::getActorIsolated(SILValue(), nomDecl);
94+
return SILIsolationInfo::getActorIsolated(SILValue(), self.get(),
95+
nomDecl);
9696
}
9797
}
9898
}
@@ -102,7 +102,8 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
102102
if (auto *ace = pai->getLoc().getAsASTNode<AbstractClosureExpr>()) {
103103
auto actorIsolation = ace->getActorIsolation();
104104
if (actorIsolation.isActorIsolated()) {
105-
return SILIsolationInfo::getActorIsolated(pai, actorIsolation);
105+
return SILIsolationInfo::getActorIsolated(pai, SILValue(),
106+
actorIsolation);
106107
}
107108
}
108109
}
@@ -114,7 +115,9 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
114115
if (auto *rei = dyn_cast<RefElementAddrInst>(inst)) {
115116
auto *nomDecl =
116117
rei->getOperand()->getType().getNominalOrBoundGenericNominal();
117-
return SILIsolationInfo::getActorIsolated(rei, nomDecl);
118+
SILValue actorInstance =
119+
nomDecl->isActor() ? rei->getOperand() : SILValue();
120+
return SILIsolationInfo::getActorIsolated(rei, actorInstance, nomDecl);
118121
}
119122

120123
// Check if we have a global_addr inst.
@@ -123,7 +126,7 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
123126
if (auto *globalDecl = global->getDecl()) {
124127
auto isolation = swift::getActorIsolation(globalDecl);
125128
if (isolation.isGlobalActor()) {
126-
return SILIsolationInfo::getActorIsolated(ga, isolation);
129+
return SILIsolationInfo::getActorIsolated(ga, SILValue(), isolation);
127130
}
128131
}
129132
}
@@ -133,7 +136,7 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
133136
if (auto *fri = dyn_cast<FunctionRefInst>(inst)) {
134137
auto isolation = fri->getReferencedFunction()->getActorIsolation();
135138
if (isolation.isActorIsolated()) {
136-
return SILIsolationInfo::getActorIsolated(fri, isolation);
139+
return SILIsolationInfo::getActorIsolated(fri, SILValue(), isolation);
137140
}
138141

139142
// Otherwise, lets look at the AST and see if our function ref is from an
@@ -143,7 +146,7 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
143146
if (funcType->hasGlobalActor()) {
144147
if (funcType->hasGlobalActor()) {
145148
return SILIsolationInfo::getActorIsolated(
146-
fri,
149+
fri, SILValue(),
147150
ActorIsolation::forGlobalActor(funcType->getGlobalActor()));
148151
}
149152
}
@@ -152,7 +155,7 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
152155
funcType->getResult()->getAs<AnyFunctionType>()) {
153156
if (resultFType->hasGlobalActor()) {
154157
return SILIsolationInfo::getActorIsolated(
155-
fri,
158+
fri, SILValue(),
156159
ActorIsolation::forGlobalActor(resultFType->getGlobalActor()));
157160
}
158161
}
@@ -166,31 +169,36 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
166169
// propagate actor isolation.
167170
if (auto isolation = swift::getActorIsolation(declRefExpr->getDecl())) {
168171
if (isolation.isActorIsolated()) {
169-
return SILIsolationInfo::getActorIsolated(cmi->getOperand(),
170-
isolation);
172+
auto actor = cmi->getOperand()->getType().isActor()
173+
? cmi->getOperand()
174+
: SILValue();
175+
return SILIsolationInfo::getActorIsolated(cmi, actor, isolation);
171176
}
172177
}
173178
}
174179
}
175180

176181
// See if we have a struct_extract from a global actor isolated type.
177182
if (auto *sei = dyn_cast<StructExtractInst>(inst)) {
178-
return SILIsolationInfo::getActorIsolated(sei, sei->getStructDecl());
183+
return SILIsolationInfo::getActorIsolated(sei, SILValue(),
184+
sei->getStructDecl());
179185
}
180186

181187
// See if we have an unchecked_enum_data from a global actor isolated type.
182188
if (auto *uedi = dyn_cast<UncheckedEnumDataInst>(inst)) {
183-
return SILIsolationInfo::getActorIsolated(uedi, uedi->getEnumDecl());
189+
return SILIsolationInfo::getActorIsolated(uedi, SILValue(),
190+
uedi->getEnumDecl());
184191
}
185192

186193
// Check if we have an unsafeMutableAddressor from a global actor, mark the
187194
// returned value as being actor derived.
188-
if (auto applySite = FullApplySite::isa(inst)) {
189-
if (auto *calleeFunction = applySite.getCalleeFunction()) {
195+
if (auto applySite = dyn_cast<ApplyInst>(inst)) {
196+
if (auto *calleeFunction = applySite->getCalleeFunction()) {
190197
if (calleeFunction->isGlobalInit()) {
191198
auto isolation = getGlobalActorInitIsolation(calleeFunction);
192199
if (isolation && isolation->isGlobalActor()) {
193-
return SILIsolationInfo::getActorIsolated(SILValue(), *isolation);
200+
return SILIsolationInfo::getActorIsolated(applySite, SILValue(),
201+
*isolation);
194202
}
195203
}
196204
}
@@ -252,7 +260,7 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
252260
if (auto *swi = dyn_cast<SwitchEnumInst>(singleTerm)) {
253261
auto enumDecl =
254262
swi->getOperand()->getType().getEnumOrBoundGenericEnum();
255-
return SILIsolationInfo::getActorIsolated(arg, enumDecl);
263+
return SILIsolationInfo::getActorIsolated(arg, SILValue(), enumDecl);
256264
}
257265
}
258266
return SILIsolationInfo();
@@ -273,7 +281,7 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
273281
if (auto functionIsolation = fArg->getFunction()->getActorIsolation()) {
274282
if (functionIsolation.isActorIsolated()) {
275283
if (auto *nomDecl = self->getType().getNominalOrBoundGenericNominal()) {
276-
return SILIsolationInfo::getActorIsolated(fArg, nomDecl);
284+
return SILIsolationInfo::getActorIsolated(fArg, SILValue(), nomDecl);
277285
}
278286
}
279287
}
@@ -288,7 +296,7 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
288296
}
289297

290298
if (isolation.isActorIsolated()) {
291-
return SILIsolationInfo::getActorIsolated(fArg, isolation);
299+
return SILIsolationInfo::getActorIsolated(fArg, SILValue(), isolation);
292300
}
293301
}
294302

@@ -321,7 +329,7 @@ SILIsolationInfo SILIsolationInfo::merge(SILIsolationInfo other) const {
321329
// TODO: Make this failing mean that we emit an unknown SIL error instead of
322330
// asserting.
323331
assert((!other.isActorIsolated() || !isActorIsolated() ||
324-
hasSameIsolation(other.getActorIsolation())) &&
332+
hasSameIsolation(other)) &&
325333
"Actor can only be merged with the same actor");
326334

327335
// Otherwise, take the other value.
@@ -345,6 +353,14 @@ bool SILIsolationInfo::hasSameIsolation(const SILIsolationInfo &other) const {
345353
case Task:
346354
return getIsolatedValue() == other.getIsolatedValue();
347355
case Actor:
356+
auto actor1 = getActorInstance();
357+
auto actor2 = other.getActorInstance();
358+
359+
// If either are non-null, and the actor instance doesn't match, return
360+
// false.
361+
if ((actor1 || actor2) && actor1 != actor2)
362+
return false;
363+
348364
auto lhsIsolation = getActorIsolation();
349365
auto rhsIsolation = other.getActorIsolation();
350366
return lhsIsolation == rhsIsolation;

0 commit comments

Comments
 (0)