Skip to content

Commit 01680de

Browse files
committed
Fixup ownership of borrow accessor results
SILGen may produce a borrow accessor result from within a local borrow scope. Such as: ``` %ld = load_borrow %self %fwd = unchecked_ownership %ld %ex = struct_extract %fwd, #Struct.storedProperty end_borrow %ld return %ex ``` This is illegal OSSA, since the return uses a value outside it's borrow scope. Add a new SILGenCleanup transform, to turn this into valid OSSA: ``` %ld = load_borrow %self %ex = struct_extract %ld, #Struct.storedProperty return_borrow %ex from_scopes %ld ```
1 parent fc71233 commit 01680de

File tree

2 files changed

+96
-1
lines changed

2 files changed

+96
-1
lines changed

lib/SIL/Utils/MemAccessUtils.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,17 @@ void swift::findGuaranteedReferenceRoots(SILValue referenceValue,
924924
// regardless of whether they were already visited.
925925
continue;
926926
}
927+
928+
// Special handling until borrowing apply is represented as a
929+
// ForwardingOperation.
930+
if (auto *apply =
931+
dyn_cast_or_null<ApplyInst>(value->getDefiningInstruction())) {
932+
if (apply->hasGuaranteedResult()) {
933+
worklist.pushIfNotVisited(apply->getSelfArgument());
934+
continue;
935+
}
936+
}
937+
927938
// Found a potential root.
928939
if (lookThroughNestedBorrows) {
929940
if (auto *bbi = dyn_cast<BeginBorrowInst>(value)) {

lib/SILOptimizer/Mandatory/SILGenCleanup.cpp

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "swift/SILOptimizer/Utils/BasicBlockOptUtils.h"
3232
#include "swift/SILOptimizer/Utils/CanonicalizeInstruction.h"
3333
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
34+
#include "swift/SILOptimizer/Utils/InstructionDeleter.h"
3435
#include "llvm/ADT/PostOrderIterator.h"
3536

3637
using namespace swift;
@@ -107,6 +108,7 @@ namespace {
107108
struct SILGenCleanup : SILModuleTransform {
108109
void run() override;
109110

111+
bool fixupBorrowAccessors(SILFunction *function);
110112
bool completeOSSALifetimes(SILFunction *function);
111113
template <typename Range>
112114
bool completeLifetimesInRange(Range const &range,
@@ -229,6 +231,87 @@ void collectReachableRoots(SILFunction *function, BasicBlockWorklist &backward,
229231
} while (changed);
230232
}
231233

234+
/* SILGen may produce a borrow accessor result from within a local borrow
235+
* scope. Such as:
236+
*
237+
* ```
238+
* %ld = load_borrow %self
239+
* %fwd = unchecked_ownership %ld
240+
* %ex = struct_extract %fwd, #Struct.storedProperty
241+
* end_borrow %ld
242+
* return %ex
243+
* ```
244+
* This is illegal OSSA, since the return uses a value outside it's borrow
245+
* scope.
246+
*
247+
* Transform this into valid OSSA:
248+
*
249+
* ```
250+
* %ld = load_borrow %self
251+
* %ex = struct_extract %ld, #Struct.storedProperty
252+
* return_borrow %ex from_scopes %ld
253+
* ```
254+
*/
255+
bool SILGenCleanup::fixupBorrowAccessors(SILFunction *function) {
256+
if (!function->getConventions().hasGuaranteedResult()) {
257+
return false;
258+
}
259+
auto returnBB = function->findReturnBB();
260+
if (returnBB == function->end()) {
261+
return false;
262+
}
263+
264+
auto *returnInst = cast<ReturnInst>(returnBB->getTerminator());
265+
if (returnInst->getOperand()->getOwnershipKind() !=
266+
OwnershipKind::Guaranteed) {
267+
return false;
268+
}
269+
270+
SmallVector<SILValue, 8> enclosingValues;
271+
findGuaranteedReferenceRoots(returnInst->getOperand(),
272+
/*lookThroughNestedBorrows=*/false,
273+
enclosingValues);
274+
SmallVector<SILInstruction *> scopeEnds;
275+
SmallVector<SILValue> operands;
276+
SmallVector<SILInstruction *> toDelete;
277+
278+
// For all the local borrow scopes that enclose the return value, delete
279+
// their end_borrow instructions and use them as an encolsing value in
280+
// return_borrow instruction.
281+
for (auto enclosingValue : enclosingValues) {
282+
BorrowedValue borrow(enclosingValue);
283+
if (!borrow.isLocalScope()) {
284+
continue;
285+
}
286+
borrow.getLocalScopeEndingInstructions(scopeEnds);
287+
for (auto *scopeEnd : scopeEnds) {
288+
if (auto *endBorrow = dyn_cast<EndBorrowInst>(scopeEnd)) {
289+
operands.push_back(endBorrow->getOperand());
290+
endBorrow->eraseFromParent();
291+
}
292+
}
293+
for (auto *uncheckedOwnership :
294+
borrow->getUsersOfType<UncheckedOwnershipInst>()) {
295+
uncheckedOwnership->replaceAllUsesWith(*borrow);
296+
toDelete.push_back(uncheckedOwnership);
297+
}
298+
}
299+
300+
for (auto *inst : toDelete) {
301+
inst->eraseFromParent();
302+
}
303+
304+
if (operands.empty()) {
305+
return false;
306+
}
307+
308+
SILBuilderWithScope(returnInst)
309+
.createReturnBorrow(returnInst->getLoc(), returnInst->getOperand(),
310+
operands);
311+
returnInst->eraseFromParent();
312+
return true;
313+
}
314+
232315
bool SILGenCleanup::completeOSSALifetimes(SILFunction *function) {
233316
if (!getModule()->getOptions().OSSACompleteLifetimes)
234317
return false;
@@ -339,7 +422,8 @@ void SILGenCleanup::run() {
339422
LLVM_DEBUG(llvm::dbgs()
340423
<< "\nRunning SILGenCleanup on " << function.getName() << "\n");
341424

342-
bool changed = completeOSSALifetimes(&function);
425+
bool changed = fixupBorrowAccessors(&function);
426+
changed |= completeOSSALifetimes(&function);
343427
DeadEndBlocks deadEndBlocks(&function);
344428
SILGenCanonicalize sgCanonicalize(deadEndBlocks);
345429

0 commit comments

Comments
 (0)