Skip to content

Commit 1bef011

Browse files
committed
[region-isolation] Add the ability for the analysis to emit "unknown error" partition ops in case we detect a case we cannot pattern match.
DISCUSSION: The analysis itself is unable to emit errors. So we achieve the same functionality by in such cases emitting a partition op that signals to our user that when they process that partition op they should emit an "unknown pattern" error at the partition op's instructions. I have wanted this for a long time, but I never got around to it.
1 parent 741244e commit 1bef011

File tree

4 files changed

+56
-0
lines changed

4 files changed

+56
-0
lines changed

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,16 @@ enum class PartitionOpKind : uint8_t {
400400

401401
/// Require the region of a value to be non-transferred, takes one arg.
402402
Require,
403+
404+
/// Emit an error saying that the given instruction was not understood for
405+
/// some reason and that a bug should be filed. It expects some sort of
406+
/// Element number since in most cases we need to define a value for later
407+
/// potential uses of the value (e.x.: an alloc_stack that we emit an unknown
408+
/// pattern error will have later uses that will use the value... without
409+
/// defining the value, the dataflow will assert).
410+
///
411+
/// This is used if we need to reject the program and do not want to assert.
412+
UnknownPatternError,
403413
};
404414

405415
/// PartitionOp represents a primitive operation that can be performed on
@@ -444,6 +454,9 @@ class PartitionOp {
444454
"Transfer needs a sourceInst");
445455
}
446456

457+
PartitionOp(PartitionOpKind opKind, SILInstruction *sourceInst)
458+
: opKind(opKind), opArgs(), source(sourceInst) {}
459+
447460
friend class Partition;
448461

449462
public:
@@ -476,6 +489,11 @@ class PartitionOp {
476489
return PartitionOp(PartitionOpKind::Require, tgt, sourceInst);
477490
}
478491

492+
static PartitionOp UnknownPatternError(Element elt,
493+
SILInstruction *sourceInst) {
494+
return PartitionOp(PartitionOpKind::UnknownPatternError, elt, sourceInst);
495+
}
496+
479497
bool operator==(const PartitionOp &other) const {
480498
return opKind == other.opKind && opArgs == other.opArgs &&
481499
source == other.source;
@@ -881,6 +899,11 @@ struct PartitionOpEvaluator {
881899
return asImpl().shouldEmitVerboseLogging();
882900
}
883901

902+
/// Call handleUnknownCodePattern on our CRTP subclass.
903+
void handleUnknownCodePattern(const PartitionOp &op) const {
904+
return asImpl().handleUnknownCodePattern(op);
905+
}
906+
884907
/// Call handleLocalUseAfterTransfer on our CRTP subclass.
885908
void handleLocalUseAfterTransfer(const PartitionOp &op, Element elt,
886909
Operand *transferringOp) const {
@@ -1119,6 +1142,13 @@ struct PartitionOpEvaluator {
11191142
}
11201143
}
11211144
return;
1145+
case PartitionOpKind::UnknownPatternError:
1146+
// Begin tracking the specified element in case we have a later use.
1147+
p.trackNewElement(op.getOpArgs()[0]);
1148+
1149+
// Then emit an unknown code pattern error.
1150+
handleUnknownCodePattern(op);
1151+
return;
11221152
}
11231153

11241154
llvm_unreachable("Covered switch isn't covered?!");
@@ -1259,6 +1289,13 @@ struct PartitionOpEvaluatorBaseImpl : PartitionOpEvaluator<Subclass> {
12591289
const PartitionOp &op, Element elt, Element otherElement,
12601290
SILDynamicMergedIsolationInfo isolationRegionInfo) const {}
12611291

1292+
/// Used to signify an "unknown code pattern" has occured while performing
1293+
/// dataflow.
1294+
///
1295+
/// DISCUSSION: Our dataflow cannot emit errors itself so this is a callback
1296+
/// to our user so that we can emit that error as we process.
1297+
void handleUnknownCodePattern(const PartitionOp &op) const {}
1298+
12621299
/// This is used to determine if an element is actor derived. If we determine
12631300
/// that a region containing such an element is transferred, we emit an error
12641301
/// since actor regions cannot be transferred.

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,6 +1189,11 @@ struct PartitionOpBuilder {
11891189
PartitionOp::Require(lookupValueID(value), currentInst));
11901190
}
11911191

1192+
void addUnknownPatternError(SILValue value) {
1193+
currentInstPartitionOps.emplace_back(
1194+
PartitionOp::UnknownPatternError(lookupValueID(value), currentInst));
1195+
}
1196+
11921197
SWIFT_DEBUG_DUMP { print(llvm::dbgs()); }
11931198

11941199
void print(llvm::raw_ostream &os) const;
@@ -2150,6 +2155,11 @@ class PartitionOpTranslator {
21502155
}
21512156
}
21522157

2158+
/// Emit an unknown pattern error.
2159+
void translateUnknownPatternError(SILValue value) {
2160+
builder.addUnknownPatternError(value);
2161+
}
2162+
21532163
/// Translate the instruction's in \p basicBlock to a vector of PartitionOps
21542164
/// that define the block's dataflow.
21552165
void translateSILBasicBlock(SILBasicBlock *basicBlock,

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,6 +1600,11 @@ struct DiagnosticEvaluator final
16001600
}
16011601
}
16021602

1603+
void handleUnknownCodePattern(const PartitionOp &op) const {
1604+
diagnoseError(op.getSourceInst(),
1605+
diag::regionbasedisolation_unknown_pattern);
1606+
}
1607+
16031608
bool isActorDerived(Element element) const {
16041609
return info->getValueMap().getIsolationRegion(element).isActorIsolated();
16051610
}

lib/SILOptimizer/Utils/PartitionUtils.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ void PartitionOp::print(llvm::raw_ostream &os, bool extraSpace) const {
9494
os << "%%" << opArgs[0];
9595
break;
9696
}
97+
case PartitionOpKind::UnknownPatternError:
98+
os << "unknown pattern error ";
99+
os << "%%" << opArgs[0];
100+
break;
97101
}
98102
os << ": " << *getSourceInst();
99103
}

0 commit comments

Comments
 (0)