Skip to content

Commit 391ad21

Browse files
gottesmmgottesmm
committed
[rbi] Make all PartitionOpErrors noncopyable types.
I am going to be adding support to passes for emitting IsolationHistory behind a flag. As part of this, we need to store the state of the partition that created the error when the error is emitted. A partition stores heap memory so it makes sense to make these types noncopyable types so we just move the heap memory rather than copying it all over the place.
1 parent 6d20c74 commit 391ad21

File tree

3 files changed

+84
-37
lines changed

3 files changed

+84
-37
lines changed

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

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,6 +1015,9 @@ class Partition {
10151015

10161016
/// Swift style enum we use to decouple and reduce boilerplate in between the
10171017
/// diagnostic and non-diagnostic part of the infrastructure.
1018+
///
1019+
/// NOTE: This is a noncopyable type so that we can store Partitions in these
1020+
/// errors without copying them all over the place.
10181021
class PartitionOpError {
10191022
public:
10201023
using Element = PartitionPrimitives::Element;
@@ -1029,6 +1032,9 @@ class PartitionOpError {
10291032
const PartitionOp *op;
10301033

10311034
UnknownCodePatternError(const PartitionOp &op) : op(&op) {}
1035+
UnknownCodePatternError(UnknownCodePatternError &&other) = default;
1036+
UnknownCodePatternError &
1037+
operator=(UnknownCodePatternError &&other) = default;
10321038

10331039
void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const;
10341040

@@ -1046,6 +1052,9 @@ class PartitionOpError {
10461052
Operand *sendingOp)
10471053
: op(&op), sentElement(elt), sendingOp(sendingOp) {}
10481054

1055+
LocalUseAfterSendError(LocalUseAfterSendError &&other) = default;
1056+
LocalUseAfterSendError &operator=(LocalUseAfterSendError &&other) = default;
1057+
10491058
void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const;
10501059

10511060
SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) {
@@ -1063,6 +1072,9 @@ class PartitionOpError {
10631072
: op(&op), sentElement(sentElement),
10641073
isolationRegionInfo(isolationRegionInfo) {}
10651074

1075+
SentNeverSendableError(SentNeverSendableError &&other) = default;
1076+
SentNeverSendableError &operator=(SentNeverSendableError &&other) = default;
1077+
10661078
void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const;
10671079

10681080
SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) {
@@ -1086,6 +1098,11 @@ class PartitionOpError {
10861098
srcElement(srcElement), srcValue(srcValue),
10871099
srcIsolationRegionInfo(srcIsolationRegionInfo) {}
10881100

1101+
AssignNeverSendableIntoSendingResultError(
1102+
AssignNeverSendableIntoSendingResultError &&other) = default;
1103+
AssignNeverSendableIntoSendingResultError &
1104+
operator=(AssignNeverSendableIntoSendingResultError &&other) = default;
1105+
10891106
void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const;
10901107

10911108
SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) {
@@ -1102,6 +1119,11 @@ class PartitionOpError {
11021119
Operand *sendingOp)
11031120
: op(&op), sentElement(elt), sendingOp(sendingOp) {}
11041121

1122+
InOutSendingNotInitializedAtExitError(
1123+
InOutSendingNotInitializedAtExitError &&other) = default;
1124+
InOutSendingNotInitializedAtExitError &
1125+
operator=(InOutSendingNotInitializedAtExitError &&other) = default;
1126+
11051127
void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const;
11061128

11071129
SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) {
@@ -1119,6 +1141,11 @@ class PartitionOpError {
11191141
SILDynamicMergedIsolationInfo isolation)
11201142
: op(&op), inoutSendingElement(elt), isolationInfo(isolation) {}
11211143

1144+
InOutSendingNotDisconnectedAtExitError(
1145+
InOutSendingNotDisconnectedAtExitError &&other) = default;
1146+
InOutSendingNotDisconnectedAtExitError &
1147+
operator=(InOutSendingNotDisconnectedAtExitError &&other) = default;
1148+
11221149
void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const;
11231150

11241151
SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) {
@@ -1153,6 +1180,10 @@ class PartitionOpError {
11531180
: InOutSendingReturnedError(op, inoutSendingElement,
11541181
inoutSendingElement, isolationInfo) {}
11551182

1183+
InOutSendingReturnedError(InOutSendingReturnedError &&other) = default;
1184+
InOutSendingReturnedError &
1185+
operator=(InOutSendingReturnedError &&other) = default;
1186+
11561187
void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const;
11571188

11581189
SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) {
@@ -1169,6 +1200,11 @@ class PartitionOpError {
11691200
Element returnValue)
11701201
: op(&op), returnValueElement(returnValue) {}
11711202

1203+
NonSendableIsolationCrossingResultError(
1204+
NonSendableIsolationCrossingResultError &&other) = default;
1205+
NonSendableIsolationCrossingResultError &
1206+
operator=(NonSendableIsolationCrossingResultError &&other) = default;
1207+
11721208
void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const;
11731209

11741210
SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) {
@@ -1177,12 +1213,14 @@ class PartitionOpError {
11771213
};
11781214

11791215
#define PARTITION_OP_ERROR(NAME) \
1180-
static_assert(std::is_copy_constructible_v<NAME##Error>, \
1181-
#NAME " must be copy constructable");
1182-
#include "PartitionOpError.def"
1183-
#define PARTITION_OP_ERROR(NAME) \
1184-
static_assert(std::is_copy_assignable_v<NAME##Error>, \
1185-
#NAME " must be copy assignable");
1216+
static_assert(!std::is_copy_constructible_v<NAME##Error>, \
1217+
#NAME " must not be copy constructable"); \
1218+
static_assert(std::is_move_constructible_v<NAME##Error>, \
1219+
#NAME " must be move constructable"); \
1220+
static_assert(!std::is_copy_assignable_v<NAME##Error>, \
1221+
#NAME " must not be copy assignable"); \
1222+
static_assert(std::is_move_assignable_v<NAME##Error>, \
1223+
#NAME " must be move assignable");
11861224
#include "PartitionOpError.def"
11871225

11881226
private:
@@ -1196,11 +1234,12 @@ class PartitionOpError {
11961234

11971235
public:
11981236
#define PARTITION_OP_ERROR(NAME) \
1199-
PartitionOpError(NAME##Error error) : kind(Kind::NAME), data(error) {}
1237+
PartitionOpError(NAME##Error &&error) \
1238+
: kind(Kind::NAME), data(std::move(error)) {}
12001239
#include "PartitionOpError.def"
12011240

1202-
PartitionOpError(const PartitionOpError &error)
1203-
: kind(error.kind), data(error.data) {
1241+
PartitionOpError(PartitionOpError &&error)
1242+
: kind(error.kind), data(std::move(error.data)) {
12041243
switch (getKind()) {
12051244
#define PARTITION_OP_ERROR(NAME) \
12061245
case NAME: \
@@ -1211,9 +1250,9 @@ class PartitionOpError {
12111250
}
12121251
}
12131252

1214-
PartitionOpError &operator=(const PartitionOpError &error) {
1253+
PartitionOpError &operator=(PartitionOpError &&error) {
12151254
kind = error.kind;
1216-
data = error.data;
1255+
data = std::move(error.data);
12171256

12181257
switch (getKind()) {
12191258
#define PARTITION_OP_ERROR(NAME) \
@@ -1232,7 +1271,7 @@ class PartitionOpError {
12321271
switch (getKind()) {
12331272
#define PARTITION_OP_ERROR(NAME) \
12341273
case NAME: \
1235-
return get##NAME##Error().print(os, valueMap);
1274+
return std::get<NAME##Error>(data).print(os, valueMap);
12361275
#include "PartitionOpError.def"
12371276
}
12381277
llvm_unreachable("Covered switch isn't covered?!");
@@ -1243,9 +1282,9 @@ class PartitionOpError {
12431282
}
12441283

12451284
#define PARTITION_OP_ERROR(NAME) \
1246-
NAME##Error get##NAME##Error() const { \
1285+
NAME##Error get##NAME##Error() && { \
12471286
assert(getKind() == Kind::NAME); \
1248-
return std::get<NAME##Error>(data); \
1287+
return std::get<NAME##Error>(std::move(data)); \
12491288
}
12501289
#include "PartitionOpError.def"
12511290
};
@@ -1288,7 +1327,9 @@ struct PartitionOpEvaluator {
12881327
return asImpl().shouldEmitVerboseLogging();
12891328
}
12901329

1291-
void handleError(PartitionOpError error) { asImpl().handleError(error); }
1330+
void handleError(PartitionOpError &&error) {
1331+
asImpl().handleError(std::move(error));
1332+
}
12921333

12931334
/// Call isActorDerived on our CRTP subclass.
12941335
bool isActorDerived(Element elt) const {
@@ -1900,7 +1941,7 @@ struct PartitionOpEvaluatorBaseImpl : PartitionOpEvaluator<Subclass> {
19001941
SILValue getInOutSendingParameter(Element elt) const { return {}; }
19011942

19021943
/// By default, do nothing upon error.
1903-
void handleError(PartitionOpError error) {}
1944+
void handleError(PartitionOpError &&error) {}
19041945

19051946
/// Returns the information about \p elt's isolation that we ascertained from
19061947
/// SIL and the AST.

‎lib/SILOptimizer/Mandatory/SendNonSendable.cpp‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -673,7 +673,7 @@ struct DiagnosticEvaluator final
673673
sendingOpToRequireInstMultiMap(sendingOpToRequireInstMultiMap),
674674
foundVerbatimErrors(foundVerbatimErrors) {}
675675

676-
void handleLocalUseAfterSend(LocalUseAfterSendError error) const {
676+
void handleLocalUseAfterSend(LocalUseAfterSendError &&error) const {
677677
const auto &partitionOp = *error.op;
678678
REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap()));
679679

@@ -689,7 +689,7 @@ struct DiagnosticEvaluator final
689689
}
690690

691691
void handleInOutSendingNotInitializedAtExitError(
692-
InOutSendingNotInitializedAtExitError error) const {
692+
InOutSendingNotInitializedAtExitError &&error) const {
693693
const PartitionOp &partitionOp = *error.op;
694694
Operand *sendingOp = error.sendingOp;
695695

@@ -700,7 +700,7 @@ struct DiagnosticEvaluator final
700700
partitionOp.getSourceInst()));
701701
}
702702

703-
void handleUnknownCodePattern(UnknownCodePatternError error) const {
703+
void handleUnknownCodePattern(UnknownCodePatternError &&error) const {
704704
const PartitionOp &op = *error.op;
705705

706706
if (shouldAbortOnUnknownPatternMatchError()) {
@@ -712,10 +712,11 @@ struct DiagnosticEvaluator final
712712
diag::regionbasedisolation_unknown_pattern);
713713
}
714714

715-
void handleError(PartitionOpError error) {
715+
void handleError(PartitionOpError &&error) {
716716
switch (error.getKind()) {
717717
case PartitionOpError::LocalUseAfterSend: {
718-
return handleLocalUseAfterSend(error.getLocalUseAfterSendError());
718+
return handleLocalUseAfterSend(
719+
std::move(error).getLocalUseAfterSendError());
719720
}
720721
case PartitionOpError::InOutSendingNotDisconnectedAtExit:
721722
case PartitionOpError::InOutSendingReturned:
@@ -727,14 +728,15 @@ struct DiagnosticEvaluator final
727728
// appropriate if they want to emit an error here (some will squelch the
728729
// error).
729730
REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap()));
730-
foundVerbatimErrors.emplace_back(error);
731+
foundVerbatimErrors.emplace_back(std::move(error));
731732
return;
732733
case PartitionOpError::InOutSendingNotInitializedAtExit: {
733734
return handleInOutSendingNotInitializedAtExitError(
734-
error.getInOutSendingNotInitializedAtExitError());
735+
std::move(error).getInOutSendingNotInitializedAtExitError());
735736
}
736737
case PartitionOpError::UnknownCodePattern: {
737-
return handleUnknownCodePattern(error.getUnknownCodePatternError());
738+
return handleUnknownCodePattern(
739+
std::move(error).getUnknownCodePatternError());
738740
}
739741
}
740742
llvm_unreachable("Covered switch isn't covered?!");
@@ -2824,10 +2826,11 @@ bool InOutSendingReturnedDiagnosticEmitter::emitOutParamIncomingValueError(
28242826

28252827
// Loop through the errors to find our InOutReturnedError. We emit one error
28262828
// per return... but we emit for multiple returns separate diagnostics.
2827-
for (auto err : errors) {
2829+
while (!errors.empty()) {
2830+
auto err = errors.pop_back_val();
28282831
if (err.getKind() != PartitionOpError::InOutSendingReturned)
28292832
continue;
2830-
auto castError = err.getInOutSendingReturnedError();
2833+
auto castError = std::move(err).getInOutSendingReturnedError();
28312834
if (castError.inoutSendingElement != inoutSendingParamElement)
28322835
continue;
28332836
// Ok, we found an error. Lets emit it and return early.
@@ -3321,8 +3324,8 @@ struct NonSendableIsolationCrossingResultDiagnosticEmitter {
33213324
SILValue representative;
33223325

33233326
NonSendableIsolationCrossingResultDiagnosticEmitter(
3324-
RegionAnalysisValueMap &valueMap, Error error)
3325-
: valueMap(valueMap), error(error),
3327+
RegionAnalysisValueMap &valueMap, Error &&error)
3328+
: valueMap(valueMap), error(std::move(error)),
33263329
representative(valueMap.getRepresentative(error.returnValueElement)) {}
33273330

33283331
SILFunction *getFunction() const {
@@ -3510,7 +3513,8 @@ void SendNonSendableImpl::emitVerbatimErrors() {
35103513
case PartitionOpError::InOutSendingNotInitializedAtExit:
35113514
llvm_unreachable("Handled elsewhere");
35123515
case PartitionOpError::AssignNeverSendableIntoSendingResult: {
3513-
auto error = erasedError.getAssignNeverSendableIntoSendingResultError();
3516+
auto error =
3517+
std::move(erasedError).getAssignNeverSendableIntoSendingResultError();
35143518
REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap()));
35153519
AssignIsolatedIntoSendingResultDiagnosticEmitter emitter(
35163520
error.op->getSourceOp(), error.destValue, error.srcValue,
@@ -3519,7 +3523,8 @@ void SendNonSendableImpl::emitVerbatimErrors() {
35193523
continue;
35203524
}
35213525
case PartitionOpError::InOutSendingNotDisconnectedAtExit: {
3522-
auto error = erasedError.getInOutSendingNotDisconnectedAtExitError();
3526+
auto error =
3527+
std::move(erasedError).getInOutSendingNotDisconnectedAtExitError();
35233528
auto inoutSendingVal =
35243529
info->getValueMap().getRepresentative(error.inoutSendingElement);
35253530
auto isolationRegionInfo = error.isolationInfo;
@@ -3533,7 +3538,7 @@ void SendNonSendableImpl::emitVerbatimErrors() {
35333538
continue;
35343539
}
35353540
case PartitionOpError::InOutSendingReturned: {
3536-
auto error = erasedError.getInOutSendingReturnedError();
3541+
auto error = std::move(erasedError).getInOutSendingReturnedError();
35373542
auto inoutSendingVal =
35383543
info->getValueMap().getRepresentative(error.inoutSendingElement);
35393544
auto returnedValue =
@@ -3548,18 +3553,19 @@ void SendNonSendableImpl::emitVerbatimErrors() {
35483553
continue;
35493554
}
35503555
case PartitionOpError::SentNeverSendable: {
3551-
auto e = erasedError.getSentNeverSendableError();
3556+
auto e = std::move(erasedError).getSentNeverSendableError();
35523557
REGIONBASEDISOLATION_LOG(e.print(llvm::dbgs(), info->getValueMap()));
35533558
SentNeverSendableDiagnosticInferrer diagnosticInferrer(
3554-
info->getValueMap(), e);
3559+
info->getValueMap(), std::move(e));
35553560
diagnosticInferrer.run();
35563561
continue;
35573562
}
35583563
case PartitionOpError::NonSendableIsolationCrossingResult: {
3559-
auto e = erasedError.getNonSendableIsolationCrossingResultError();
3564+
auto e =
3565+
std::move(erasedError).getNonSendableIsolationCrossingResultError();
35603566
REGIONBASEDISOLATION_LOG(e.print(llvm::dbgs(), info->getValueMap()));
35613567
NonSendableIsolationCrossingResultDiagnosticEmitter diagnosticInferrer(
3562-
info->getValueMap(), e);
3568+
info->getValueMap(), std::move(e));
35633569
diagnosticInferrer.emit();
35643570
continue;
35653571
}

‎unittests/SILOptimizer/PartitionUtilsTest.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ struct MockedPartitionOpEvaluatorWithFailureCallback final
9090
operandToStateMap),
9191
failureCallback(failureCallback) {}
9292

93-
void handleError(PartitionOpError error) {
93+
void handleError(PartitionOpError &&error) {
9494
switch (error.getKind()) {
9595
case PartitionOpError::UnknownCodePattern:
9696
case PartitionOpError::SentNeverSendable:
@@ -101,7 +101,7 @@ struct MockedPartitionOpEvaluatorWithFailureCallback final
101101
case PartitionOpError::InOutSendingReturned:
102102
llvm_unreachable("Unsupported");
103103
case PartitionOpError::LocalUseAfterSend: {
104-
auto state = error.getLocalUseAfterSendError();
104+
auto state = std::move(error).getLocalUseAfterSendError();
105105
failureCallback(*state.op, state.sentElement, state.sendingOp);
106106
}
107107
}

0 commit comments

Comments
 (0)