Skip to content

Commit bdb71e0

Browse files
authored
Merge pull request #72966 from jckarter/enable-borrowing-switch-backend-6.0
[6.0] Use the BorrowingSwitch implementation for all noncopyable switches.
2 parents 66e7c6a + e8b0398 commit bdb71e0

15 files changed

+199
-883
lines changed

SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,10 @@ extension Instruction {
289289
if bi.id == .OnFastPath {
290290
return false
291291
}
292+
case is UncheckedEnumDataInst:
293+
// Don't remove UncheckedEnumDataInst in OSSA in case it is responsible
294+
// for consuming an enum value.
295+
return !parentFunction.hasOwnership
292296
default:
293297
break
294298
}

lib/SILGen/SILGenPattern.cpp

Lines changed: 101 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -3352,21 +3352,19 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
33523352
// the match, we'll have to perform the initial match as a borrow and then
33533353
// reproject the value to consume it.
33543354
auto ownership = ValueOwnership::Default;
3355-
if (getASTContext().LangOpts.hasFeature(Feature::BorrowingSwitch)) {
3356-
if (subjectTy->isNoncopyable()) {
3357-
// Determine the overall ownership behavior of the switch, based on the
3358-
// subject expression and the patterns' ownership behavior.
3359-
3360-
// If the subject expression is borrowable, then perform the switch as
3361-
// a borrow. (A `consume` expression would render the expression
3362-
// non-borrowable.) Otherwise, perform it as a consume.
3363-
ownership = isBorrowableSubject(*this, subjectExpr)
3364-
? ValueOwnership::Shared
3365-
: ValueOwnership::Owned;
3366-
for (auto caseLabel : S->getCases()) {
3367-
for (auto item : caseLabel->getCaseLabelItems()) {
3368-
ownership = std::max(ownership, item.getPattern()->getOwnership());
3369-
}
3355+
if (subjectTy->isNoncopyable()) {
3356+
// Determine the overall ownership behavior of the switch, based on the
3357+
// subject expression and the patterns' ownership behavior.
3358+
3359+
// If the subject expression is borrowable, then perform the switch as
3360+
// a borrow. (A `consume` expression would render the expression
3361+
// non-borrowable.) Otherwise, perform it as a consume.
3362+
ownership = isBorrowableSubject(*this, subjectExpr)
3363+
? ValueOwnership::Shared
3364+
: ValueOwnership::Owned;
3365+
for (auto caseLabel : S->getCases()) {
3366+
for (auto item : caseLabel->getCaseLabelItems()) {
3367+
ownership = std::max(ownership, item.getPattern()->getOwnership());
33703368
}
33713369
}
33723370
}
@@ -3505,103 +3503,101 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
35053503

35063504
// Inline constructor for subject.
35073505
auto subject = ([&]() -> ConsumableManagedValue {
3508-
if (subjectMV.getType().isMoveOnly()) {
3509-
if (ownership > ValueOwnership::Default) { // should become an assert when BorrowingSwitch is enabled
3510-
// Based on the ownership behavior, prepare the subject.
3511-
// The pattern match itself will always be performed on a borrow, to
3512-
// ensure that the order of pattern evaluation doesn't prematurely
3513-
// consume or modify the value until we commit to a match. But if the
3514-
// match consumes the value, then we need a +1 value to go back to in
3515-
// order to consume the parts we match to, so we force a +1 value then
3516-
// borrow that for the pattern match.
3517-
switch (ownership) {
3518-
case ValueOwnership::Default:
3519-
llvm_unreachable("invalid");
3506+
// TODO: Move-only-wrapped subjects should also undergo a noncopying switch.
3507+
if (subjectMV.getType().isMoveOnly(/*or wrapped*/ false)) {
3508+
assert(ownership > ValueOwnership::Default);
3509+
// Based on the ownership behavior, prepare the subject.
3510+
// The pattern match itself will always be performed on a borrow, to
3511+
// ensure that the order of pattern evaluation doesn't prematurely
3512+
// consume or modify the value until we commit to a match. But if the
3513+
// match consumes the value, then we need a +1 value to go back to in
3514+
// order to consume the parts we match to, so we force a +1 value then
3515+
// borrow that for the pattern match.
3516+
switch (ownership) {
3517+
case ValueOwnership::Default:
3518+
llvm_unreachable("invalid");
3519+
3520+
case ValueOwnership::Shared:
3521+
emission.setNoncopyableBorrowingOwnership();
3522+
if (subjectMV.getType().isAddress()) {
3523+
// Initiate a read access on the memory, to ensure that even
3524+
// if the underlying memory is mutable or consumable, the pattern
3525+
// match is not allowed to modify it.
3526+
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
3527+
if (subjectMV.getType().isLoadable(F)) {
3528+
// Load a borrow if the type is loadable.
3529+
subjectMV = subjectUndergoesFormalAccess
3530+
? B.createFormalAccessLoadBorrow(S, subjectMV)
3531+
: B.createLoadBorrow(S, subjectMV);
3532+
}
3533+
} else {
3534+
// Initiate a fixed borrow on the subject, so that it's treated as
3535+
// opaque by the move checker.
3536+
subjectMV =
3537+
subjectUndergoesFormalAccess
3538+
? B.createFormalAccessBeginBorrow(
3539+
S, subjectMV, IsNotLexical, BeginBorrowInst::IsFixed)
3540+
: B.createBeginBorrow(S, subjectMV, IsNotLexical,
3541+
BeginBorrowInst::IsFixed);
3542+
}
3543+
return {subjectMV, CastConsumptionKind::BorrowAlways};
35203544

3521-
case ValueOwnership::Shared:
3522-
emission.setNoncopyableBorrowingOwnership();
3523-
if (subjectMV.getType().isAddress()) {
3524-
// Initiate a read access on the memory, to ensure that even
3525-
// if the underlying memory is mutable or consumable, the pattern
3526-
// match is not allowed to modify it.
3527-
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
3528-
if (subjectMV.getType().isLoadable(F)) {
3529-
// Load a borrow if the type is loadable.
3530-
subjectMV = subjectUndergoesFormalAccess
3531-
? B.createFormalAccessLoadBorrow(S, subjectMV)
3532-
: B.createLoadBorrow(S, subjectMV);
3533-
}
3534-
} else {
3535-
// Initiate a fixed borrow on the subject, so that it's treated as
3536-
// opaque by the move checker.
3537-
subjectMV =
3538-
subjectUndergoesFormalAccess
3539-
? B.createFormalAccessBeginBorrow(
3540-
S, subjectMV, IsNotLexical, BeginBorrowInst::IsFixed)
3541-
: B.createBeginBorrow(S, subjectMV, IsNotLexical,
3545+
case ValueOwnership::InOut:
3546+
// TODO: mutating switches
3547+
llvm_unreachable("not implemented");
3548+
3549+
case ValueOwnership::Owned:
3550+
// Make sure we own the subject value.
3551+
subjectMV = subjectMV.ensurePlusOne(*this, S);
3552+
if (subjectMV.getType().isAddress() &&
3553+
subjectMV.getType().isLoadable(F)) {
3554+
// Move the value into memory if it's loadable.
3555+
subjectMV = B.createLoadTake(S, subjectMV);
3556+
}
3557+
3558+
// Unwind cleanups to this point when we're ready to reproject
3559+
// the value for consumption or mutation.
3560+
emission.setNoncopyableConsumingOwnership(
3561+
Cleanups.getCleanupsDepth(),
3562+
subjectMV);
3563+
3564+
// Perform the pattern match on an opaque borrow or read access of the
3565+
// subject.
3566+
if (subjectMV.getType().isAddress()) {
3567+
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
3568+
} else {
3569+
subjectMV = B.createBeginBorrow(S, subjectMV, IsNotLexical,
35423570
BeginBorrowInst::IsFixed);
3543-
}
3544-
return {subjectMV, CastConsumptionKind::BorrowAlways};
3545-
3546-
case ValueOwnership::InOut:
3547-
// TODO: mutating switches
3548-
llvm_unreachable("not implemented");
3549-
3550-
case ValueOwnership::Owned:
3551-
// Make sure we own the subject value.
3552-
subjectMV = subjectMV.ensurePlusOne(*this, S);
3553-
if (subjectMV.getType().isAddress() &&
3554-
subjectMV.getType().isLoadable(F)) {
3555-
// Move the value into memory if it's loadable.
3556-
subjectMV = B.createLoadTake(S, subjectMV);
3557-
}
3558-
3559-
// Unwind cleanups to this point when we're ready to reproject
3560-
// the value for consumption or mutation.
3561-
emission.setNoncopyableConsumingOwnership(
3562-
Cleanups.getCleanupsDepth(),
3563-
subjectMV);
3564-
3565-
// Perform the pattern match on an opaque borrow or read access of the
3566-
// subject.
3567-
if (subjectMV.getType().isAddress()) {
3568-
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
3569-
} else {
3570-
subjectMV = B.createBeginBorrow(S, subjectMV, IsNotLexical,
3571-
BeginBorrowInst::IsFixed);
3572-
}
3573-
return {subjectMV, CastConsumptionKind::BorrowAlways};
35743571
}
3575-
llvm_unreachable("unhandled value ownership");
3572+
return {subjectMV, CastConsumptionKind::BorrowAlways};
3573+
}
3574+
llvm_unreachable("unhandled value ownership");
3575+
}
3576+
3577+
// TODO: Move-only-wrapped subjects should also undergo a noncopying switch.
3578+
// For now, unwrap them and perform a normal switch over them.
3579+
if (subjectMV.getType().isMoveOnlyWrapped()) {
3580+
if (subjectMV.getType().isAddress()) {
3581+
auto isPlusOne = subjectMV.isPlusOne(*this);
3582+
auto subjectAddr
3583+
= B.createMoveOnlyWrapperToCopyableAddr(S, subjectMV.forward(*this));
3584+
3585+
if (isPlusOne) {
3586+
subjectMV = emitManagedRValueWithCleanup(subjectAddr);
3587+
} else {
3588+
subjectMV = ManagedValue::forBorrowedAddressRValue(subjectAddr);
3589+
}
35763590
} else {
3577-
// TODO: Remove.
3578-
// If we have a noImplicitCopy value, ensure plus one and convert
3579-
// it. Switches always consume move only values.
3580-
//
3581-
// NOTE: We purposely do not do this for pure move only types since for them
3582-
// we emit everything at +0 and then run the BorrowToDestructure transform
3583-
// upon them. The reason that we do this is that internally to
3584-
// SILGenPattern, we always attempt to move from +1 -> +0 meaning that even
3585-
// if we start at +1, we will go back to +0 given enough patterns to go
3586-
// through. It is simpler to just let SILGenPattern do what it already wants
3587-
// to do, rather than fight it or try to resusitate the "fake owned borrow"
3588-
// path that we still use for address only types (and that we want to delete
3589-
// once we have opaque values).
3590-
if (subjectMV.getType().isObject()) {
3591-
if (subjectMV.getType().isMoveOnlyWrapped()) {
3592-
subjectMV = B.createOwnedMoveOnlyWrapperToCopyableValue(
3593-
S, subjectMV.ensurePlusOne(*this, S));
3594-
} else {
3595-
// If we have a pure move only type and it is owned, borrow it so that
3596-
// BorrowToDestructure can handle it.
3597-
if (subjectMV.getOwnershipKind() == OwnershipKind::Owned) {
3598-
subjectMV = subjectMV.borrow(*this, S);
3599-
}
3600-
}
3591+
if (subjectMV.isPlusOne(*this)) {
3592+
subjectMV
3593+
= B.createOwnedMoveOnlyWrapperToCopyableValue(S, subjectMV);
3594+
} else {
3595+
subjectMV
3596+
= B.createGuaranteedMoveOnlyWrapperToCopyableValue(S, subjectMV);
36013597
}
36023598
}
36033599
}
3604-
3600+
36053601
// If we have a plus one value...
36063602
if (subjectMV.isPlusOne(*this)) {
36073603
// And we have an address that is loadable, perform a load [take].

0 commit comments

Comments
 (0)