Skip to content

Commit 7d86bf0

Browse files
committed
[move-only] Teach the move checker how to find the uses of a non-escaping partial apply and use that to emit diagnostics.
The previous commits in this series of changes reverted the allocbox to stack change. Even with that though, we were treating the forming of the partial apply and the destroys of the partial apply as the liveness requiring uses. This is not the correct semantics for the non-escaping let closures. Instead, we want to treat the passing off of the partial apply to another function or the invocation of the partial apply as the liveness requiring uses. This makes sense since when we capture a noncopyable type in the closure, we capture them as inout_aliasable, that is as a pointer, so we are not going to actually use the value until we call the partial_apply.
1 parent 9c9fe09 commit 7d86bf0

File tree

1 file changed

+117
-9
lines changed

1 file changed

+117
-9
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 117 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@
225225
#include "swift/AST/AccessScope.h"
226226
#include "swift/AST/DiagnosticEngine.h"
227227
#include "swift/AST/DiagnosticsSIL.h"
228+
#include "swift/AST/SemanticAttrs.h"
228229
#include "swift/Basic/Debug.h"
229230
#include "swift/Basic/Defer.h"
230231
#include "swift/Basic/FrozenMultiMap.h"
@@ -492,6 +493,92 @@ static bool isInOutDefThatNeedsEndOfFunctionLiveness(MarkMustCheckInst *markedAd
492493
return false;
493494
}
494495

496+
//===----------------------------------------------------------------------===//
497+
// MARK: Partial Apply Utilities
498+
//===----------------------------------------------------------------------===//
499+
500+
static bool findNonEscapingPartialApplyUses(
501+
PartialApplyInst *pai, TypeTreeLeafTypeRange leafRange,
502+
llvm::SmallMapVector<SILInstruction *, TypeTreeLeafTypeRange, 4>
503+
&livenessUses) {
504+
StackList<Operand *> worklist(pai->getFunction());
505+
for (auto *use : pai->getUses())
506+
worklist.push_back(use);
507+
508+
LLVM_DEBUG(llvm::dbgs() << "Searching for partial apply uses!\n");
509+
while (!worklist.empty()) {
510+
auto *use = worklist.pop_back_val();
511+
512+
if (use->isTypeDependent())
513+
continue;
514+
515+
auto *user = use->getUser();
516+
517+
// These instructions do not cause us to escape.
518+
if (isIncidentalUse(user) || isa<DestroyValueInst>(user))
519+
continue;
520+
521+
// Look through these instructions.
522+
if (isa<BeginBorrowInst>(user) || isa<CopyValueInst>(user) ||
523+
isa<MoveValueInst>(user) ||
524+
// If we capture this partial_apply in another partial_apply, then we
525+
// know that said partial_apply must not have escaped the value since
526+
// otherwise we could not have an inout_aliasable argument or be
527+
// on_stack. Process it recursively so that we treat uses of that
528+
// partial_apply and applies of that partial_apply as uses of our
529+
// partial_apply.
530+
//
531+
// We have this separately from the other look through sections so that
532+
// we can make it clearer what we are doing here.
533+
isa<PartialApplyInst>(user)) {
534+
for (auto *use : cast<SingleValueInstruction>(user)->getUses())
535+
worklist.push_back(use);
536+
continue;
537+
}
538+
539+
// If we have a mark_dependence and are the value, look through the
540+
// mark_dependence.
541+
if (auto *mdi = dyn_cast<MarkDependenceInst>(user)) {
542+
if (mdi->getValue() == use->get()) {
543+
for (auto *use : mdi->getUses())
544+
worklist.push_back(use);
545+
continue;
546+
}
547+
}
548+
549+
if (auto apply = FullApplySite::isa(user)) {
550+
// If we apply the function or pass the function off to an apply, then we
551+
// need to treat the function application as a liveness use of the
552+
// variable since if the partial_apply is invoked within the function
553+
// application, we may access the captured variable.
554+
livenessUses.insert({user, leafRange});
555+
if (apply.beginsCoroutineEvaluation()) {
556+
// If we have a coroutine, we need to treat the abort_apply and
557+
// end_apply as liveness uses since once we execute one of those
558+
// instructions, we have returned control to the coroutine which means
559+
// that we could then access the captured variable again.
560+
auto *bai = cast<BeginApplyInst>(user);
561+
SmallVector<EndApplyInst *, 4> endApplies;
562+
SmallVector<AbortApplyInst *, 4> abortApplies;
563+
bai->getCoroutineEndPoints(endApplies, abortApplies);
564+
for (auto *eai : endApplies)
565+
livenessUses.insert({eai, leafRange});
566+
for (auto *aai : abortApplies)
567+
livenessUses.insert({aai, leafRange});
568+
}
569+
continue;
570+
}
571+
572+
LLVM_DEBUG(
573+
llvm::dbgs()
574+
<< "Found instruction we did not understand... returning false!\n");
575+
LLVM_DEBUG(llvm::dbgs() << "Instruction: " << *user);
576+
return false;
577+
}
578+
579+
return true;
580+
}
581+
495582
//===----------------------------------------------------------------------===//
496583
// MARK: Find Candidate Mark Must Checks
497584
//===----------------------------------------------------------------------===//
@@ -1634,10 +1721,15 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
16341721
<< "Found mark must check [nocopy] error: " << *user);
16351722
auto *fArg = dyn_cast<SILFunctionArgument>(
16361723
stripAccessMarkers(markedValue->getOperand()));
1637-
if (fArg && fArg->isClosureCapture() && fArg->getType().isAddress()) {
1638-
moveChecker.diagnosticEmitter.emitPromotedBoxArgumentError(
1639-
markedValue, fArg);
1724+
// If we have a closure captured that we specialized, we should have a
1725+
// no consume or assign and should emit a normal guaranteed diagnostic.
1726+
if (fArg && fArg->isClosureCapture() &&
1727+
fArg->getArgumentConvention().isInoutConvention()) {
1728+
assert(checkKind == MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
1729+
moveChecker.diagnosticEmitter.emitObjectGuaranteedDiagnostic(
1730+
markedValue);
16401731
} else {
1732+
// Otherwise, we need to emit an escaping closure error.
16411733
moveChecker.diagnosticEmitter
16421734
.emitAddressEscapingClosureCaptureLoadedAndConsumed(markedValue);
16431735
}
@@ -1792,6 +1884,17 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
17921884
}
17931885

17941886
if (auto *pas = dyn_cast<PartialApplyInst>(user)) {
1887+
if (auto *fArg = dyn_cast<SILFunctionArgument>(
1888+
stripAccessMarkers(markedValue->getOperand()))) {
1889+
// If we are processing an inout convention and we emitted an error on the
1890+
// partial_apply, we shouldn't process this mark_must_check, but squelch
1891+
// the compiler doesn't understand error.
1892+
if (fArg->getArgumentConvention().isInoutConvention() &&
1893+
pas->getCalleeFunction()->hasSemanticsAttr(
1894+
semantics::NO_MOVEONLY_DIAGNOSTICS)) {
1895+
}
1896+
}
1897+
17951898
if (pas->isOnStack() ||
17961899
ApplySite(pas).getArgumentConvention(*op).isInoutConvention()) {
17971900
LLVM_DEBUG(llvm::dbgs() << "Found on stack partial apply or inout usage!\n");
@@ -1803,13 +1906,18 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
18031906
return false;
18041907
}
18051908

1806-
useState.livenessUses.insert({user, *leafRange});
1807-
for (auto *use : pas->getConsumingUses()) {
1808-
LLVM_DEBUG(llvm::dbgs()
1809-
<< "Adding consuming use of partial apply as liveness use: "
1810-
<< *use->getUser());
1811-
useState.livenessUses.insert({use->getUser(), *leafRange});
1909+
// Attempt to find calls of the non-escaping partial apply and places
1910+
// where the partial apply is passed to a function. We treat those as
1911+
// liveness uses. If we find a use we don't understand, we return false
1912+
// here.
1913+
if (!findNonEscapingPartialApplyUses(pas, *leafRange,
1914+
useState.livenessUses)) {
1915+
LLVM_DEBUG(
1916+
llvm::dbgs()
1917+
<< "Failed to understand use of a non-escaping partial apply?!\n");
1918+
return false;
18121919
}
1920+
18131921
return true;
18141922
}
18151923
}

0 commit comments

Comments
 (0)