Skip to content

[6.0] Use the BorrowingSwitch implementation for all noncopyable switches. #72966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 11, 2024
Merged

[6.0] Use the BorrowingSwitch implementation for all noncopyable switches. #72966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffdb5db
Optimizer/OptUtils: Don't trivially remove dead-looking `unchecked_en…
jckarter Apr 9, 2024
e8b0398
Use the `BorrowingSwitch` implementation for all noncopyable switches.
jckarter Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,10 @@ extension Instruction {
if bi.id == .OnFastPath {
return false
}
case is UncheckedEnumDataInst:
// Don't remove UncheckedEnumDataInst in OSSA in case it is responsible
// for consuming an enum value.
return !parentFunction.hasOwnership
default:
break
}
Expand Down
206 changes: 101 additions & 105 deletions lib/SILGen/SILGenPattern.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3352,21 +3352,19 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
// the match, we'll have to perform the initial match as a borrow and then
// reproject the value to consume it.
auto ownership = ValueOwnership::Default;
if (getASTContext().LangOpts.hasFeature(Feature::BorrowingSwitch)) {
if (subjectTy->isNoncopyable()) {
// Determine the overall ownership behavior of the switch, based on the
// subject expression and the patterns' ownership behavior.

// If the subject expression is borrowable, then perform the switch as
// a borrow. (A `consume` expression would render the expression
// non-borrowable.) Otherwise, perform it as a consume.
ownership = isBorrowableSubject(*this, subjectExpr)
? ValueOwnership::Shared
: ValueOwnership::Owned;
for (auto caseLabel : S->getCases()) {
for (auto item : caseLabel->getCaseLabelItems()) {
ownership = std::max(ownership, item.getPattern()->getOwnership());
}
if (subjectTy->isNoncopyable()) {
// Determine the overall ownership behavior of the switch, based on the
// subject expression and the patterns' ownership behavior.

// If the subject expression is borrowable, then perform the switch as
// a borrow. (A `consume` expression would render the expression
// non-borrowable.) Otherwise, perform it as a consume.
ownership = isBorrowableSubject(*this, subjectExpr)
? ValueOwnership::Shared
: ValueOwnership::Owned;
for (auto caseLabel : S->getCases()) {
for (auto item : caseLabel->getCaseLabelItems()) {
ownership = std::max(ownership, item.getPattern()->getOwnership());
}
}
}
Expand Down Expand Up @@ -3505,103 +3503,101 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {

// Inline constructor for subject.
auto subject = ([&]() -> ConsumableManagedValue {
if (subjectMV.getType().isMoveOnly()) {
if (ownership > ValueOwnership::Default) { // should become an assert when BorrowingSwitch is enabled
// Based on the ownership behavior, prepare the subject.
// The pattern match itself will always be performed on a borrow, to
// ensure that the order of pattern evaluation doesn't prematurely
// consume or modify the value until we commit to a match. But if the
// match consumes the value, then we need a +1 value to go back to in
// order to consume the parts we match to, so we force a +1 value then
// borrow that for the pattern match.
switch (ownership) {
case ValueOwnership::Default:
llvm_unreachable("invalid");
// TODO: Move-only-wrapped subjects should also undergo a noncopying switch.
if (subjectMV.getType().isMoveOnly(/*or wrapped*/ false)) {
assert(ownership > ValueOwnership::Default);
// Based on the ownership behavior, prepare the subject.
// The pattern match itself will always be performed on a borrow, to
// ensure that the order of pattern evaluation doesn't prematurely
// consume or modify the value until we commit to a match. But if the
// match consumes the value, then we need a +1 value to go back to in
// order to consume the parts we match to, so we force a +1 value then
// borrow that for the pattern match.
switch (ownership) {
case ValueOwnership::Default:
llvm_unreachable("invalid");

case ValueOwnership::Shared:
emission.setNoncopyableBorrowingOwnership();
if (subjectMV.getType().isAddress()) {
// Initiate a read access on the memory, to ensure that even
// if the underlying memory is mutable or consumable, the pattern
// match is not allowed to modify it.
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
if (subjectMV.getType().isLoadable(F)) {
// Load a borrow if the type is loadable.
subjectMV = subjectUndergoesFormalAccess
? B.createFormalAccessLoadBorrow(S, subjectMV)
: B.createLoadBorrow(S, subjectMV);
}
} else {
// Initiate a fixed borrow on the subject, so that it's treated as
// opaque by the move checker.
subjectMV =
subjectUndergoesFormalAccess
? B.createFormalAccessBeginBorrow(
S, subjectMV, IsNotLexical, BeginBorrowInst::IsFixed)
: B.createBeginBorrow(S, subjectMV, IsNotLexical,
BeginBorrowInst::IsFixed);
}
return {subjectMV, CastConsumptionKind::BorrowAlways};

case ValueOwnership::Shared:
emission.setNoncopyableBorrowingOwnership();
if (subjectMV.getType().isAddress()) {
// Initiate a read access on the memory, to ensure that even
// if the underlying memory is mutable or consumable, the pattern
// match is not allowed to modify it.
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
if (subjectMV.getType().isLoadable(F)) {
// Load a borrow if the type is loadable.
subjectMV = subjectUndergoesFormalAccess
? B.createFormalAccessLoadBorrow(S, subjectMV)
: B.createLoadBorrow(S, subjectMV);
}
} else {
// Initiate a fixed borrow on the subject, so that it's treated as
// opaque by the move checker.
subjectMV =
subjectUndergoesFormalAccess
? B.createFormalAccessBeginBorrow(
S, subjectMV, IsNotLexical, BeginBorrowInst::IsFixed)
: B.createBeginBorrow(S, subjectMV, IsNotLexical,
case ValueOwnership::InOut:
// TODO: mutating switches
llvm_unreachable("not implemented");

case ValueOwnership::Owned:
// Make sure we own the subject value.
subjectMV = subjectMV.ensurePlusOne(*this, S);
if (subjectMV.getType().isAddress() &&
subjectMV.getType().isLoadable(F)) {
// Move the value into memory if it's loadable.
subjectMV = B.createLoadTake(S, subjectMV);
}

// Unwind cleanups to this point when we're ready to reproject
// the value for consumption or mutation.
emission.setNoncopyableConsumingOwnership(
Cleanups.getCleanupsDepth(),
subjectMV);

// Perform the pattern match on an opaque borrow or read access of the
// subject.
if (subjectMV.getType().isAddress()) {
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
} else {
subjectMV = B.createBeginBorrow(S, subjectMV, IsNotLexical,
BeginBorrowInst::IsFixed);
}
return {subjectMV, CastConsumptionKind::BorrowAlways};

case ValueOwnership::InOut:
// TODO: mutating switches
llvm_unreachable("not implemented");

case ValueOwnership::Owned:
// Make sure we own the subject value.
subjectMV = subjectMV.ensurePlusOne(*this, S);
if (subjectMV.getType().isAddress() &&
subjectMV.getType().isLoadable(F)) {
// Move the value into memory if it's loadable.
subjectMV = B.createLoadTake(S, subjectMV);
}

// Unwind cleanups to this point when we're ready to reproject
// the value for consumption or mutation.
emission.setNoncopyableConsumingOwnership(
Cleanups.getCleanupsDepth(),
subjectMV);

// Perform the pattern match on an opaque borrow or read access of the
// subject.
if (subjectMV.getType().isAddress()) {
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
} else {
subjectMV = B.createBeginBorrow(S, subjectMV, IsNotLexical,
BeginBorrowInst::IsFixed);
}
return {subjectMV, CastConsumptionKind::BorrowAlways};
}
llvm_unreachable("unhandled value ownership");
return {subjectMV, CastConsumptionKind::BorrowAlways};
}
llvm_unreachable("unhandled value ownership");
}

// TODO: Move-only-wrapped subjects should also undergo a noncopying switch.
// For now, unwrap them and perform a normal switch over them.
if (subjectMV.getType().isMoveOnlyWrapped()) {
if (subjectMV.getType().isAddress()) {
auto isPlusOne = subjectMV.isPlusOne(*this);
auto subjectAddr
= B.createMoveOnlyWrapperToCopyableAddr(S, subjectMV.forward(*this));

if (isPlusOne) {
subjectMV = emitManagedRValueWithCleanup(subjectAddr);
} else {
subjectMV = ManagedValue::forBorrowedAddressRValue(subjectAddr);
}
} else {
// TODO: Remove.
// If we have a noImplicitCopy value, ensure plus one and convert
// it. Switches always consume move only values.
//
// NOTE: We purposely do not do this for pure move only types since for them
// we emit everything at +0 and then run the BorrowToDestructure transform
// upon them. The reason that we do this is that internally to
// SILGenPattern, we always attempt to move from +1 -> +0 meaning that even
// if we start at +1, we will go back to +0 given enough patterns to go
// through. It is simpler to just let SILGenPattern do what it already wants
// to do, rather than fight it or try to resusitate the "fake owned borrow"
// path that we still use for address only types (and that we want to delete
// once we have opaque values).
if (subjectMV.getType().isObject()) {
if (subjectMV.getType().isMoveOnlyWrapped()) {
subjectMV = B.createOwnedMoveOnlyWrapperToCopyableValue(
S, subjectMV.ensurePlusOne(*this, S));
} else {
// If we have a pure move only type and it is owned, borrow it so that
// BorrowToDestructure can handle it.
if (subjectMV.getOwnershipKind() == OwnershipKind::Owned) {
subjectMV = subjectMV.borrow(*this, S);
}
}
if (subjectMV.isPlusOne(*this)) {
subjectMV
= B.createOwnedMoveOnlyWrapperToCopyableValue(S, subjectMV);
} else {
subjectMV
= B.createGuaranteedMoveOnlyWrapperToCopyableValue(S, subjectMV);
}
}
}

// If we have a plus one value...
if (subjectMV.isPlusOne(*this)) {
// And we have an address that is loadable, perform a load [take].
Expand Down
Loading