Skip to content

Commit 2170b4e

Browse files
jckarterjckarter
authored
Merge pull request #71489 from jckarter/borrowing-switch-8
MoveOnlyAddressUtils: Fixes for borrowing switch over address-only types.
2 parents db020c1 + 0365c6a commit 2170b4e

15 files changed

+370
-54
lines changed

‎include/swift/SIL/SILBuilder.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1444,9 +1444,11 @@ class SILBuilder {
14441444

14451445
MarkUnresolvedNonCopyableValueInst *createMarkUnresolvedNonCopyableValueInst(
14461446
SILLocation loc, SILValue src,
1447-
MarkUnresolvedNonCopyableValueInst::CheckKind kind) {
1447+
MarkUnresolvedNonCopyableValueInst::CheckKind kind,
1448+
MarkUnresolvedNonCopyableValueInst::IsStrict_t strict
1449+
= MarkUnresolvedNonCopyableValueInst::IsNotStrict) {
14481450
return insert(new (getModule()) MarkUnresolvedNonCopyableValueInst(
1449-
getSILDebugLocation(loc), src, kind));
1451+
getSILDebugLocation(loc), src, kind, strict));
14501452
}
14511453

14521454
MarkUnresolvedReferenceBindingInst *createMarkUnresolvedReferenceBindingInst(

‎include/swift/SIL/SILInstruction.h

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8716,15 +8716,31 @@ class MarkUnresolvedNonCopyableValueInst
87168716
/// like class initializers.
87178717
InitableButNotConsumable,
87188718
};
8719+
8720+
/// During SILGen, we have not yet done escape analysis on local variables,
8721+
/// so we conservatively emit them as boxed and let the AllocBoxToStack
8722+
/// pass promote unescaped local variables. As part of this promotion,
8723+
/// non-strict `NoConsumeOrAssign` accesses can be promoted to
8724+
/// `ConsumableAndAssignable` since the variable is locally owned
8725+
/// if it doesn't escape. "Strict" accesses on the other hand preserve
8726+
/// their stricter access constraints. This is useful for representing things
8727+
/// like `borrow` bindings.
8728+
enum IsStrict_t : bool {
8729+
IsNotStrict = false,
8730+
IsStrict = true,
8731+
};
87198732

87208733
private:
87218734
CheckKind kind;
8735+
IsStrict_t strict;
87228736

87238737
MarkUnresolvedNonCopyableValueInst(SILDebugLocation DebugLoc,
8724-
SILValue operand, CheckKind checkKind)
8738+
SILValue operand, CheckKind checkKind,
8739+
IsStrict_t strict = IsNotStrict)
87258740
: UnaryInstructionBase(DebugLoc, operand, operand->getType(),
87268741
operand->getOwnershipKind()),
8727-
kind(checkKind) {
8742+
kind(checkKind),
8743+
strict(strict) {
87288744
assert(operand->getType().isMoveOnly() &&
87298745
"mark_unresolved_non_copyable_value can only take a move only typed "
87308746
"value");
@@ -8746,6 +8762,10 @@ class MarkUnresolvedNonCopyableValueInst
87468762
return true;
87478763
}
87488764
}
8765+
8766+
IsStrict_t isStrict() const {
8767+
return strict;
8768+
}
87498769
};
87508770

87518771
/// A marker instruction that states a given alloc_box or alloc_stack is a

‎lib/SIL/IR/SILPrinter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,6 +2114,9 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
21142114
void visitMarkUnresolvedNonCopyableValueInst(
21152115
MarkUnresolvedNonCopyableValueInst *I) {
21162116
using CheckKind = MarkUnresolvedNonCopyableValueInst::CheckKind;
2117+
if (I->isStrict()) {
2118+
*this << "[strict] ";
2119+
}
21172120
switch (I->getCheckKind()) {
21182121
case CheckKind::Invalid:
21192122
llvm_unreachable("Invalid?!");

‎lib/SIL/Parser/ParseSIL.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3511,6 +3511,16 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
35113511
P.diagnose(InstLoc.getSourceLoc(), diag);
35123512
return true;
35133513
}
3514+
3515+
auto Strict = MarkUnresolvedNonCopyableValueInst::IsNotStrict;
3516+
if (AttrName.equals("strict")) {
3517+
Strict = MarkUnresolvedNonCopyableValueInst::IsStrict;
3518+
if (!parseSILOptional(AttrName, *this)) {
3519+
auto diag = diag::sil_markmustcheck_requires_attribute;
3520+
P.diagnose(InstLoc.getSourceLoc(), diag);
3521+
return true;
3522+
}
3523+
}
35143524

35153525
using CheckKind = MarkUnresolvedNonCopyableValueInst::CheckKind;
35163526
CheckKind CKind =
@@ -3535,7 +3545,8 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
35353545
if (parseSILDebugLocation(InstLoc, B))
35363546
return true;
35373547

3538-
auto *MVI = B.createMarkUnresolvedNonCopyableValueInst(InstLoc, Val, CKind);
3548+
auto *MVI = B.createMarkUnresolvedNonCopyableValueInst(InstLoc, Val, CKind,
3549+
Strict);
35393550
ResultVal = MVI;
35403551
break;
35413552
}

‎lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,9 @@ SubElementOffset::computeForAddress(SILValue projectionDerivedFromRoot,
202202
// really do not want to abort. Instead, our caller can choose to abort if
203203
// they get back a None. This ensures that we do not abort in cases where we
204204
// just want to emit to the user a "I do not understand" error.
205+
LLVM_DEBUG(llvm::dbgs() << "unhandled projection derived from root:\n";
206+
projectionDerivedFromRoot->print(llvm::dbgs()));
207+
205208
return llvm::None;
206209
}
207210
}

‎lib/SILGen/SILGenBuilder.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,13 +1090,14 @@ ManagedValue SILGenBuilder::createGuaranteedCopyableToMoveOnlyWrapperValue(
10901090

10911091
ManagedValue SILGenBuilder::createMarkUnresolvedNonCopyableValueInst(
10921092
SILLocation loc, ManagedValue value,
1093-
MarkUnresolvedNonCopyableValueInst::CheckKind kind) {
1093+
MarkUnresolvedNonCopyableValueInst::CheckKind kind,
1094+
MarkUnresolvedNonCopyableValueInst::IsStrict_t strict) {
10941095
assert((value.isPlusOne(SGF) || value.isLValue() ||
10951096
value.getType().isAddress()) &&
10961097
"Argument must be at +1 or be an address!");
10971098
CleanupCloner cloner(*this, value);
10981099
auto *mdi = SILBuilder::createMarkUnresolvedNonCopyableValueInst(
1099-
loc, value.forward(getSILGenFunction()), kind);
1100+
loc, value.forward(getSILGenFunction()), kind, strict);
11001101
return cloner.clone(mdi);
11011102
}
11021103

‎lib/SILGen/SILGenBuilder.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,9 @@ class SILGenBuilder : public SILBuilder {
493493
using SILBuilder::createMarkUnresolvedNonCopyableValueInst;
494494
ManagedValue createMarkUnresolvedNonCopyableValueInst(
495495
SILLocation loc, ManagedValue value,
496-
MarkUnresolvedNonCopyableValueInst::CheckKind kind);
496+
MarkUnresolvedNonCopyableValueInst::CheckKind kind,
497+
MarkUnresolvedNonCopyableValueInst::IsStrict_t strict
498+
= MarkUnresolvedNonCopyableValueInst::IsNotStrict);
497499

498500
using SILBuilder::emitCopyValueOperation;
499501
ManagedValue emitCopyValueOperation(SILLocation Loc, ManagedValue v);

‎lib/SILGen/SILGenPattern.cpp

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,8 +1420,12 @@ void PatternMatchEmission::bindBorrow(Pattern *pattern, VarDecl *var,
14201420
// Create a notional copy for the borrow checker to use.
14211421
bindValue = bindValue.copy(SGF, pattern);
14221422
}
1423+
// We mark the borrow check as "strict" because we don't want to allow
1424+
// consumes through the binding, even if the original value manages to be
1425+
// stack promoted during AllocBoxToStack or anything like that.
14231426
bindValue = SGF.B.createMarkUnresolvedNonCopyableValueInst(pattern, bindValue,
1424-
MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign);
1427+
MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign,
1428+
MarkUnresolvedNonCopyableValueInst::IsStrict);
14251429

14261430
SGF.VarLocs[var] = SILGenFunction::VarLoc::get(bindValue.getValue());
14271431
}
@@ -3198,6 +3202,26 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF,
31983202
SGF.Cleanups.emitBranchAndCleanups(sharedDest, caseBlock, args);
31993203
}
32003204

3205+
class EndAccessCleanup final : public Cleanup {
3206+
SILValue beginAccess;
3207+
public:
3208+
EndAccessCleanup(SILValue beginAccess)
3209+
: beginAccess(beginAccess)
3210+
{}
3211+
3212+
void emit(SILGenFunction &SGF, CleanupLocation loc, ForUnwind_t forUnwind)
3213+
override {
3214+
SGF.B.createEndAccess(loc, beginAccess, /*aborted*/ false);
3215+
}
3216+
3217+
void dump(SILGenFunction &SGF) const override {
3218+
llvm::errs() << "EndAccessCleanup\n";
3219+
if (beginAccess) {
3220+
beginAccess->print(llvm::errs());
3221+
}
3222+
}
3223+
};
3224+
32013225
void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
32023226
LLVM_DEBUG(llvm::dbgs() << "emitting switch stmt\n";
32033227
S->dump(llvm::dbgs());
@@ -3385,12 +3409,22 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
33853409
if (!subjectMV.isPlusZero()) {
33863410
subjectMV = subjectMV.borrow(*this, S);
33873411
}
3388-
if (subjectMV.getType().isAddress() &&
3389-
subjectMV.getType().isLoadable(F)) {
3390-
// Load a borrow if the type is loadable.
3391-
subjectMV = subjectUndergoesFormalAccess
3392-
? B.createFormalAccessLoadBorrow(S, subjectMV)
3393-
: B.createLoadBorrow(S, subjectMV);
3412+
if (subjectMV.getType().isAddress()) {
3413+
if (subjectMV.getType().isLoadable(F)) {
3414+
// Load a borrow if the type is loadable.
3415+
subjectMV = subjectUndergoesFormalAccess
3416+
? B.createFormalAccessLoadBorrow(S, subjectMV)
3417+
: B.createLoadBorrow(S, subjectMV);
3418+
} else {
3419+
// Initiate a read access on the memory, to ensure that even
3420+
// if the underlying memory is mutable or consumable, the pattern
3421+
// match is not allowed to modify it.
3422+
auto access = B.createBeginAccess(S, subjectMV.getValue(),
3423+
SILAccessKind::Read,
3424+
SILAccessEnforcement::Static, false, false);
3425+
Cleanups.pushCleanup<EndAccessCleanup>(access);
3426+
subjectMV = ManagedValue::forBorrowedAddressRValue(access);
3427+
}
33943428
}
33953429
return {subjectMV, CastConsumptionKind::BorrowAlways};
33963430

0 commit comments

Comments
 (0)