Skip to content

Commit 2c0efe6

Browse files
committed
[region-isolation] Go through the implementation again changing Consume -> Transfer.
I just missed these.
1 parent 01b1475 commit 2c0efe6

File tree

3 files changed

+286
-267
lines changed

3 files changed

+286
-267
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -877,12 +877,12 @@ 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_consumed, none,
880+
WARNING(arg_region_transferred, 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(consumption_yields_race, none,
882+
WARNING(transfer_yields_race, none,
883883
"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)",
884884
(unsigned, bool, bool, unsigned))
885-
WARNING(call_site_consumption_yields_race, none,
885+
WARNING(call_site_transfer_yields_race, 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 (%3 access site%select{|s}4 displayed%select{|, %6 more hidden}5)",
887887
(Type, ActorIsolation, ActorIsolation, unsigned, bool, bool, unsigned))
888888
NOTE(possible_racy_access_site, none,

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 50 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ struct Region {
4949

5050
operator signed() const { return num; }
5151

52-
bool isConsumed() const { return num < 0; }
52+
bool isTransferred() const { return num < 0; }
5353

54-
static Region consumed() { return Region(-1); }
54+
static Region transferred() { return Region(-1); }
5555
};
5656
}
5757

@@ -61,20 +61,20 @@ using namespace PartitionPrimitives;
6161
/// SILInstructions can be translated to
6262
enum class PartitionOpKind : uint8_t {
6363
/// Assign one value to the region of another, takes two args, second arg
64-
/// must already be tracked with a non-consumed region
64+
/// must already be tracked with a non-transferred region
6565
Assign,
6666

6767
/// Assign one value to a fresh region, takes one arg.
6868
AssignFresh,
6969

7070
/// Merge the regions of two values, takes two args, both must be from
71-
/// non-consumed regions.
71+
/// non-transferred regions.
7272
Merge,
7373

74-
/// Consume the region of a value if not already consumed, takes one arg.
74+
/// Consume the region of a value if not already transferred, takes one arg.
7575
Transfer,
7676

77-
/// Require the region of a value to be non-consumed, takes one arg.
77+
/// Require the region of a value to be non-transferred, takes one arg.
7878
Require,
7979
};
8080

@@ -92,7 +92,8 @@ class PartitionOp {
9292
SILInstruction *sourceInst;
9393

9494
// Record an AST expression corresponding to this PartitionOp, currently
95-
// populated only for Consume expressions to indicate the value being consumed
95+
// populated only for Consume expressions to indicate the value being
96+
// transferred
9697
Expr *sourceExpr;
9798

9899
// TODO: can the following declarations be merged?
@@ -176,7 +177,7 @@ class PartitionOp {
176177
os << "assign_fresh %%" << OpArgs[0] << "\n";
177178
break;
178179
case PartitionOpKind::Transfer:
179-
os << "consume %%" << OpArgs[0] << "\n";
180+
os << "transfer %%" << OpArgs[0] << "\n";
180181
break;
181182
case PartitionOpKind::Merge:
182183
os << "merge %%" << OpArgs[0] << " with %%" << OpArgs[1] << "\n";
@@ -213,8 +214,8 @@ static void horizontalUpdate(std::map<Element, Region> &map, Element key,
213214
class Partition {
214215
private:
215216
// Label each index with a non-negative (unsigned) label if it is associated
216-
// with a valid region, and with -1 if it is associated with a consumed region
217-
// in-order traversal relied upon.
217+
// with a valid region, and with -1 if it is associated with a transferred
218+
// region in-order traversal relied upon.
218219
std::map<Element, Region> labels;
219220

220221
// Track a label that is guaranteed to be strictly larger than all in use,
@@ -240,8 +241,8 @@ class Partition {
240241
};
241242

242243
for (auto &[i, label] : labels) {
243-
// correctness vacuous at consumed indices
244-
if (label.isConsumed())
244+
// correctness vacuous at transferred indices
245+
if (label.isTransferred())
245246
continue;
246247

247248
// this label should not exceed fresh_label
@@ -266,7 +267,7 @@ class Partition {
266267

267268
// linear time - For each region label that occurs, find the first index
268269
// at which it occurs and relabel all instances of it to that index.
269-
// This excludes the -1 label for consumed regions.
270+
// This excludes the -1 label for transferred regions.
270271
void canonicalize() {
271272
if (canonical)
272273
return;
@@ -276,8 +277,8 @@ class Partition {
276277

277278
// relies on in-order traversal of labels
278279
for (auto &[i, label] : labels) {
279-
// leave -1 (consumed region) as is
280-
if (label.isConsumed())
280+
// leave -1 (transferred region) as is
281+
if (label.isTransferred())
281282
continue;
282283

283284
if (!relabel.count(label)) {
@@ -348,8 +349,8 @@ class Partition {
348349

349350
bool isTracked(Element val) const { return labels.count(val); }
350351

351-
bool isConsumed(Element val) const {
352-
return isTracked(val) && labels.at(val).isConsumed();
352+
bool isTransferred(Element val) const {
353+
return isTracked(val) && labels.at(val).isTransferred();
353354
}
354355

355356
// quadratic time - Construct the partition corresponding to the join of the
@@ -374,9 +375,10 @@ class Partition {
374375
// of indices that are in the same region in snd are also in the same region
375376
// in fst - the desired property
376377
for (const auto [i, snd_label] : snd_reduced.labels) {
377-
if (snd_label.isConsumed())
378-
// if snd says that the region has been consumed, mark it consumed in fst
379-
horizontalUpdate(fst_reduced.labels, i, Region::consumed());
378+
if (snd_label.isTransferred())
379+
// if snd says that the region has been transferred, mark it transferred
380+
// in fst
381+
horizontalUpdate(fst_reduced.labels, i, Region::transferred());
380382
else
381383
fst_reduced.merge(i, Element(snd_label));
382384
}
@@ -394,12 +396,12 @@ class Partition {
394396

395397
// Apply the passed PartitionOp to this partition, performing its action.
396398
// A `handleFailure` closure can optionally be passed in that will be called
397-
// if a consumed region is required. The closure is given the PartitionOp that
398-
// failed, and the index of the SIL value that was required but consumed.
399-
// Additionally, a list of "nonconsumable" indices can be passed in along with
400-
// a handleConsumeNonConsumable closure. In the event that a region containing
401-
// one of the nonconsumable indices is consumed, the closure will be called
402-
// with the offending Consume.
399+
// if a transferred region is required. The closure is given the PartitionOp
400+
// that failed, and the index of the SIL value that was required but
401+
// transferred. Additionally, a list of "nonconsumable" indices can be passed
402+
// in along with a handleConsumeNonConsumable closure. In the event that a
403+
// region containing one of the nonconsumable indices is transferred, the
404+
// closure will be called with the offending Consume.
403405
void apply(
404406
PartitionOp op,
405407
llvm::function_ref<void(const PartitionOp &, Element)> handleFailure =
@@ -422,7 +424,7 @@ class Partition {
422424
assert(labels.count(op.OpArgs[1]) &&
423425
"Assign PartitionOp's source argument should be already tracked");
424426
// if assigning to a missing region, handle the failure
425-
if (isConsumed(op.OpArgs[1]))
427+
if (isTransferred(op.OpArgs[1]))
426428
handleFailure(op, op.OpArgs[1]);
427429

428430
labels.insert_or_assign(op.OpArgs[0], labels.at(op.OpArgs[1]));
@@ -448,25 +450,24 @@ class Partition {
448450
assert(labels.count(op.OpArgs[0]) &&
449451
"Consume PartitionOp's argument should already be tracked");
450452

451-
// check if any nonconsumables are consumed here, and handle the failure if so
453+
// check if any nonconsumables are transferred here, and handle the
454+
// failure if so
452455
for (Element nonconsumable : nonconsumables) {
453456
assert(labels.count(nonconsumable) &&
454457
"nonconsumables should be function args and self, and therefore"
455458
"always present in the label map because of initialization at "
456459
"entry");
457-
if (!isConsumed(nonconsumable) &&
460+
if (!isTransferred(nonconsumable) &&
458461
labels.at(nonconsumable) == labels.at(op.OpArgs[0])) {
459462
handleConsumeNonConsumable(op, nonconsumable);
460463
break;
461464
}
462465
}
463466

464-
// ensure region is consumed
465-
if (!isConsumed(op.OpArgs[0]))
466-
// mark region as consumed
467-
horizontalUpdate(labels, op.OpArgs[0], Region::consumed());
468-
469-
467+
// ensure region is transferred
468+
if (!isTransferred(op.OpArgs[0]))
469+
// mark region as transferred
470+
horizontalUpdate(labels, op.OpArgs[0], Region::transferred());
470471

471472
break;
472473
case PartitionOpKind::Merge:
@@ -475,10 +476,10 @@ class Partition {
475476
assert(labels.count(op.OpArgs[0]) && labels.count(op.OpArgs[1]) &&
476477
"Merge PartitionOp's arguments should already be tracked");
477478

478-
// if attempting to merge a consumed region, handle the failure
479-
if (isConsumed(op.OpArgs[0]))
479+
// if attempting to merge a transferred region, handle the failure
480+
if (isTransferred(op.OpArgs[0]))
480481
handleFailure(op, op.OpArgs[0]);
481-
if (isConsumed(op.OpArgs[1]))
482+
if (isTransferred(op.OpArgs[1]))
482483
handleFailure(op, op.OpArgs[1]);
483484

484485
merge(op.OpArgs[0], op.OpArgs[1]);
@@ -488,26 +489,26 @@ class Partition {
488489
"Require PartitionOp should be passed 1 argument");
489490
assert(labels.count(op.OpArgs[0]) &&
490491
"Require PartitionOp's argument should already be tracked");
491-
if (isConsumed(op.OpArgs[0]))
492+
if (isTransferred(op.OpArgs[0]))
492493
handleFailure(op, op.OpArgs[0]);
493494
}
494495

495496
assert(is_canonical_correct());
496497
}
497498

498-
// return a vector of the consumed values in this partition
499-
std::vector<Element> getConsumedVals() const {
499+
// return a vector of the transferred values in this partition
500+
std::vector<Element> getTransferredVals() const {
500501
// for effeciency, this could return an iterator not a vector
501-
std::vector<Element> consumedVals;
502+
std::vector<Element> transferredVals;
502503
for (auto [i, _] : labels)
503-
if (isConsumed(i))
504-
consumedVals.push_back(i);
505-
return consumedVals;
504+
if (isTransferred(i))
505+
transferredVals.push_back(i);
506+
return transferredVals;
506507
}
507508

508-
// return a vector of the non-consumed regions in this partition, each
509+
// return a vector of the non-transferred regions in this partition, each
509510
// represented as a vector of values
510-
std::vector<std::vector<Element>> getNonConsumedRegions() const {
511+
std::vector<std::vector<Element>> getNonTransferredRegions() const {
511512
// for effeciency, this could return an iterator not a vector
512513
std::map<Region, std::vector<Element>> buckets;
513514

@@ -542,12 +543,12 @@ class Partition {
542543

543544
os << "[";
544545
for (auto [label, indices] : buckets) {
545-
os << (label.isConsumed() ? "{" : "(");
546+
os << (label.isTransferred() ? "{" : "(");
546547
int j = 0;
547548
for (Element i : indices) {
548549
os << (j++ ? " " : "") << i;
549550
}
550-
os << (label.isConsumed() ? "}" : ")");
551+
os << (label.isTransferred() ? "}" : ")");
551552
}
552553
os << "]\n";
553554
}

0 commit comments

Comments
 (0)