Skip to content

Commit f2057f8

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. (cherry picked from commit 5d4b93f)
1 parent 036cf8b commit f2057f8

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
@@ -347,9 +347,9 @@ void RequireLiveness::process(Collection requireInstList) {
347347

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

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

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

485514
class UseAfterTransferDiagnosticEmitter {
486515
Operand *transferOp;
487-
SmallVectorImpl<SILInstruction *> &requireInsts;
516+
SmallVectorImpl<RequireInst> &requireInsts;
488517
bool emittedErrorDiagnostic = false;
489518

490519
public:
491-
UseAfterTransferDiagnosticEmitter(
492-
Operand *transferOp, SmallVectorImpl<SILInstruction *> &requireInsts)
520+
UseAfterTransferDiagnosticEmitter(Operand *transferOp,
521+
SmallVectorImpl<RequireInst> &requireInsts)
493522
: transferOp(transferOp), requireInsts(requireInsts) {}
494523

495524
~UseAfterTransferDiagnosticEmitter() {
@@ -715,8 +744,8 @@ class UseAfterTransferDiagnosticEmitter {
715744
void emitRequireInstDiagnostics() {
716745
// Now actually emit the require notes.
717746
while (!requireInsts.empty()) {
718-
auto *require = requireInsts.pop_back_val();
719-
diagnoseNote(require, diag::regionbasedisolation_maybe_race)
747+
auto require = requireInsts.pop_back_val();
748+
diagnoseNote(*require, diag::regionbasedisolation_maybe_race)
720749
.highlight(require->getLoc().getSourceRange());
721750
}
722751
}
@@ -734,7 +763,7 @@ class UseAfterTransferDiagnosticInferrer {
734763

735764
public:
736765
UseAfterTransferDiagnosticInferrer(
737-
Operand *transferOp, SmallVectorImpl<SILInstruction *> &requireInsts,
766+
Operand *transferOp, SmallVectorImpl<RequireInst> &requireInsts,
738767
RegionAnalysisValueMap &valueMap,
739768
TransferringOperandToStateMap &transferringOpToStateMap)
740769
: transferOp(transferOp), diagnosticEmitter(transferOp, requireInsts),
@@ -1037,17 +1066,17 @@ void TransferNonSendableImpl::emitUseAfterTransferDiagnostics() {
10371066
++blockLivenessInfoGeneration;
10381067
liveness.process(requireInsts);
10391068

1040-
SmallVector<SILInstruction *, 8> requireInstsForError;
1041-
for (auto *require : requireInsts) {
1069+
SmallVector<RequireInst, 8> requireInstsForError;
1070+
for (auto require : requireInsts) {
10421071
// We can have multiple of the same require insts if we had a require
10431072
// and an assign from the same instruction. Our liveness checking
10441073
// above doesn't care about that, but we still need to make sure we do
10451074
// not emit twice.
1046-
if (!requireInstsUnique.insert(require))
1075+
if (!requireInstsUnique.insert(*require))
10471076
continue;
10481077

10491078
// If this was not a last require, do not emit an error.
1050-
if (!liveness.finalRequires.contains(require))
1079+
if (!liveness.finalRequires.contains(*require))
10511080
continue;
10521081

10531082
requireInstsForError.push_back(require);
@@ -1485,7 +1514,7 @@ namespace {
14851514
struct DiagnosticEvaluator final
14861515
: PartitionOpEvaluatorBaseImpl<DiagnosticEvaluator> {
14871516
RegionAnalysisFunctionInfo *info;
1488-
SmallFrozenMultiMap<Operand *, SILInstruction *, 8>
1517+
SmallFrozenMultiMap<Operand *, RequireInst, 8>
14891518
&transferOpToRequireInstMultiMap;
14901519

14911520
/// First value is the operand that was transferred... second value is the
@@ -1495,7 +1524,7 @@ struct DiagnosticEvaluator final
14951524

14961525
DiagnosticEvaluator(Partition &workingPartition,
14971526
RegionAnalysisFunctionInfo *info,
1498-
SmallFrozenMultiMap<Operand *, SILInstruction *, 8>
1527+
SmallFrozenMultiMap<Operand *, RequireInst, 8>
14991528
&transferOpToRequireInstMultiMap,
15001529
SmallVectorImpl<TransferredNonTransferrableInfo>
15011530
&transferredNonTransferrable,
@@ -1534,8 +1563,9 @@ struct DiagnosticEvaluator final
15341563
<< " ID: %%" << transferredVal << "\n"
15351564
<< " Rep: " << *rep << " Transferring Op Num: "
15361565
<< transferringOp->getOperandNumber() << '\n');
1537-
transferOpToRequireInstMultiMap.insert(transferringOp,
1538-
partitionOp.getSourceInst());
1566+
transferOpToRequireInstMultiMap.insert(
1567+
transferringOp,
1568+
RequireInst::forUseAfterTransfer(partitionOp.getSourceInst()));
15391569
}
15401570

15411571
void handleTransferNonTransferrable(

0 commit comments

Comments
 (0)