Skip to content

Commit 161183c

Browse files
jckarterjckarter
authoredFeb 22, 2024
Merge pull request #71803 from jckarter/begin-borrow-fixed-switch-subject
SIL: Enclose switch subjects in a new begin_borrow [fixed] variant.
2 parents a27cf55 + d75a62c commit 161183c

14 files changed

+100
-35
lines changed
 

‎include/swift/SIL/SILBuilder.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -817,12 +817,13 @@ class SILBuilder {
817817
BeginBorrowInst *createBeginBorrow(SILLocation Loc, SILValue LV,
818818
bool isLexical = false,
819819
bool hasPointerEscape = false,
820-
bool fromVarDecl = false) {
820+
bool fromVarDecl = false,
821+
bool fixed = false) {
821822
assert(getFunction().hasOwnership());
822823
assert(!LV->getType().isAddress());
823824
return insert(new (getModule())
824825
BeginBorrowInst(getSILDebugLocation(Loc), LV, isLexical,
825-
hasPointerEscape, fromVarDecl));
826+
hasPointerEscape, fromVarDecl, fixed));
826827
}
827828

828829
/// Convenience function for creating a load_borrow on non-trivial values and

‎include/swift/SIL/SILInstruction.h

+10-1
Original file line numberDiff line numberDiff line change
@@ -4520,15 +4520,18 @@ class BeginBorrowInst
45204520
USE_SHARED_UINT8;
45214521

45224522
BeginBorrowInst(SILDebugLocation DebugLoc, SILValue LValue, bool isLexical,
4523-
bool hasPointerEscape, bool fromVarDecl)
4523+
bool hasPointerEscape, bool fromVarDecl, bool fixed)
45244524
: UnaryInstructionBase(DebugLoc, LValue,
45254525
LValue->getType().getObjectType()) {
45264526
sharedUInt8().BeginBorrowInst.lexical = isLexical;
45274527
sharedUInt8().BeginBorrowInst.pointerEscape = hasPointerEscape;
45284528
sharedUInt8().BeginBorrowInst.fromVarDecl = fromVarDecl;
4529+
sharedUInt8().BeginBorrowInst.fixed = fixed;
45294530
}
45304531

45314532
public:
4533+
4534+
45324535
// FIXME: this does not return all instructions that end a local borrow
45334536
// scope. Branches can also end it via a reborrow, so APIs using this are
45344537
// incorrect. Instead, either iterate over all uses and return those with
@@ -4556,6 +4559,12 @@ class BeginBorrowInst
45564559
return sharedUInt8().BeginBorrowInst.fromVarDecl;
45574560
}
45584561

4562+
/// Whether the borrow scope is fixed during move checking and should be
4563+
/// treated as an opaque use of the value.
4564+
bool isFixed() const {
4565+
return sharedUInt8().BeginBorrowInst.fixed;
4566+
}
4567+
45594568
/// Return a range over all EndBorrow instructions for this BeginBorrow.
45604569
EndBorrowRange getEndBorrows() const;
45614570

‎include/swift/SIL/SILNode.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,8 @@ class alignas(8) SILNode :
243243
SHARED_FIELD(BeginBorrowInst, uint8_t
244244
lexical : 1,
245245
pointerEscape : 1,
246-
fromVarDecl : 1);
246+
fromVarDecl : 1,
247+
fixed : 1);
247248

248249
SHARED_FIELD(CopyAddrInst, uint8_t
249250
isTakeOfSrc : 1,

‎lib/SIL/IR/SILPrinter.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -1737,6 +1737,9 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
17371737
if (BBI->isFromVarDecl()) {
17381738
*this << "[var_decl] ";
17391739
}
1740+
if (BBI->isFixed()) {
1741+
*this << "[fixed] ";
1742+
}
17401743
*this << getIDAndType(BBI->getOperand());
17411744
}
17421745

‎lib/SIL/Parser/ParseSIL.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -3707,6 +3707,7 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
37073707
bool isLexical = false;
37083708
bool hasPointerEscape = false;
37093709
bool fromVarDecl = false;
3710+
bool fixed = false;
37103711

37113712
StringRef AttrName;
37123713
SourceLoc AttrLoc;
@@ -3717,6 +3718,8 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
37173718
hasPointerEscape = true;
37183719
else if (AttrName == "var_decl")
37193720
fromVarDecl = true;
3721+
else if (AttrName == "fixed")
3722+
fixed = true;
37203723
else {
37213724
P.diagnose(InstLoc.getSourceLoc(),
37223725
diag::sil_invalid_attribute_for_instruction, AttrName,
@@ -3730,7 +3733,7 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
37303733
return true;
37313734

37323735
ResultVal = B.createBeginBorrow(InstLoc, Val, isLexical, hasPointerEscape,
3733-
fromVarDecl);
3736+
fromVarDecl, fixed);
37343737
break;
37353738
}
37363739

‎lib/SILGen/SILGenBuilder.cpp

+15-2
Original file line numberDiff line numberDiff line change
@@ -1038,13 +1038,26 @@ ManagedValue SILGenBuilder::createMarkDependence(
10381038

10391039
ManagedValue SILGenBuilder::createBeginBorrow(SILLocation loc,
10401040
ManagedValue value,
1041-
bool isLexical) {
1041+
bool isLexical,
1042+
bool isFixed) {
10421043
auto *newValue =
1043-
SILBuilder::createBeginBorrow(loc, value.getValue(), isLexical);
1044+
SILBuilder::createBeginBorrow(loc, value.getValue(),
1045+
isLexical, false, false, isFixed);
10441046
SGF.emitManagedBorrowedRValueWithCleanup(newValue);
10451047
return ManagedValue::forBorrowedObjectRValue(newValue);
10461048
}
10471049

1050+
ManagedValue SILGenBuilder::createFormalAccessBeginBorrow(SILLocation loc,
1051+
ManagedValue value,
1052+
bool isLexical,
1053+
bool isFixed) {
1054+
auto *newValue =
1055+
SILBuilder::createBeginBorrow(loc, value.getValue(),
1056+
isLexical, false, false, isFixed);
1057+
return SGF.emitFormalEvaluationManagedBorrowedRValueWithCleanup(loc,
1058+
value.getValue(), newValue);
1059+
}
1060+
10481061
ManagedValue SILGenBuilder::createMoveValue(SILLocation loc, ManagedValue value,
10491062
bool isLexical) {
10501063
assert(value.isPlusOne(SGF) && "Must be +1 to be moved!");

‎lib/SILGen/SILGenBuilder.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,13 @@ class SILGenBuilder : public SILBuilder {
470470

471471
using SILBuilder::createBeginBorrow;
472472
ManagedValue createBeginBorrow(SILLocation loc, ManagedValue value,
473-
bool isLexical = false);
473+
bool isLexical = false,
474+
bool isFixed = false);
475+
476+
ManagedValue createFormalAccessBeginBorrow(SILLocation loc,
477+
ManagedValue value,
478+
bool isLexical = false,
479+
bool isFixed = false);
474480

475481
using SILBuilder::createMoveValue;
476482
ManagedValue createMoveValue(SILLocation loc, ManagedValue value,

‎lib/SILGen/SILGenPattern.cpp

+29-15
Original file line numberDiff line numberDiff line change
@@ -3424,26 +3424,29 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
34243424

34253425
case ValueOwnership::Shared:
34263426
emission.setNoncopyableBorrowingOwnership();
3427-
if (!subjectMV.isPlusZero()) {
3428-
subjectMV = subjectMV.borrow(*this, S);
3429-
}
34303427
if (subjectMV.getType().isAddress()) {
3428+
// Initiate a read access on the memory, to ensure that even
3429+
// if the underlying memory is mutable or consumable, the pattern
3430+
// match is not allowed to modify it.
3431+
auto access = B.createBeginAccess(S, subjectMV.getValue(),
3432+
SILAccessKind::Read,
3433+
SILAccessEnforcement::Static,
3434+
/*no nested conflict*/ true, false);
3435+
Cleanups.pushCleanup<EndAccessCleanup>(access);
3436+
subjectMV = ManagedValue::forBorrowedAddressRValue(access);
34313437
if (subjectMV.getType().isLoadable(F)) {
34323438
// Load a borrow if the type is loadable.
34333439
subjectMV = subjectUndergoesFormalAccess
34343440
? B.createFormalAccessLoadBorrow(S, subjectMV)
34353441
: B.createLoadBorrow(S, subjectMV);
3436-
} else {
3437-
// Initiate a read access on the memory, to ensure that even
3438-
// if the underlying memory is mutable or consumable, the pattern
3439-
// match is not allowed to modify it.
3440-
auto access = B.createBeginAccess(S, subjectMV.getValue(),
3441-
SILAccessKind::Read,
3442-
SILAccessEnforcement::Static,
3443-
/*no nested conflict*/ true, false);
3444-
Cleanups.pushCleanup<EndAccessCleanup>(access);
3445-
subjectMV = ManagedValue::forBorrowedAddressRValue(access);
34463442
}
3443+
} else {
3444+
// Initiate a fixed borrow on the subject, so that it's treated as
3445+
// opaque by the move checker.
3446+
subjectMV = subjectUndergoesFormalAccess
3447+
? B.createFormalAccessBeginBorrow(S, subjectMV,
3448+
false, /*fixed*/true)
3449+
: B.createBeginBorrow(S, subjectMV, false, /*fixed*/ true);
34473450
}
34483451
return {subjectMV, CastConsumptionKind::BorrowAlways};
34493452

@@ -3466,8 +3469,19 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
34663469
Cleanups.getCleanupsDepth(),
34673470
subjectMV);
34683471

3469-
// Perform the pattern match on a borrow of the subject.
3470-
subjectMV = subjectMV.borrow(*this, S);
3472+
// Perform the pattern match on an opaque borrow or read access of the
3473+
// subject.
3474+
if (subjectMV.getType().isAddress()) {
3475+
auto access = B.createBeginAccess(S, subjectMV.getValue(),
3476+
SILAccessKind::Read,
3477+
SILAccessEnforcement::Static,
3478+
/*no nested conflict*/ true, false);
3479+
Cleanups.pushCleanup<EndAccessCleanup>(access);
3480+
subjectMV = ManagedValue::forBorrowedAddressRValue(access);
3481+
} else {
3482+
subjectMV = B.createBeginBorrow(S, subjectMV,
3483+
false, /*fixed*/ true);
3484+
}
34713485
return {subjectMV, CastConsumptionKind::BorrowAlways};
34723486
}
34733487
llvm_unreachable("unhandled value ownership");

‎lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -435,8 +435,9 @@ bool Implementation::gatherUses(SILValue value) {
435435
continue;
436436
}
437437
case OperandOwnership::Borrow: {
438-
// Look through borrows.
439-
if (auto *bbi = dyn_cast<BeginBorrowInst>(nextUse->getUser())) {
438+
if (auto *bbi = dyn_cast<BeginBorrowInst>(nextUse->getUser());
439+
bbi && !bbi->isFixed()) {
440+
// Look through non-fixed borrows.
440441
LLVM_DEBUG(llvm::dbgs() << " Found recursive borrow!\n");
441442
for (auto *use : bbi->getUses()) {
442443
useWorklist.push_back(use);
@@ -452,7 +453,7 @@ bool Implementation::gatherUses(SILValue value) {
452453
}
453454

454455
// Otherwise, treat it as a normal use.
455-
LLVM_DEBUG(llvm::dbgs() << " Treating non-begin_borrow borrow as "
456+
LLVM_DEBUG(llvm::dbgs() << " Treating borrow as "
456457
"a non lifetime ending use!\n");
457458
blocksToUses.insert(nextUse->getParentBlock(),
458459
{nextUse,

‎lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp

+8-1
Original file line numberDiff line numberDiff line change
@@ -484,11 +484,18 @@ eliminateSimpleBorrows(BeginBorrowInst *bbi, CanonicalizeInstruction &pass) {
484484
if (bbi->isLexical() && (bbi->getModule().getStage() == SILStage::Raw ||
485485
!isNestedLexicalBeginBorrow(bbi)))
486486
return next;
487+
488+
// Fixed borrow scopes can't be eliminated during the raw stage since they
489+
// control move checker behavior.
490+
if (bbi->isFixed() && bbi->getModule().getStage() == SILStage::Raw) {
491+
return next;
492+
}
487493

488494
// Borrow scopes representing a VarDecl can't be eliminated during the raw
489495
// stage because they may be needed for diagnostics.
490-
if (bbi->isFromVarDecl() && (bbi->getModule().getStage() == SILStage::Raw))
496+
if (bbi->isFromVarDecl() && bbi->getModule().getStage() == SILStage::Raw) {
491497
return next;
498+
}
492499

493500
// We know that our borrow is completely within the lifetime of its base value
494501
// if the borrow is never reborrowed. We check for reborrows and do not

‎lib/Serialization/DeserializeSIL.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -2225,11 +2225,12 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn,
22252225
bool isLexical = Attr & 0x1;
22262226
bool hasPointerEscape = (Attr >> 1) & 0x1;
22272227
bool fromVarDecl = (Attr >> 2) & 0x1;
2228+
bool isFixed = (Attr >> 3) & 0x1;
22282229
ResultInst = Builder.createBeginBorrow(
22292230
Loc,
22302231
getLocalValue(ValID, getSILType(MF->getType(TyID),
22312232
(SILValueCategory)TyCategory, Fn)),
2232-
isLexical, hasPointerEscape, fromVarDecl);
2233+
isLexical, hasPointerEscape, fromVarDecl, isFixed);
22332234
break;
22342235
}
22352236

‎lib/Serialization/SerializeSIL.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -1572,7 +1572,8 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) {
15721572
} else if (auto *BBI = dyn_cast<BeginBorrowInst>(&SI)) {
15731573
Attr = unsigned(BBI->isLexical()) |
15741574
(unsigned(BBI->hasPointerEscape() << 1)) |
1575-
(unsigned(BBI->isFromVarDecl() << 2));
1575+
(unsigned(BBI->isFromVarDecl() << 2)) |
1576+
(unsigned(BBI->isFixed() << 3));
15761577
} else if (auto *MVI = dyn_cast<MoveValueInst>(&SI)) {
15771578
Attr = unsigned(MVI->getAllowDiagnostics()) |
15781579
(unsigned(MVI->isLexical() << 1)) |

‎test/SILGen/borrowing_switch_subjects.swift

+5-3
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ func borrowParam(x: borrowing Outer) {
3434

3535
// CHECK: [[BORROW_OUTER:%.*]] = begin_borrow {{.*}} : $Outer
3636
// CHECK: [[BORROW_INNER:%.*]] = struct_extract [[BORROW_OUTER]]
37-
// CHECK: [[COPY_INNER:%.*]] = copy_value [[BORROW_INNER]]
37+
// CHECK: [[BORROW_FIX:%.*]] = begin_borrow [fixed] [[BORROW_INNER]]
38+
// CHECK: [[COPY_INNER:%.*]] = copy_value [[BORROW_FIX]]
3839
// CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [strict] [no_consume_or_assign] [[COPY_INNER]]
3940
// CHECK: [[BORROW:%.*]] = begin_borrow [[MARK]]
4041
switch x.storedInner {
@@ -50,7 +51,8 @@ func borrowParam(x: borrowing Outer) {
5051
// CHECK: [[COPY_INNER:%.*]] = copy_value [[BORROW_INNER]]
5152
// CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[COPY_INNER]]
5253
// CHECK: [[BORROW:%.*]] = begin_borrow [[MARK]]
53-
// CHECK: [[COPY2:%.*]] = copy_value [[BORROW]]
54+
// CHECK: [[BORROW_FIX:%.*]] = begin_borrow [fixed] [[BORROW]]
55+
// CHECK: [[COPY2:%.*]] = copy_value [[BORROW_FIX]]
5456
// CHECK: [[MARK2:%.*]] = mark_unresolved_non_copyable_value [strict] [no_consume_or_assign] [[COPY2]]
5557
// CHECK: [[BORROW2:%.*]] = begin_borrow [[MARK2]]
5658
switch x.readInner {
@@ -63,7 +65,7 @@ func borrowParam(x: borrowing Outer) {
6365

6466
// CHECK: [[FN:%.*]] = function_ref @{{.*}}9temporary
6567
// CHECK: [[TMP:%.*]] = apply [[FN]]()
66-
// CHECK: [[BORROW_OUTER:%.*]] = begin_borrow [[TMP]]
68+
// CHECK: [[BORROW_OUTER:%.*]] = begin_borrow [fixed] [[TMP]]
6769
// CHECK: [[COPY:%.*]] = copy_value [[BORROW_OUTER]]
6870
// CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [strict] [no_consume_or_assign] [[COPY]]
6971
// CHECK: [[BORROW:%.*]] = begin_borrow [[MARK]]

‎test/SILOptimizer/moveonly_borrowing_switch_yield.swift

+6-3
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
extension List {
44
var peek: Element {
55
_read {
6-
// TODO: fix move-only checker induced ownership bug when
7-
// we try to switch over `self.head` here.
8-
yield head.peek
6+
switch self.head {
7+
case .empty: fatalError()
8+
case .more(_borrowing box):
9+
yield box.wrapped.element
10+
}
11+
//yield head.peek
912
}
1013
}
1114
}

0 commit comments

Comments
 (0)