Skip to content

Commit 6844b99

Browse files
gottesmmgottesmm
committed
[region-isolation] Wire up history diagnostics through the checker, but do not use it to emit diagnostics yet.
Specifically: 1. I copy the history that we have been tracking from the transferring operand value at the transfer point. This is then available for use to emit diagnostics. 2. I added the ability for SILIsolationInfo to not only track the ActorIsolation of an actor isolated value, but also if we have a value, we can track that as well. Since we now track a value for task isolated and actor isolated SILIsolationInfo, I just renamed the field to isolatedValue and moved it out of the enum. In a subsequent commit, I am going to wire it up to a few diagnostics. rdar://123479934
1 parent d6b4f16 commit 6844b99

File tree

3 files changed

+143
-76
lines changed

3 files changed

+143
-76
lines changed

‎include/swift/SILOptimizer/Utils/PartitionUtils.h‎

Lines changed: 82 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -107,24 +107,26 @@ class SILIsolationInfo {
107107

108108
private:
109109
Kind kind;
110-
// clang-format off
111-
std::variant<
112-
// Used for actor isolated when we have ActorIsolation info from the AST.
113-
ActorIsolation,
114-
// The task isolated parameter when we find a task isolated value.
115-
SILValue
116-
> data;
117-
// clang-format on
118110

119-
SILIsolationInfo(ActorIsolation actorIsolation)
120-
: kind(Actor), data(actorIsolation) {}
111+
/// The actor isolation if this value has one. The default unspecified case
112+
/// otherwise.
113+
ActorIsolation actorIsolation;
114+
115+
/// This is the value that we got isolation from if we were able to find
116+
/// one. Used for isolation history.
117+
SILValue isolationSource;
118+
119+
SILIsolationInfo(ActorIsolation actorIsolation, SILValue isolationSource)
120+
: kind(Actor), actorIsolation(actorIsolation),
121+
isolationSource(isolationSource) {}
121122

122-
SILIsolationInfo(Kind kind, SILValue value) : kind(kind), data(value) {}
123+
SILIsolationInfo(Kind kind, SILValue isolationSource)
124+
: kind(kind), actorIsolation(), isolationSource(isolationSource) {}
123125

124-
SILIsolationInfo(Kind kind) : kind(kind), data() {}
126+
SILIsolationInfo(Kind kind) : kind(kind), actorIsolation() {}
125127

126128
public:
127-
SILIsolationInfo() : kind(Kind::Unknown), data() {}
129+
SILIsolationInfo() : kind(Kind::Unknown), actorIsolation() {}
128130

129131
operator bool() const { return kind != Kind::Unknown; }
130132

@@ -147,47 +149,50 @@ class SILIsolationInfo {
147149

148150
ActorIsolation getActorIsolation() const {
149151
assert(kind == Actor);
150-
assert(std::holds_alternative<ActorIsolation>(data) &&
151-
"Doesn't have an actor isolation?!");
152-
return std::get<ActorIsolation>(data);
152+
return actorIsolation;
153153
}
154154

155-
SILValue getTaskIsolatedValue() const {
156-
assert(kind == Task);
157-
assert(std::holds_alternative<SILValue>(data) &&
158-
"Doesn't have a task isolated value");
159-
return std::get<SILValue>(data);
155+
// If we are actor or task isolated and could find a specific value that
156+
// caused the isolation, put it here. Used for isolation history.
157+
SILValue getIsolatedValue() const {
158+
assert(kind == Task || kind == Actor);
159+
return isolationSource;
160160
}
161161

162162
bool hasActorIsolation() const { return kind == Actor; }
163163

164-
bool hasTaskIsolatedValue() const {
165-
return kind == Task && std::holds_alternative<SILValue>(data);
164+
bool hasIsolatedValue() const {
165+
return (kind == Task || kind == Actor) && bool(isolationSource);
166166
}
167167

168168
[[nodiscard]] SILIsolationInfo merge(SILIsolationInfo other) const;
169169

170-
SILIsolationInfo withActorIsolated(ActorIsolation isolation) {
171-
return SILIsolationInfo::getActorIsolated(isolation);
170+
SILIsolationInfo withActorIsolated(SILValue isolatedValue,
171+
ActorIsolation isolation) {
172+
return SILIsolationInfo::getActorIsolated(isolatedValue, isolation);
172173
}
173174

174175
static SILIsolationInfo getDisconnected() { return {Kind::Disconnected}; }
175176

176-
static SILIsolationInfo getActorIsolated(ActorIsolation actorIsolation) {
177-
return {actorIsolation};
177+
static SILIsolationInfo getActorIsolated(SILValue isolatedValue,
178+
ActorIsolation actorIsolation) {
179+
return {actorIsolation, isolatedValue};
178180
}
179181

180-
static SILIsolationInfo getActorIsolated(NominalTypeDecl *typeDecl) {
182+
static SILIsolationInfo getActorIsolated(SILValue isolatedValue,
183+
NominalTypeDecl *typeDecl) {
181184
if (typeDecl->isActor())
182-
return {ActorIsolation::forActorInstanceSelf(typeDecl)};
185+
return {ActorIsolation::forActorInstanceSelf(typeDecl), isolatedValue};
183186
auto isolation = swift::getActorIsolation(typeDecl);
184187
if (isolation.isGlobalActor())
185-
return {isolation};
188+
return {isolation, isolatedValue};
186189
return {};
187190
}
188191

189-
static SILIsolationInfo getGlobalActorIsolated(Type globalActorType) {
190-
return getActorIsolated(ActorIsolation::forGlobalActor(globalActorType));
192+
static SILIsolationInfo getGlobalActorIsolated(SILValue value,
193+
Type globalActorType) {
194+
return getActorIsolated(value,
195+
ActorIsolation::forGlobalActor(globalActorType));
191196
}
192197

193198
static SILIsolationInfo getTaskIsolated(SILValue value) {
@@ -200,7 +205,19 @@ class SILIsolationInfo {
200205
/// Attempt to infer the isolation region info for \p arg.
201206
static SILIsolationInfo get(SILFunctionArgument *arg);
202207

203-
bool operator==(const SILIsolationInfo &other) const;
208+
bool hasSameIsolation(ActorIsolation actorIsolation) const;
209+
210+
/// Returns true if \p this and \p other have the same isolation. It allows
211+
/// for the isolated values if any to not match.
212+
///
213+
/// This is useful if one has two non-Sendable values projected from the same
214+
/// actor or global actor isolated value. E.x.: two different ref_element_addr
215+
/// from the same actor.
216+
bool hasSameIsolation(const SILIsolationInfo &other) const;
217+
218+
/// Returns true if this SILIsolationInfo is deeply equal to other. This means
219+
/// that the isolation and the isolated value match.
220+
bool isEqual(const SILIsolationInfo &other) const;
204221

205222
void Profile(llvm::FoldingSetNodeID &id) const;
206223
};
@@ -444,20 +461,31 @@ class TransferringOperand {
444461
ValueType value;
445462

446463
/// The dynamic isolation info of the region of value when we transferred.
464+
///
465+
/// This will contain the isolated value if we found one.
447466
SILIsolationInfo isolationInfo;
448467

449-
TransferringOperand(ValueType newValue, SILIsolationInfo isolationRegionInfo)
450-
: value(newValue), isolationInfo(isolationRegionInfo) {
468+
/// The dynamic isolation history at this point.
469+
IsolationHistory isolationHistory;
470+
471+
TransferringOperand(ValueType newValue, SILIsolationInfo isolationRegionInfo,
472+
IsolationHistory isolationHistory)
473+
: value(newValue), isolationInfo(isolationRegionInfo),
474+
isolationHistory(isolationHistory) {
451475
assert(isolationInfo && "Should never see unknown isolation info");
452476
}
453477

454478
public:
455479
TransferringOperand(Operand *op, bool isClosureCaptured,
456-
SILIsolationInfo isolationRegionInfo)
457-
: TransferringOperand({op, isClosureCaptured}, isolationRegionInfo) {}
480+
SILIsolationInfo isolationRegionInfo,
481+
IsolationHistory isolationHistory)
482+
: TransferringOperand({op, isClosureCaptured}, isolationRegionInfo,
483+
isolationHistory) {}
458484
explicit TransferringOperand(Operand *op,
459-
SILIsolationInfo isolationRegionInfo)
460-
: TransferringOperand({op, false}, isolationRegionInfo) {}
485+
SILIsolationInfo isolationRegionInfo,
486+
IsolationHistory isolationHistory)
487+
: TransferringOperand({op, false}, isolationRegionInfo,
488+
isolationHistory) {}
461489

462490
operator bool() const { return bool(value.getPointer()); }
463491

@@ -471,6 +499,8 @@ class TransferringOperand {
471499

472500
SILIsolationInfo getIsolationInfo() const { return isolationInfo; }
473501

502+
IsolationHistory getIsolationHistory() const { return isolationHistory; }
503+
474504
unsigned getOperandNumber() const { return getOperand()->getOperandNumber(); }
475505

476506
void print(llvm::raw_ostream &os) const {
@@ -483,14 +513,17 @@ class TransferringOperand {
483513

484514
static void Profile(llvm::FoldingSetNodeID &id, Operand *op,
485515
bool isClosureCaptured,
486-
SILIsolationInfo isolationRegionInfo) {
516+
SILIsolationInfo isolationRegionInfo,
517+
IsolationHistory isolationHistory) {
487518
id.AddPointer(op);
488519
id.AddBoolean(isClosureCaptured);
489520
isolationRegionInfo.Profile(id);
521+
id.AddPointer(isolationHistory.getHead());
490522
}
491523

492524
void Profile(llvm::FoldingSetNodeID &id) const {
493-
Profile(id, getOperand(), isClosureCaptured(), isolationInfo);
525+
Profile(id, getOperand(), isClosureCaptured(), isolationInfo,
526+
isolationHistory);
494527
}
495528

496529
SWIFT_DEBUG_DUMP { print(llvm::dbgs()); }
@@ -1134,7 +1167,8 @@ struct PartitionOpEvaluator {
11341167
if (transferredRegionIsolation.isActorIsolated()) {
11351168
if (auto calleeIsolationInfo =
11361169
SILIsolationInfo::get(op.getSourceInst())) {
1137-
if (transferredRegionIsolation == calleeIsolationInfo) {
1170+
if (transferredRegionIsolation.hasSameIsolation(
1171+
calleeIsolationInfo)) {
11381172
return;
11391173
}
11401174
}
@@ -1151,7 +1185,8 @@ struct PartitionOpEvaluator {
11511185

11521186
// Mark op.getOpArgs()[0] as transferred.
11531187
auto *ptrSet = ptrSetFactory.emplace(
1154-
op.getSourceOp(), isClosureCapturedElt, transferredRegionIsolation);
1188+
op.getSourceOp(), isClosureCapturedElt, transferredRegionIsolation,
1189+
p.getIsolationHistory());
11551190
p.markTransferred(op.getOpArgs()[0], ptrSet);
11561191
return;
11571192
}
@@ -1227,7 +1262,8 @@ struct PartitionOpEvaluator {
12271262
if (shouldTryToSquelchErrors()) {
12281263
if (auto isolationInfo = SILIsolationInfo::get(op.getSourceInst())) {
12291264
if (isolationInfo.isActorIsolated() &&
1230-
isolationInfo == SILIsolationInfo::get(transferringOp->getUser()))
1265+
isolationInfo.hasSameIsolation(
1266+
SILIsolationInfo::get(transferringOp->getUser())))
12311267
return;
12321268
}
12331269

@@ -1237,8 +1273,8 @@ struct PartitionOpEvaluator {
12371273
if (auto functionIsolation =
12381274
transferringOp->getUser()->getFunction()->getActorIsolation()) {
12391275
if (functionIsolation.isActorIsolated() &&
1240-
SILIsolationInfo::getActorIsolated(functionIsolation) ==
1241-
SILIsolationInfo::get(transferringOp->getUser()))
1276+
SILIsolationInfo::get(transferringOp->getUser())
1277+
.hasSameIsolation(functionIsolation))
12421278
return;
12431279
}
12441280
}

‎lib/SILOptimizer/Analysis/RegionAnalysis.cpp‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3255,7 +3255,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
32553255
// underlying object, use that. It is never wrong.
32563256
if (info.actorIsolation) {
32573257
iter.first->getSecond().mergeIsolationRegionInfo(
3258-
SILIsolationInfo::getActorIsolated(*info.actorIsolation));
3258+
SILIsolationInfo::getActorIsolated(value, *info.actorIsolation));
32593259
}
32603260

32613261
auto storage = AccessStorageWithBase::compute(value);
@@ -3275,7 +3275,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
32753275
auto *nomDecl =
32763276
rei->getOperand()->getType().getNominalOrBoundGenericNominal();
32773277
iter.first->getSecond().mergeIsolationRegionInfo(
3278-
SILIsolationInfo::getActorIsolated(nomDecl));
3278+
SILIsolationInfo::getActorIsolated(rei, nomDecl));
32793279
}
32803280

32813281
// See if the memory base is a global_addr from a global actor protected global.
@@ -3285,7 +3285,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
32853285
auto isolation = getActorIsolation(globalDecl);
32863286
if (isolation.isGlobalActor()) {
32873287
iter.first->getSecond().mergeIsolationRegionInfo(
3288-
SILIsolationInfo::getActorIsolated(isolation));
3288+
SILIsolationInfo::getActorIsolated(ga, isolation));
32893289
}
32903290
}
32913291
}
@@ -3299,7 +3299,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
32993299
auto isolation = fri->getReferencedFunction()->getActorIsolation();
33003300
if (isolation.isActorIsolated()) {
33013301
iter.first->getSecond().mergeIsolationRegionInfo(
3302-
SILIsolationInfo::getActorIsolated(isolation));
3302+
SILIsolationInfo::getActorIsolated(value, isolation));
33033303
return {iter.first->first, iter.first->second};
33043304
}
33053305

@@ -3311,8 +3311,8 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
33113311
if (funcType->hasGlobalActor()) {
33123312
iter.first->getSecond().mergeIsolationRegionInfo(
33133313
SILIsolationInfo::getActorIsolated(
3314-
ActorIsolation::forGlobalActor(
3315-
funcType->getGlobalActor())));
3314+
fri, ActorIsolation::forGlobalActor(
3315+
funcType->getGlobalActor())));
33163316
return {iter.first->first, iter.first->second};
33173317
}
33183318
}
@@ -3322,8 +3322,8 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
33223322
if (resultFType->hasGlobalActor()) {
33233323
iter.first->getSecond().mergeIsolationRegionInfo(
33243324
SILIsolationInfo::getActorIsolated(
3325-
ActorIsolation::forGlobalActor(
3326-
resultFType->getGlobalActor())));
3325+
fri, ActorIsolation::forGlobalActor(
3326+
resultFType->getGlobalActor())));
33273327
return {iter.first->first, iter.first->second};
33283328
}
33293329
}
@@ -3341,11 +3341,13 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
33413341
if (auto isolation = getActorIsolation(declRefExpr->getDecl())) {
33423342
if (isolation.isActorIsolated()) {
33433343
iter.first->getSecond().mergeIsolationRegionInfo(
3344-
SILIsolationInfo::getActorIsolated(isolation));
3344+
SILIsolationInfo::getActorIsolated(cmi->getOperand(),
3345+
isolation));
33453346
return {iter.first->first, iter.first->second};
33463347
}
33473348
}
33483349
}
3350+
33493351
iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
33503352
return {iter.first->first, iter.first->second};
33513353
}
@@ -3370,7 +3372,8 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
33703372
auto parentAddrInfo = getUnderlyingTrackedValue(svi);
33713373
if (parentAddrInfo.actorIsolation) {
33723374
iter.first->getSecond().mergeIsolationRegionInfo(
3373-
SILIsolationInfo::getActorIsolated(*parentAddrInfo.actorIsolation));
3375+
SILIsolationInfo::getActorIsolated(svi,
3376+
*parentAddrInfo.actorIsolation));
33743377
}
33753378

33763379
auto storage = AccessStorageWithBase::compute(svi->getOperand(0));
@@ -3380,32 +3383,32 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
33803383
->getType()
33813384
.getNominalOrBoundGenericNominal();
33823385
iter.first->getSecond().mergeIsolationRegionInfo(
3383-
SILIsolationInfo::getActorIsolated(nomDecl));
3386+
SILIsolationInfo::getActorIsolated(reai->getOperand(), nomDecl));
33843387
}
33853388
}
33863389
}
33873390

33883391
// See if we have a struct_extract from a global actor isolated type.
33893392
if (auto *sei = dyn_cast<StructExtractInst>(iter.first->first.getValue())) {
33903393
iter.first->getSecond().mergeIsolationRegionInfo(
3391-
SILIsolationInfo::getActorIsolated(sei->getStructDecl()));
3394+
SILIsolationInfo::getActorIsolated(sei, sei->getStructDecl()));
33923395
}
33933396

33943397
// See if we have an unchecked_enum_data from a global actor isolated type.
33953398
if (auto *uedi =
33963399
dyn_cast<UncheckedEnumDataInst>(iter.first->first.getValue())) {
33973400
iter.first->getSecond().mergeIsolationRegionInfo(
3398-
SILIsolationInfo::getActorIsolated(uedi->getEnumDecl()));
3401+
SILIsolationInfo::getActorIsolated(uedi, uedi->getEnumDecl()));
33993402
}
34003403

34013404
// Handle a switch_enum from a global actor isolated type.
34023405
if (auto *arg = dyn_cast<SILPhiArgument>(iter.first->first.getValue())) {
34033406
if (auto *singleTerm = arg->getSingleTerminator()) {
3404-
if (auto *sei = dyn_cast<SwitchEnumInst>(singleTerm)) {
3407+
if (auto *swi = dyn_cast<SwitchEnumInst>(singleTerm)) {
34053408
auto enumDecl =
3406-
sei->getOperand()->getType().getEnumOrBoundGenericEnum();
3409+
swi->getOperand()->getType().getEnumOrBoundGenericEnum();
34073410
iter.first->getSecond().mergeIsolationRegionInfo(
3408-
SILIsolationInfo::getActorIsolated(enumDecl));
3411+
SILIsolationInfo::getActorIsolated(arg, enumDecl));
34093412
}
34103413
}
34113414
}
@@ -3418,7 +3421,8 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
34183421
auto isolation = getGlobalActorInitIsolation(calleeFunction);
34193422
if (isolation && isolation->isGlobalActor()) {
34203423
iter.first->getSecond().mergeIsolationRegionInfo(
3421-
SILIsolationInfo::getActorIsolated(*isolation));
3424+
// TODO: What to do about this.
3425+
SILIsolationInfo::getActorIsolated(SILValue(), *isolation));
34223426
}
34233427
}
34243428
}
@@ -3469,7 +3473,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
34693473
if (callType->hasGlobalActor()) {
34703474
iter.first->getSecond().mergeIsolationRegionInfo(
34713475
SILIsolationInfo::getGlobalActorIsolated(
3472-
callType->getGlobalActor()));
3476+
ai, callType->getGlobalActor()));
34733477
return {iter.first->first, iter.first->second};
34743478
}
34753479
}
@@ -3481,7 +3485,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
34813485
fri->getReferencedFunction()->getActorIsolation()) {
34823486
if (actorIsolation.isActorIsolated()) {
34833487
iter.first->getSecond().mergeIsolationRegionInfo(
3484-
SILIsolationInfo::getActorIsolated(actorIsolation));
3488+
SILIsolationInfo::getActorIsolated(fri, actorIsolation));
34853489
return {iter.first->first, iter.first->second};
34863490
}
34873491
}

0 commit comments

Comments
 (0)