Skip to content

Commit 18f91c0

Browse files
committed
[region-isolation] Add support for async let.
Specifically: 1. If the value is transferred such that it becomes part of an actor region, the value is permanently part of the actor region as one would normally have. 2. If the value is just used in an async let or is used by a nonisolated async function within the async let then while the async let is alive it cannot be used. But once the async let has been awaited upon, we allow for it to be used again. rdar://117506395
1 parent 1166e22 commit 18f91c0

File tree

7 files changed

+1286
-133
lines changed

7 files changed

+1286
-133
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -877,16 +877,19 @@ NOTE(sil_referencebinding_inout_binding_here, none,
877877

878878
// Warnings arising from the flow-sensitive checking of Sendability of
879879
// non-Sendable values
880-
WARNING(arg_region_transferred, none,
880+
WARNING(regionbasedisolation_selforargtransferred, none,
881881
"call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller", ())
882-
WARNING(transfer_yields_race, none,
883-
"non-sendable value sent across isolation domains that could be concurrently accessed later in this function (%0 access site%select{|s}1 displayed%select{|, %3 more hidden}2)",
884-
(unsigned, bool, bool, unsigned))
885-
WARNING(call_site_transfer_yields_race, none,
882+
WARNING(regionbasedisolation_transfer_yields_race_no_isolation, none,
883+
"transferred value of non-Sendable type %0 that could race with later accesses",
884+
(Type))
885+
WARNING(regionbasedisolation_transfer_yields_race_with_isolation, none,
886886
"passing argument of non-sendable type %0 from %1 context to %2 context at this call site could yield a race with accesses later in this function",
887887
(Type, ActorIsolation, ActorIsolation))
888-
NOTE(possible_racy_access_site, none,
888+
NOTE(regionbasedisolation_maybe_race, none,
889889
"access here could race", ())
890+
ERROR(regionbasedisolation_unknown_pattern, none,
891+
"pattern that the region based isolation checker does not understand how to check. Please file a bug",
892+
())
890893

891894
#define UNDEFINE_DIAGNOSTIC_MACROS
892895
#include "DefineDiagnosticMacros.h"

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 65 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ enum class PartitionOpKind : uint8_t {
105105
/// Transfer the region of a value if not already transferred, takes one arg.
106106
Transfer,
107107

108+
/// Due to an async let or something like that a value that was transferred is
109+
/// no longer transferred.
110+
UndoTransfer,
111+
108112
/// Require the region of a value to be non-transferred, takes one arg.
109113
Require,
110114
};
@@ -128,20 +132,26 @@ class PartitionOp {
128132
PartitionOp(PartitionOpKind opKind, Element arg1,
129133
SILInstruction *sourceInst = nullptr)
130134
: opKind(opKind), opArgs({arg1}), source(sourceInst) {
131-
assert((opKind != PartitionOpKind::Transfer || sourceInst) &&
135+
assert(((opKind != PartitionOpKind::Transfer &&
136+
opKind != PartitionOpKind::UndoTransfer) ||
137+
sourceInst) &&
132138
"Transfer needs a sourceInst");
133139
}
134140

135141
PartitionOp(PartitionOpKind opKind, Element arg1, Operand *sourceOperand)
136142
: opKind(opKind), opArgs({arg1}), source(sourceOperand) {
137-
assert((opKind != PartitionOpKind::Transfer || sourceOperand) &&
143+
assert(((opKind != PartitionOpKind::Transfer &&
144+
opKind != PartitionOpKind::UndoTransfer) ||
145+
sourceOperand) &&
138146
"Transfer needs a sourceInst");
139147
}
140148

141149
PartitionOp(PartitionOpKind opKind, Element arg1, Element arg2,
142150
SILInstruction *sourceInst = nullptr)
143151
: opKind(opKind), opArgs({arg1, arg2}), source(sourceInst) {
144-
assert((opKind != PartitionOpKind::Transfer || sourceInst) &&
152+
assert(((opKind != PartitionOpKind::Transfer &&
153+
opKind != PartitionOpKind::UndoTransfer) ||
154+
sourceInst) &&
145155
"Transfer needs a sourceInst");
146156
}
147157

@@ -162,6 +172,11 @@ class PartitionOp {
162172
return PartitionOp(PartitionOpKind::Transfer, tgt, transferringOp);
163173
}
164174

175+
static PartitionOp UndoTransfer(Element tgt,
176+
SILInstruction *untransferringInst) {
177+
return PartitionOp(PartitionOpKind::UndoTransfer, tgt, untransferringInst);
178+
}
179+
165180
static PartitionOp Merge(Element tgt1, Element tgt2,
166181
SILInstruction *sourceInst = nullptr) {
167182
return PartitionOp(PartitionOpKind::Merge, tgt1, tgt2, sourceInst);
@@ -222,6 +237,14 @@ class PartitionOp {
222237
os << "%%" << opArgs[0];
223238
break;
224239
}
240+
case PartitionOpKind::UndoTransfer: {
241+
constexpr static char extraSpaceLiteral[10] = " ";
242+
os << "undo_transfer ";
243+
if (extraSpace)
244+
os << extraSpaceLiteral;
245+
os << "%%" << opArgs[0];
246+
break;
247+
}
225248
case PartitionOpKind::Merge: {
226249
constexpr static char extraSpaceLiteral[10] = " ";
227250
os << "merge ";
@@ -354,6 +377,33 @@ class Partition {
354377
return iter2.second;
355378
}
356379

380+
/// If val was marked as transferred, unmark it as transfer. Returns true if
381+
/// we found that \p val was transferred. We return false otherwise.
382+
bool undoTransfer(Element val) {
383+
// First see if our val is tracked. If it is not tracked, insert it.
384+
if (!isTracked(val)) {
385+
elementToRegionMap.insert_or_assign(val, fresh_label);
386+
fresh_label = Region(fresh_label + 1);
387+
canonical = false;
388+
return true;
389+
}
390+
391+
// Otherwise, we already have this value in the map. Remove it from the
392+
// transferred map.
393+
auto iter1 = elementToRegionMap.find(val);
394+
assert(iter1 != elementToRegionMap.end());
395+
return regionToTransferredOpMap.erase(iter1->second);
396+
}
397+
398+
void addElement(Element newElt) {
399+
// Map index newElt to a fresh label.
400+
elementToRegionMap.insert_or_assign(newElt, fresh_label);
401+
402+
// Increment the fresh label so it remains fresh.
403+
fresh_label = Region(fresh_label + 1);
404+
canonical = false;
405+
}
406+
357407
/// Construct the partition corresponding to the union of the two passed
358408
/// partitions.
359409
///
@@ -784,12 +834,7 @@ struct PartitionOpEvaluator {
784834
assert(op.getOpArgs().size() == 1 &&
785835
"AssignFresh PartitionOp should be passed 1 argument");
786836

787-
// map index op.getOpArgs()[0] to a fresh label
788-
p.elementToRegionMap.insert_or_assign(op.getOpArgs()[0], p.fresh_label);
789-
790-
// increment the fresh label so it remains fresh
791-
p.fresh_label = Region(p.fresh_label + 1);
792-
p.canonical = false;
837+
p.addElement(op.getOpArgs()[0]);
793838
break;
794839
case PartitionOpKind::Transfer: {
795840
assert(op.getOpArgs().size() == 1 &&
@@ -817,6 +862,7 @@ struct PartitionOpEvaluator {
817862
// actor derived, we need to treat as nontransferrable.
818863
if (isActorDerived(op.getOpArgs()[0]))
819864
return handleTransferNonTransferrable(op, op.getOpArgs()[0]);
865+
820866
Region elementRegion = p.elementToRegionMap.at(op.getOpArgs()[0]);
821867
if (llvm::any_of(p.elementToRegionMap,
822868
[&](const std::pair<Element, Region> &pair) -> bool {
@@ -830,6 +876,16 @@ struct PartitionOpEvaluator {
830876
p.markTransferred(op.getOpArgs()[0], op.getSourceOp());
831877
break;
832878
}
879+
case PartitionOpKind::UndoTransfer: {
880+
assert(op.getOpArgs().size() == 1 &&
881+
"UndoTransfer PartitionOp should be passed 1 argument");
882+
assert(p.elementToRegionMap.count(op.getOpArgs()[0]) &&
883+
"UndoTransfer PartitionOp's argument should already be tracked");
884+
885+
// Mark op.getOpArgs()[0] as not transferred.
886+
p.undoTransfer(op.getOpArgs()[0]);
887+
break;
888+
}
833889
case PartitionOpKind::Merge:
834890
assert(op.getOpArgs().size() == 2 &&
835891
"Merge PartitionOp should be passed 2 arguments");

0 commit comments

Comments
 (0)