Skip to content

Commit 25061fb

Browse files
jckarterjckarter
authored
Merge pull request #69169 from jckarter/store-borrow-lifetime-nesting
SIL: Treat store_borrow as borrowing its source, and have the move-only checker account for borrow scopes.
2 parents 24fbd54 + 1dcf271 commit 25061fb

File tree

8 files changed

+138
-51
lines changed

8 files changed

+138
-51
lines changed

‎include/swift/SIL/OwnershipUtils.h

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ class BorrowingOperandKind {
254254
enum Kind : uint8_t {
255255
Invalid = 0,
256256
BeginBorrow,
257+
StoreBorrow,
257258
BeginApply,
258259
Branch,
259260
Apply,
@@ -277,6 +278,8 @@ class BorrowingOperandKind {
277278
return Kind::Invalid;
278279
case SILInstructionKind::BeginBorrowInst:
279280
return Kind::BeginBorrow;
281+
case SILInstructionKind::StoreBorrowInst:
282+
return Kind::StoreBorrow;
280283
case SILInstructionKind::BeginApplyInst:
281284
return Kind::BeginApply;
282285
case SILInstructionKind::BranchInst:
@@ -386,6 +389,7 @@ struct BorrowingOperand {
386389
case BorrowingOperandKind::Invalid:
387390
llvm_unreachable("Using invalid case?!");
388391
case BorrowingOperandKind::BeginBorrow:
392+
case BorrowingOperandKind::StoreBorrow:
389393
case BorrowingOperandKind::BeginApply:
390394
case BorrowingOperandKind::Apply:
391395
case BorrowingOperandKind::TryApply:
@@ -418,6 +422,7 @@ struct BorrowingOperand {
418422
case BorrowingOperandKind::BeginBorrow:
419423
case BorrowingOperandKind::Branch:
420424
return true;
425+
case BorrowingOperandKind::StoreBorrow:
421426
case BorrowingOperandKind::BeginApply:
422427
case BorrowingOperandKind::Apply:
423428
case BorrowingOperandKind::TryApply:
@@ -790,7 +795,6 @@ class InteriorPointerOperandKind {
790795
RefTailAddr,
791796
OpenExistentialBox,
792797
ProjectBox,
793-
StoreBorrow,
794798
};
795799

796800
private:
@@ -819,8 +823,6 @@ class InteriorPointerOperandKind {
819823
return Kind::OpenExistentialBox;
820824
case SILInstructionKind::ProjectBoxInst:
821825
return Kind::ProjectBox;
822-
case SILInstructionKind::StoreBorrowInst:
823-
return Kind::StoreBorrow;
824826
}
825827
}
826828

@@ -839,8 +841,6 @@ class InteriorPointerOperandKind {
839841
return Kind::OpenExistentialBox;
840842
case ValueKind::ProjectBoxInst:
841843
return Kind::ProjectBox;
842-
case ValueKind::StoreBorrowInst:
843-
return Kind::StoreBorrow;
844844
}
845845
}
846846

@@ -897,8 +897,7 @@ struct InteriorPointerOperand {
897897
case InteriorPointerOperandKind::RefElementAddr:
898898
case InteriorPointerOperandKind::RefTailAddr:
899899
case InteriorPointerOperandKind::OpenExistentialBox:
900-
case InteriorPointerOperandKind::ProjectBox:
901-
case InteriorPointerOperandKind::StoreBorrow: {
900+
case InteriorPointerOperandKind::ProjectBox: {
902901
// Ok, we have a valid instruction. Return the relevant operand.
903902
auto *op =
904903
&cast<SingleValueInstruction>(resultValue)->getAllOperands()[0];
@@ -941,8 +940,6 @@ struct InteriorPointerOperand {
941940
return cast<OpenExistentialBoxInst>(operand->getUser());
942941
case InteriorPointerOperandKind::ProjectBox:
943942
return cast<ProjectBoxInst>(operand->getUser());
944-
case InteriorPointerOperandKind::StoreBorrow:
945-
return cast<StoreBorrowInst>(operand->getUser());
946943
}
947944
llvm_unreachable("Covered switch isn't covered?!");
948945
}

‎lib/SIL/IR/OperandOwnership.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ OperandOwnership OperandOwnershipClassifier::visitBranchInst(BranchInst *bi) {
452452
OperandOwnership
453453
OperandOwnershipClassifier::visitStoreBorrowInst(StoreBorrowInst *i) {
454454
if (getValue() == i->getSrc()) {
455-
return OperandOwnership::InteriorPointer;
455+
return OperandOwnership::Borrow;
456456
}
457457
return OperandOwnership::TrivialUse;
458458
}

‎lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,9 @@ void BorrowingOperandKind::print(llvm::raw_ostream &os) const {
600600
case Kind::BeginBorrow:
601601
os << "BeginBorrow";
602602
return;
603+
case Kind::StoreBorrow:
604+
os << "StoreBorrow";
605+
return;
603606
case Kind::BeginApply:
604607
os << "BeginApply";
605608
return;
@@ -649,6 +652,7 @@ bool BorrowingOperand::hasEmptyRequiredEndingUses() const {
649652
case BorrowingOperandKind::Invalid:
650653
llvm_unreachable("Using invalid case");
651654
case BorrowingOperandKind::BeginBorrow:
655+
case BorrowingOperandKind::StoreBorrow:
652656
case BorrowingOperandKind::BeginApply:
653657
case BorrowingOperandKind::BeginAsyncLet:
654658
case BorrowingOperandKind::PartialApplyStack: {
@@ -687,6 +691,20 @@ bool BorrowingOperand::visitScopeEndingUses(
687691
// false.
688692
return !deadBorrow;
689693
}
694+
case BorrowingOperandKind::StoreBorrow: {
695+
bool deadBorrow = true;
696+
for (auto *use : cast<StoreBorrowInst>(op->getUser())->getUses()) {
697+
if (isa<EndBorrowInst>(use->getUser())) {
698+
deadBorrow = false;
699+
if (!func(use))
700+
return false;
701+
}
702+
}
703+
// FIXME: special case for dead borrows. This is dangerous because clients
704+
// only expect visitScopeEndingUses to return false if the visitor returned
705+
// false.
706+
return !deadBorrow;
707+
}
690708
case BorrowingOperandKind::BeginApply: {
691709
bool deadApply = true;
692710
auto *user = cast<BeginApplyInst>(op->getUser());
@@ -763,6 +781,7 @@ BorrowedValue BorrowingOperand::getBorrowIntroducingUserResult() const {
763781
case BorrowingOperandKind::Yield:
764782
case BorrowingOperandKind::PartialApplyStack:
765783
case BorrowingOperandKind::BeginAsyncLet:
784+
case BorrowingOperandKind::StoreBorrow:
766785
return BorrowedValue();
767786

768787
case BorrowingOperandKind::BeginBorrow: {
@@ -792,6 +811,7 @@ SILValue BorrowingOperand::getScopeIntroducingUserResult() {
792811
case BorrowingOperandKind::BeginAsyncLet:
793812
case BorrowingOperandKind::PartialApplyStack:
794813
case BorrowingOperandKind::BeginBorrow:
814+
case BorrowingOperandKind::StoreBorrow:
795815
return cast<SingleValueInstruction>(op->getUser());
796816

797817
case BorrowingOperandKind::BeginApply:

‎lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp

Lines changed: 70 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,17 @@ bool Implementation::gatherUses(SILValue value) {
437437
false /*is lifetime ending*/}});
438438
liveness.updateForUse(nextUse->getUser(), *leafRange,
439439
false /*is lifetime ending*/);
440+
// The liveness extends to the scope-ending uses of the borrow.
441+
BorrowingOperand(nextUse).visitScopeEndingUses([&](Operand *end) -> bool {
442+
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
443+
return false;
444+
}
445+
LLVM_DEBUG(llvm::dbgs() << " ++ Scope-ending use: ";
446+
end->getUser()->print(llvm::dbgs()));
447+
liveness.updateForUse(end->getUser(), *leafRange,
448+
false /*is lifetime ending*/);
449+
return true;
450+
});
440451
instToInterestingOperandIndexMap.insert(nextUse->getUser(), nextUse);
441452

442453
continue;
@@ -1001,6 +1012,48 @@ dumpIntervalMap(IntervalMapAllocator::Map &map) {
10011012
}
10021013
#endif
10031014

1015+
// Helper to insert end_borrows after the end of a non-consuming use. If the
1016+
// use is momentary, one end_borrow is inserted after the use. If it is an
1017+
// interior pointer projection or nested borrow, then end_borrows are inserted
1018+
// after every scope-ending instruction for the use.
1019+
static void insertEndBorrowsForNonConsumingUse(Operand *op,
1020+
SILValue borrow) {
1021+
if (auto iOp = InteriorPointerOperand::get(op)) {
1022+
LLVM_DEBUG(llvm::dbgs() << " -- Ending borrow after interior pointer scope:\n"
1023+
" ";
1024+
op->getUser()->print(llvm::dbgs()));
1025+
iOp.visitBaseValueScopeEndingUses([&](Operand *endScope) -> bool {
1026+
auto *endScopeInst = endScope->getUser();
1027+
LLVM_DEBUG(llvm::dbgs() << " ";
1028+
endScopeInst->print(llvm::dbgs()));
1029+
SILBuilderWithScope endBuilder(endScopeInst);
1030+
endBuilder.createEndBorrow(getSafeLoc(endScopeInst), borrow);
1031+
return true;
1032+
});
1033+
} else if (auto bOp = BorrowingOperand(op)) {
1034+
LLVM_DEBUG(llvm::dbgs() << " -- Ending borrow after borrow scope:\n"
1035+
" ";
1036+
op->getUser()->print(llvm::dbgs()));
1037+
bOp.visitScopeEndingUses([&](Operand *endScope) -> bool {
1038+
auto *endScopeInst = endScope->getUser();
1039+
LLVM_DEBUG(llvm::dbgs() << " ";
1040+
endScopeInst->print(llvm::dbgs()));
1041+
auto afterScopeInst = endScopeInst->getNextInstruction();
1042+
SILBuilderWithScope endBuilder(afterScopeInst);
1043+
endBuilder.createEndBorrow(getSafeLoc(afterScopeInst),
1044+
borrow);
1045+
return true;
1046+
});
1047+
} else {
1048+
auto *nextInst = op->getUser()->getNextInstruction();
1049+
LLVM_DEBUG(llvm::dbgs() << " -- Ending borrow after momentary use at: ";
1050+
nextInst->print(llvm::dbgs()));
1051+
SILBuilderWithScope endBuilder(nextInst);
1052+
endBuilder.createEndBorrow(getSafeLoc(nextInst), borrow);
1053+
}
1054+
1055+
}
1056+
10041057
void Implementation::rewriteUses(InstructionDeleter *deleter) {
10051058
blocksToUses.setFrozen();
10061059

@@ -1129,6 +1182,8 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
11291182
}
11301183

11311184
// Otherwise, we need to insert a borrow.
1185+
LLVM_DEBUG(llvm::dbgs() << " Inserting borrow for: ";
1186+
inst->print(llvm::dbgs()));
11321187
SILBuilderWithScope borrowBuilder(inst);
11331188
SILValue borrow =
11341189
borrowBuilder.createBeginBorrow(getSafeLoc(inst), first);
@@ -1138,21 +1193,10 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
11381193
borrowBuilder.createGuaranteedMoveOnlyWrapperToCopyableValue(
11391194
getSafeLoc(inst), innerValue);
11401195
}
1196+
1197+
insertEndBorrowsForNonConsumingUse(&operand, borrow);
11411198

1142-
if (auto op = InteriorPointerOperand::get(&operand)) {
1143-
op.visitBaseValueScopeEndingUses([&](Operand *endScope) -> bool {
1144-
auto *endScopeInst = endScope->getUser();
1145-
SILBuilderWithScope endBuilder(endScopeInst);
1146-
endBuilder.createEndBorrow(getSafeLoc(endScopeInst), borrow);
1147-
return true;
1148-
});
1149-
} else {
1150-
auto *nextInst = inst->getNextInstruction();
1151-
SILBuilderWithScope endBuilder(nextInst);
1152-
endBuilder.createEndBorrow(getSafeLoc(nextInst), borrow);
1153-
}
1154-
1155-
// NOTE: This needs to be /after/the interior pointer operand usage
1199+
// NOTE: This needs to be /after/ the interior pointer operand usage
11561200
// above so that we can use the end scope of our interior pointer base
11571201
// value.
11581202
// NOTE: oldInst may be nullptr if our operand is a SILArgument
@@ -1245,9 +1289,7 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
12451289
}
12461290
}
12471291

1248-
auto *nextInst = inst->getNextInstruction();
1249-
SILBuilderWithScope endBuilder(nextInst);
1250-
endBuilder.createEndBorrow(getSafeLoc(nextInst), borrow);
1292+
insertEndBorrowsForNonConsumingUse(&operand, borrow);
12511293
continue;
12521294
}
12531295

@@ -1333,6 +1375,13 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
13331375
}
13341376

13351377
void Implementation::cleanup() {
1378+
LLVM_DEBUG(llvm::dbgs()
1379+
<< "Performing BorrowToDestructureTransform::cleanup()!\n");
1380+
SWIFT_DEFER {
1381+
LLVM_DEBUG(llvm::dbgs() << "Function after cleanup!\n";
1382+
getMarkedValue()->getFunction()->dump());
1383+
};
1384+
13361385
// Then add destroys for any destructure elements that we inserted that we did
13371386
// not actually completely consume.
13381387
auto *fn = getMarkedValue()->getFunction();
@@ -1793,11 +1842,15 @@ bool BorrowToDestructureTransform::transform() {
17931842
// Then clean up all of our borrows/copies/struct_extracts which no longer
17941843
// have any uses...
17951844
{
1845+
LLVM_DEBUG(llvm::dbgs() << "Deleting dead instructions!\n");
1846+
17961847
InstructionDeleter deleter;
17971848
while (!borrowWorklist.empty()) {
17981849
deleter.recursivelyForceDeleteUsersAndFixLifetimes(
17991850
borrowWorklist.pop_back_val());
18001851
}
1852+
LLVM_DEBUG(llvm::dbgs() << "Function after deletion!\n";
1853+
impl.getMarkedValue()->getFunction()->dump());
18011854
}
18021855

18031856
return true;

‎lib/SILOptimizer/SILCombiner/SILCombinerBuiltinVisitors.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ SILCombiner::optimizeBuiltinCOWBufferForReadingOSSA(BuiltinInst *bi) {
162162
return;
163163
case InteriorPointerOperandKind::OpenExistentialBox:
164164
case InteriorPointerOperandKind::ProjectBox:
165-
case InteriorPointerOperandKind::StoreBorrow:
166165
// Can not mark this immutable.
167166
return;
168167
}

‎lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ static bool isDestroyOfCopyOf(SILInstruction *instruction, SILValue def) {
129129
//===----------------------------------------------------------------------===//
130130

131131
bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
132+
LLVM_DEBUG(llvm::dbgs() << "Computing canonical liveness from:\n";
133+
getCurrentDef()->print(llvm::dbgs()));
132134
defUseWorklist.initialize(getCurrentDef());
133135
// Only the first level of reborrows need to be consider. All nested inner
134136
// adjacent reborrows and phis are encapsulated within their lifetimes.
@@ -140,7 +142,13 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
140142
});
141143
}
142144
while (SILValue value = defUseWorklist.pop()) {
145+
LLVM_DEBUG(llvm::dbgs() << " Uses of value:\n";
146+
value->print(llvm::dbgs()));
147+
143148
for (Operand *use : value->getUses()) {
149+
LLVM_DEBUG(llvm::dbgs() << " Use:\n";
150+
use->getUser()->print(llvm::dbgs()));
151+
144152
auto *user = use->getUser();
145153
// Recurse through copies.
146154
if (auto *copy = dyn_cast<CopyValueInst>(user)) {
@@ -169,6 +177,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
169177
// escape. Is it legal to canonicalize ForwardingUnowned?
170178
case OperandOwnership::ForwardingUnowned:
171179
case OperandOwnership::PointerEscape:
180+
LLVM_DEBUG(llvm::dbgs() << " Value escaped! Giving up\n");
172181
return false;
173182
case OperandOwnership::InstantaneousUse:
174183
case OperandOwnership::UnownedInstantaneousUse:
@@ -197,7 +206,8 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
197206
break;
198207
case OperandOwnership::Borrow:
199208
if (liveness->updateForBorrowingOperand(use)
200-
!= InnerBorrowKind::Contained) {
209+
!= InnerBorrowKind::Contained) {
210+
LLVM_DEBUG(llvm::dbgs() << " Inner borrow can't be contained! Giving up\n");
201211
return false;
202212
}
203213
break;
@@ -1136,11 +1146,13 @@ void CanonicalizeOSSALifetime::rewriteCopies() {
11361146
//===----------------------------------------------------------------------===//
11371147

11381148
bool CanonicalizeOSSALifetime::computeLiveness() {
1139-
if (currentDef->getOwnershipKind() != OwnershipKind::Owned)
1140-
return false;
1141-
11421149
LLVM_DEBUG(llvm::dbgs() << " Canonicalizing: " << currentDef);
11431150

1151+
if (currentDef->getOwnershipKind() != OwnershipKind::Owned) {
1152+
LLVM_DEBUG(llvm::dbgs() << " not owned, never mind\n");
1153+
return false;
1154+
}
1155+
11441156
// Note: There is no need to register callbacks with this utility. 'onDelete'
11451157
// is the only one in use to handle dangling pointers, which could be done
11461158
// instead be registering a temporary handler with the pass. Canonicalization
@@ -1157,7 +1169,7 @@ bool CanonicalizeOSSALifetime::computeLiveness() {
11571169

11581170
// Step 1: compute liveness
11591171
if (!computeCanonicalLiveness()) {
1160-
LLVM_DEBUG(llvm::errs() << "Failed to compute canonical liveness?!\n");
1172+
LLVM_DEBUG(llvm::dbgs() << "Failed to compute canonical liveness?!\n");
11611173
clear();
11621174
return false;
11631175
}

0 commit comments

Comments
 (0)