Skip to content

SIL: Treat store_borrow as borrowing its source, and have the move-only checker account for borrow scopes. #69169

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 1 commit into from
Oct 16, 2023
Merged

SIL: Treat store_borrow as borrowing its source, and have the move-only checker account for borrow scopes. #69169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 6 additions & 9 deletions include/swift/SIL/OwnershipUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ class BorrowingOperandKind {
enum Kind : uint8_t {
Invalid = 0,
BeginBorrow,
StoreBorrow,
BeginApply,
Branch,
Apply,
Expand All @@ -277,6 +278,8 @@ class BorrowingOperandKind {
return Kind::Invalid;
case SILInstructionKind::BeginBorrowInst:
return Kind::BeginBorrow;
case SILInstructionKind::StoreBorrowInst:
return Kind::StoreBorrow;
case SILInstructionKind::BeginApplyInst:
return Kind::BeginApply;
case SILInstructionKind::BranchInst:
Expand Down Expand Up @@ -386,6 +389,7 @@ struct BorrowingOperand {
case BorrowingOperandKind::Invalid:
llvm_unreachable("Using invalid case?!");
case BorrowingOperandKind::BeginBorrow:
case BorrowingOperandKind::StoreBorrow:
case BorrowingOperandKind::BeginApply:
case BorrowingOperandKind::Apply:
case BorrowingOperandKind::TryApply:
Expand Down Expand Up @@ -418,6 +422,7 @@ struct BorrowingOperand {
case BorrowingOperandKind::BeginBorrow:
case BorrowingOperandKind::Branch:
return true;
case BorrowingOperandKind::StoreBorrow:
case BorrowingOperandKind::BeginApply:
case BorrowingOperandKind::Apply:
case BorrowingOperandKind::TryApply:
Expand Down Expand Up @@ -790,7 +795,6 @@ class InteriorPointerOperandKind {
RefTailAddr,
OpenExistentialBox,
ProjectBox,
StoreBorrow,
};

private:
Expand Down Expand Up @@ -819,8 +823,6 @@ class InteriorPointerOperandKind {
return Kind::OpenExistentialBox;
case SILInstructionKind::ProjectBoxInst:
return Kind::ProjectBox;
case SILInstructionKind::StoreBorrowInst:
return Kind::StoreBorrow;
}
}

Expand All @@ -839,8 +841,6 @@ class InteriorPointerOperandKind {
return Kind::OpenExistentialBox;
case ValueKind::ProjectBoxInst:
return Kind::ProjectBox;
case ValueKind::StoreBorrowInst:
return Kind::StoreBorrow;
}
}

Expand Down Expand Up @@ -897,8 +897,7 @@ struct InteriorPointerOperand {
case InteriorPointerOperandKind::RefElementAddr:
case InteriorPointerOperandKind::RefTailAddr:
case InteriorPointerOperandKind::OpenExistentialBox:
case InteriorPointerOperandKind::ProjectBox:
case InteriorPointerOperandKind::StoreBorrow: {
case InteriorPointerOperandKind::ProjectBox: {
// Ok, we have a valid instruction. Return the relevant operand.
auto *op =
&cast<SingleValueInstruction>(resultValue)->getAllOperands()[0];
Expand Down Expand Up @@ -941,8 +940,6 @@ struct InteriorPointerOperand {
return cast<OpenExistentialBoxInst>(operand->getUser());
case InteriorPointerOperandKind::ProjectBox:
return cast<ProjectBoxInst>(operand->getUser());
case InteriorPointerOperandKind::StoreBorrow:
return cast<StoreBorrowInst>(operand->getUser());
}
llvm_unreachable("Covered switch isn't covered?!");
}
Expand Down
2 changes: 1 addition & 1 deletion lib/SIL/IR/OperandOwnership.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ OperandOwnership OperandOwnershipClassifier::visitBranchInst(BranchInst *bi) {
OperandOwnership
OperandOwnershipClassifier::visitStoreBorrowInst(StoreBorrowInst *i) {
if (getValue() == i->getSrc()) {
return OperandOwnership::InteriorPointer;
return OperandOwnership::Borrow;
}
return OperandOwnership::TrivialUse;
}
Expand Down
20 changes: 20 additions & 0 deletions lib/SIL/Utils/OwnershipUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,9 @@ void BorrowingOperandKind::print(llvm::raw_ostream &os) const {
case Kind::BeginBorrow:
os << "BeginBorrow";
return;
case Kind::StoreBorrow:
os << "StoreBorrow";
return;
case Kind::BeginApply:
os << "BeginApply";
return;
Expand Down Expand Up @@ -649,6 +652,7 @@ bool BorrowingOperand::hasEmptyRequiredEndingUses() const {
case BorrowingOperandKind::Invalid:
llvm_unreachable("Using invalid case");
case BorrowingOperandKind::BeginBorrow:
case BorrowingOperandKind::StoreBorrow:
case BorrowingOperandKind::BeginApply:
case BorrowingOperandKind::BeginAsyncLet:
case BorrowingOperandKind::PartialApplyStack: {
Expand Down Expand Up @@ -687,6 +691,20 @@ bool BorrowingOperand::visitScopeEndingUses(
// false.
return !deadBorrow;
}
case BorrowingOperandKind::StoreBorrow: {
bool deadBorrow = true;
for (auto *use : cast<StoreBorrowInst>(op->getUser())->getUses()) {
if (isa<EndBorrowInst>(use->getUser())) {
deadBorrow = false;
if (!func(use))
return false;
}
}
// FIXME: special case for dead borrows. This is dangerous because clients
// only expect visitScopeEndingUses to return false if the visitor returned
// false.
return !deadBorrow;
}
case BorrowingOperandKind::BeginApply: {
bool deadApply = true;
auto *user = cast<BeginApplyInst>(op->getUser());
Expand Down Expand Up @@ -763,6 +781,7 @@ BorrowedValue BorrowingOperand::getBorrowIntroducingUserResult() const {
case BorrowingOperandKind::Yield:
case BorrowingOperandKind::PartialApplyStack:
case BorrowingOperandKind::BeginAsyncLet:
case BorrowingOperandKind::StoreBorrow:
return BorrowedValue();

case BorrowingOperandKind::BeginBorrow: {
Expand Down Expand Up @@ -792,6 +811,7 @@ SILValue BorrowingOperand::getScopeIntroducingUserResult() {
case BorrowingOperandKind::BeginAsyncLet:
case BorrowingOperandKind::PartialApplyStack:
case BorrowingOperandKind::BeginBorrow:
case BorrowingOperandKind::StoreBorrow:
return cast<SingleValueInstruction>(op->getUser());

case BorrowingOperandKind::BeginApply:
Expand Down
87 changes: 70 additions & 17 deletions lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,17 @@ bool Implementation::gatherUses(SILValue value) {
false /*is lifetime ending*/}});
liveness.updateForUse(nextUse->getUser(), *leafRange,
false /*is lifetime ending*/);
// The liveness extends to the scope-ending uses of the borrow.
BorrowingOperand(nextUse).visitScopeEndingUses([&](Operand *end) -> bool {
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
return false;
}
LLVM_DEBUG(llvm::dbgs() << " ++ Scope-ending use: ";
end->getUser()->print(llvm::dbgs()));
liveness.updateForUse(end->getUser(), *leafRange,
false /*is lifetime ending*/);
return true;
});
instToInterestingOperandIndexMap.insert(nextUse->getUser(), nextUse);

continue;
Expand Down Expand Up @@ -1001,6 +1012,48 @@ dumpIntervalMap(IntervalMapAllocator::Map &map) {
}
#endif

// Helper to insert end_borrows after the end of a non-consuming use. If the
// use is momentary, one end_borrow is inserted after the use. If it is an
// interior pointer projection or nested borrow, then end_borrows are inserted
// after every scope-ending instruction for the use.
static void insertEndBorrowsForNonConsumingUse(Operand *op,
SILValue borrow) {
if (auto iOp = InteriorPointerOperand::get(op)) {
LLVM_DEBUG(llvm::dbgs() << " -- Ending borrow after interior pointer scope:\n"
" ";
op->getUser()->print(llvm::dbgs()));
iOp.visitBaseValueScopeEndingUses([&](Operand *endScope) -> bool {
auto *endScopeInst = endScope->getUser();
LLVM_DEBUG(llvm::dbgs() << " ";
endScopeInst->print(llvm::dbgs()));
SILBuilderWithScope endBuilder(endScopeInst);
endBuilder.createEndBorrow(getSafeLoc(endScopeInst), borrow);
return true;
});
} else if (auto bOp = BorrowingOperand(op)) {
LLVM_DEBUG(llvm::dbgs() << " -- Ending borrow after borrow scope:\n"
" ";
op->getUser()->print(llvm::dbgs()));
bOp.visitScopeEndingUses([&](Operand *endScope) -> bool {
auto *endScopeInst = endScope->getUser();
LLVM_DEBUG(llvm::dbgs() << " ";
endScopeInst->print(llvm::dbgs()));
auto afterScopeInst = endScopeInst->getNextInstruction();
SILBuilderWithScope endBuilder(afterScopeInst);
endBuilder.createEndBorrow(getSafeLoc(afterScopeInst),
borrow);
return true;
});
} else {
auto *nextInst = op->getUser()->getNextInstruction();
LLVM_DEBUG(llvm::dbgs() << " -- Ending borrow after momentary use at: ";
nextInst->print(llvm::dbgs()));
SILBuilderWithScope endBuilder(nextInst);
endBuilder.createEndBorrow(getSafeLoc(nextInst), borrow);
}

}

void Implementation::rewriteUses(InstructionDeleter *deleter) {
blocksToUses.setFrozen();

Expand Down Expand Up @@ -1129,6 +1182,8 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
}

// Otherwise, we need to insert a borrow.
LLVM_DEBUG(llvm::dbgs() << " Inserting borrow for: ";
inst->print(llvm::dbgs()));
SILBuilderWithScope borrowBuilder(inst);
SILValue borrow =
borrowBuilder.createBeginBorrow(getSafeLoc(inst), first);
Expand All @@ -1138,21 +1193,10 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
borrowBuilder.createGuaranteedMoveOnlyWrapperToCopyableValue(
getSafeLoc(inst), innerValue);
}

insertEndBorrowsForNonConsumingUse(&operand, borrow);

if (auto op = InteriorPointerOperand::get(&operand)) {
op.visitBaseValueScopeEndingUses([&](Operand *endScope) -> bool {
auto *endScopeInst = endScope->getUser();
SILBuilderWithScope endBuilder(endScopeInst);
endBuilder.createEndBorrow(getSafeLoc(endScopeInst), borrow);
return true;
});
} else {
auto *nextInst = inst->getNextInstruction();
SILBuilderWithScope endBuilder(nextInst);
endBuilder.createEndBorrow(getSafeLoc(nextInst), borrow);
}

// NOTE: This needs to be /after/the interior pointer operand usage
// NOTE: This needs to be /after/ the interior pointer operand usage
// above so that we can use the end scope of our interior pointer base
// value.
// NOTE: oldInst may be nullptr if our operand is a SILArgument
Expand Down Expand Up @@ -1245,9 +1289,7 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
}
}

auto *nextInst = inst->getNextInstruction();
SILBuilderWithScope endBuilder(nextInst);
endBuilder.createEndBorrow(getSafeLoc(nextInst), borrow);
insertEndBorrowsForNonConsumingUse(&operand, borrow);
continue;
}

Expand Down Expand Up @@ -1333,6 +1375,13 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
}

void Implementation::cleanup() {
LLVM_DEBUG(llvm::dbgs()
<< "Performing BorrowToDestructureTransform::cleanup()!\n");
SWIFT_DEFER {
LLVM_DEBUG(llvm::dbgs() << "Function after cleanup!\n";
getMarkedValue()->getFunction()->dump());
};

// Then add destroys for any destructure elements that we inserted that we did
// not actually completely consume.
auto *fn = getMarkedValue()->getFunction();
Expand Down Expand Up @@ -1793,11 +1842,15 @@ bool BorrowToDestructureTransform::transform() {
// Then clean up all of our borrows/copies/struct_extracts which no longer
// have any uses...
{
LLVM_DEBUG(llvm::dbgs() << "Deleting dead instructions!\n");

InstructionDeleter deleter;
while (!borrowWorklist.empty()) {
deleter.recursivelyForceDeleteUsersAndFixLifetimes(
borrowWorklist.pop_back_val());
}
LLVM_DEBUG(llvm::dbgs() << "Function after deletion!\n";
impl.getMarkedValue()->getFunction()->dump());
}

return true;
Expand Down
1 change: 0 additions & 1 deletion lib/SILOptimizer/SILCombiner/SILCombinerBuiltinVisitors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ SILCombiner::optimizeBuiltinCOWBufferForReadingOSSA(BuiltinInst *bi) {
return;
case InteriorPointerOperandKind::OpenExistentialBox:
case InteriorPointerOperandKind::ProjectBox:
case InteriorPointerOperandKind::StoreBorrow:
// Can not mark this immutable.
return;
}
Expand Down
22 changes: 17 additions & 5 deletions lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ static bool isDestroyOfCopyOf(SILInstruction *instruction, SILValue def) {
//===----------------------------------------------------------------------===//

bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
LLVM_DEBUG(llvm::dbgs() << "Computing canonical liveness from:\n";
getCurrentDef()->print(llvm::dbgs()));
defUseWorklist.initialize(getCurrentDef());
// Only the first level of reborrows need to be consider. All nested inner
// adjacent reborrows and phis are encapsulated within their lifetimes.
Expand All @@ -140,7 +142,13 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
});
}
while (SILValue value = defUseWorklist.pop()) {
LLVM_DEBUG(llvm::dbgs() << " Uses of value:\n";
value->print(llvm::dbgs()));

for (Operand *use : value->getUses()) {
LLVM_DEBUG(llvm::dbgs() << " Use:\n";
use->getUser()->print(llvm::dbgs()));

auto *user = use->getUser();
// Recurse through copies.
if (auto *copy = dyn_cast<CopyValueInst>(user)) {
Expand Down Expand Up @@ -169,6 +177,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
// escape. Is it legal to canonicalize ForwardingUnowned?
case OperandOwnership::ForwardingUnowned:
case OperandOwnership::PointerEscape:
LLVM_DEBUG(llvm::dbgs() << " Value escaped! Giving up\n");
return false;
case OperandOwnership::InstantaneousUse:
case OperandOwnership::UnownedInstantaneousUse:
Expand Down Expand Up @@ -197,7 +206,8 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
break;
case OperandOwnership::Borrow:
if (liveness->updateForBorrowingOperand(use)
!= InnerBorrowKind::Contained) {
!= InnerBorrowKind::Contained) {
LLVM_DEBUG(llvm::dbgs() << " Inner borrow can't be contained! Giving up\n");
return false;
}
break;
Expand Down Expand Up @@ -1133,11 +1143,13 @@ void CanonicalizeOSSALifetime::rewriteCopies() {
//===----------------------------------------------------------------------===//

bool CanonicalizeOSSALifetime::computeLiveness() {
if (currentDef->getOwnershipKind() != OwnershipKind::Owned)
return false;

LLVM_DEBUG(llvm::dbgs() << " Canonicalizing: " << currentDef);

if (currentDef->getOwnershipKind() != OwnershipKind::Owned) {
LLVM_DEBUG(llvm::dbgs() << " not owned, never mind\n");
return false;
}

// Note: There is no need to register callbacks with this utility. 'onDelete'
// is the only one in use to handle dangling pointers, which could be done
// instead be registering a temporary handler with the pass. Canonicalization
Expand All @@ -1154,7 +1166,7 @@ bool CanonicalizeOSSALifetime::computeLiveness() {

// Step 1: compute liveness
if (!computeCanonicalLiveness()) {
LLVM_DEBUG(llvm::errs() << "Failed to compute canonical liveness?!\n");
LLVM_DEBUG(llvm::dbgs() << "Failed to compute canonical liveness?!\n");
clear();
return false;
}
Expand Down
Loading