Skip to content

Commit 5d4b93f

Browse files
committed
[region-isolation] Add a wrapper ADT for RequireInst SILInstructions so we can attach a Kind and propagate it.
The reason why I am doing this is that I am adding support for failing to reinitialize an inout sending parameter that was transferred. To make it really easy to do I am just going to explicitly represent this on the RequireInst and let the decision on what diagnostic to be done to be represented in the pseudo-IR instead of trying to just infer it straight from the type of require inst. This keeps all of the logic in the same place and attempts to keep the diagnostic emitter not use logic and instead just emit. This is NFC so I can make sure that things work before adding the additional code.
1 parent 69f0d2c commit 5d4b93f

File tree

1 file changed

+48
-18
lines changed

1 file changed

+48
-18
lines changed

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -348,9 +348,9 @@ void RequireLiveness::process(Collection requireInstList) {
348348

349349
// Then put all of our requires into our allRequires set.
350350
BasicBlockWorklist initializingWorklist(transferInst->getFunction());
351-
for (auto *require : requireInstList) {
352-
LLVM_DEBUG(llvm::dbgs() << " Require Inst: " << *require);
353-
allRequires.insert(require);
351+
for (auto require : requireInstList) {
352+
LLVM_DEBUG(llvm::dbgs() << " Require Inst: " << **require);
353+
allRequires.insert(*require);
354354
initializingWorklist.pushIfNotVisited(require->getParent());
355355
}
356356

@@ -455,9 +455,38 @@ struct TransferredNonTransferrableInfo {
455455
isolationRegionInfo(isolationRegionInfo) {}
456456
};
457457

458+
/// Wrapper around a SILInstruction that internally specifies whether we are
459+
/// dealing with an inout reinitialization needed or if it is just a normal
460+
/// use after transfer.
461+
class RequireInst {
462+
enum Kind {
463+
UseAfterTransfer,
464+
InOutReinitializationNeeded,
465+
};
466+
467+
llvm::PointerIntPair<SILInstruction *, 1> instAndKind;
468+
469+
RequireInst(SILInstruction *inst, Kind kind) : instAndKind(inst, kind) {}
470+
471+
public:
472+
static RequireInst forUseAfterTransfer(SILInstruction *inst) {
473+
return {inst, Kind::UseAfterTransfer};
474+
}
475+
476+
static RequireInst forInOutReinitializationNeeded(SILInstruction *inst) {
477+
return {inst, Kind::InOutReinitializationNeeded};
478+
}
479+
480+
SILInstruction *getInst() const { return instAndKind.getPointer(); }
481+
Kind getKind() const { return Kind(instAndKind.getInt()); }
482+
483+
SILInstruction *operator*() const { return getInst(); }
484+
SILInstruction *operator->() const { return getInst(); }
485+
};
486+
458487
class TransferNonSendableImpl {
459488
RegionAnalysisFunctionInfo *regionInfo;
460-
SmallFrozenMultiMap<Operand *, SILInstruction *, 8>
489+
SmallFrozenMultiMap<Operand *, RequireInst, 8>
461490
transferOpToRequireInstMultiMap;
462491
SmallVector<TransferredNonTransferrableInfo, 8>
463492
transferredNonTransferrableInfoList;
@@ -485,12 +514,12 @@ namespace {
485514

486515
class UseAfterTransferDiagnosticEmitter {
487516
Operand *transferOp;
488-
SmallVectorImpl<SILInstruction *> &requireInsts;
517+
SmallVectorImpl<RequireInst> &requireInsts;
489518
bool emittedErrorDiagnostic = false;
490519

491520
public:
492-
UseAfterTransferDiagnosticEmitter(
493-
Operand *transferOp, SmallVectorImpl<SILInstruction *> &requireInsts)
521+
UseAfterTransferDiagnosticEmitter(Operand *transferOp,
522+
SmallVectorImpl<RequireInst> &requireInsts)
494523
: transferOp(transferOp), requireInsts(requireInsts) {}
495524

496525
~UseAfterTransferDiagnosticEmitter() {
@@ -734,8 +763,8 @@ class UseAfterTransferDiagnosticEmitter {
734763
void emitRequireInstDiagnostics() {
735764
// Now actually emit the require notes.
736765
while (!requireInsts.empty()) {
737-
auto *require = requireInsts.pop_back_val();
738-
diagnoseNote(require, diag::regionbasedisolation_maybe_race)
766+
auto require = requireInsts.pop_back_val();
767+
diagnoseNote(*require, diag::regionbasedisolation_maybe_race)
739768
.highlight(require->getLoc().getSourceRange());
740769
}
741770
}
@@ -753,7 +782,7 @@ class UseAfterTransferDiagnosticInferrer {
753782

754783
public:
755784
UseAfterTransferDiagnosticInferrer(
756-
Operand *transferOp, SmallVectorImpl<SILInstruction *> &requireInsts,
785+
Operand *transferOp, SmallVectorImpl<RequireInst> &requireInsts,
757786
RegionAnalysisValueMap &valueMap,
758787
TransferringOperandToStateMap &transferringOpToStateMap)
759788
: transferOp(transferOp), diagnosticEmitter(transferOp, requireInsts),
@@ -1074,17 +1103,17 @@ void TransferNonSendableImpl::emitUseAfterTransferDiagnostics() {
10741103
++blockLivenessInfoGeneration;
10751104
liveness.process(requireInsts);
10761105

1077-
SmallVector<SILInstruction *, 8> requireInstsForError;
1078-
for (auto *require : requireInsts) {
1106+
SmallVector<RequireInst, 8> requireInstsForError;
1107+
for (auto require : requireInsts) {
10791108
// We can have multiple of the same require insts if we had a require
10801109
// and an assign from the same instruction. Our liveness checking
10811110
// above doesn't care about that, but we still need to make sure we do
10821111
// not emit twice.
1083-
if (!requireInstsUnique.insert(require))
1112+
if (!requireInstsUnique.insert(*require))
10841113
continue;
10851114

10861115
// If this was not a last require, do not emit an error.
1087-
if (!liveness.finalRequires.contains(require))
1116+
if (!liveness.finalRequires.contains(*require))
10881117
continue;
10891118

10901119
requireInstsForError.push_back(require);
@@ -1532,7 +1561,7 @@ namespace {
15321561
struct DiagnosticEvaluator final
15331562
: PartitionOpEvaluatorBaseImpl<DiagnosticEvaluator> {
15341563
RegionAnalysisFunctionInfo *info;
1535-
SmallFrozenMultiMap<Operand *, SILInstruction *, 8>
1564+
SmallFrozenMultiMap<Operand *, RequireInst, 8>
15361565
&transferOpToRequireInstMultiMap;
15371566

15381567
/// First value is the operand that was transferred... second value is the
@@ -1542,7 +1571,7 @@ struct DiagnosticEvaluator final
15421571

15431572
DiagnosticEvaluator(Partition &workingPartition,
15441573
RegionAnalysisFunctionInfo *info,
1545-
SmallFrozenMultiMap<Operand *, SILInstruction *, 8>
1574+
SmallFrozenMultiMap<Operand *, RequireInst, 8>
15461575
&transferOpToRequireInstMultiMap,
15471576
SmallVectorImpl<TransferredNonTransferrableInfo>
15481577
&transferredNonTransferrable,
@@ -1581,8 +1610,9 @@ struct DiagnosticEvaluator final
15811610
<< " ID: %%" << transferredVal << "\n"
15821611
<< " Rep: " << *rep << " Transferring Op Num: "
15831612
<< transferringOp->getOperandNumber() << '\n');
1584-
transferOpToRequireInstMultiMap.insert(transferringOp,
1585-
partitionOp.getSourceInst());
1613+
transferOpToRequireInstMultiMap.insert(
1614+
transferringOp,
1615+
RequireInst::forUseAfterTransfer(partitionOp.getSourceInst()));
15861616
}
15871617

15881618
void handleTransferNonTransferrable(

0 commit comments

Comments
 (0)