Skip to content

Commit 3ef2e19

Browse files
committed
[AddressLowering] Fixed destructures' end_borrows.
A destructure may have users that have escaping operand ownership. Consequently, we can't rely on findInnerTransitiveGuaranteedUses as used by emitEndBorrows.
1 parent aad2b6e commit 3ef2e19

File tree

2 files changed

+98
-3
lines changed

2 files changed

+98
-3
lines changed

lib/SILOptimizer/Mandatory/AddressLowering.cpp

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@
150150
#include "swift/SIL/SILInstruction.h"
151151
#include "swift/SIL/SILValue.h"
152152
#include "swift/SIL/SILVisitor.h"
153+
#include "swift/SIL/StackList.h"
153154
#include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h"
154155
#include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
155156
#include "swift/SILOptimizer/PassManager/Transforms.h"
@@ -2259,6 +2260,11 @@ void ApplyRewriter::rewriteApply(ArrayRef<SILValue> newCallArgs) {
22592260
/// ending uses.
22602261
static void emitEndBorrows(SILValue value, AddressLoweringState &pass);
22612262

2263+
/// Emit end_borrows at the lifetime ends of the guaranteed value which
2264+
/// encloses \p enclosingValue.
2265+
static void emitEndBorrowsAtEnclosingGuaranteedBoundary(
2266+
SILValue introducer, SILValue enclosingValue, AddressLoweringState &pass);
2267+
22622268
void ApplyRewriter::convertBeginApplyWithOpaqueYield() {
22632269
// Avoid revisiting this apply.
22642270
bool erased = pass.indirectApplies.erase(apply);
@@ -3265,7 +3271,8 @@ void UseRewriter::rewriteDestructure(SILInstruction *destructure) {
32653271
}
32663272
result->replaceAllUsesWith(loadElement);
32673273
if (guaranteed) {
3268-
emitEndBorrows(loadElement, pass);
3274+
emitEndBorrowsAtEnclosingGuaranteedBoundary(
3275+
loadElement, destructure->getOperand(0), pass);
32693276
}
32703277
}
32713278
}
@@ -3392,6 +3399,23 @@ static void emitEndBorrows(SILValue value, AddressLoweringState &pass) {
33923399
});
33933400
}
33943401

3402+
/// Emit end_borrows at the lifetime ends of the guaranteed value which
3403+
/// encloses \p enclosingValue.
3404+
static void
3405+
emitEndBorrowsAtEnclosingGuaranteedBoundary(SILValue lifetimeToEnd,
3406+
SILValue enclosingValue,
3407+
AddressLoweringState &pass) {
3408+
visitBorrowIntroducers(enclosingValue, [&](SILValue introducer) {
3409+
auto borrow = BorrowedValue(introducer);
3410+
borrow.visitLocalScopeEndingUses([&](Operand *use) {
3411+
pass.getBuilder(use->getUser()->getIterator())
3412+
.createEndBorrow(pass.genLoc(), lifetimeToEnd);
3413+
return true;
3414+
});
3415+
return true;
3416+
});
3417+
}
3418+
33953419
// Extract from an opaque struct or tuple.
33963420
void UseRewriter::emitExtract(SingleValueInstruction *extractInst) {
33973421
SILValue extractAddr = addrMat.materializeDefProjection(extractInst);
@@ -3805,12 +3829,22 @@ static void rewriteFunction(AddressLoweringState &pass) {
38053829
if (valueDef->getType().isAddress())
38063830
continue;
38073831

3832+
// Collect end_borrows and rewrite them last so that when rewriting other
3833+
// users the lifetime ends of a borrow introducer can be used.
3834+
StackList<EndBorrowInst *> endBorrows(pass.function);
38083835
SmallPtrSet<Operand *, 8> originalUses;
38093836
SmallVector<Operand *, 8> uses(valueDef->getUses());
38103837
for (auto *oper : uses) {
38113838
originalUses.insert(oper);
3839+
if (auto *ebi = dyn_cast<EndBorrowInst>(oper->getUser())) {
3840+
endBorrows.push_back(ebi);
3841+
continue;
3842+
}
38123843
UseRewriter::rewriteUse(oper, pass);
38133844
}
3845+
for (auto *ebi : endBorrows) {
3846+
UseRewriter::rewriteUse(&ebi->getOperandRef(), pass);
3847+
}
38143848
// Rewrite every new use that was added.
38153849
uses.clear();
38163850
for (auto *use : valueDef->getUses()) {

test/SILOptimizer/address_lowering.sil

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,12 @@ enum Mixed<T> {
6060
case i(Int)
6161
case t(T)
6262
case o(AnyObject)
63-
};
63+
}
64+
65+
indirect enum RE<T> {
66+
case recursive(RE, T)
67+
case other(AnyObject)
68+
}
6469

6570
class TestGeneric<T> {
6671
init()
@@ -2224,9 +2229,9 @@ exit:
22242229
// CHECK: [[INSTANCE:%[^,]+]] = unchecked_take_enum_data_addr
22252230
// CHECK: [[KLASS_ADDR:%[^,]+]] = tuple_element_addr [[INSTANCE]] {{.*}}, 0
22262231
// CHECK: [[KLASS:%[^,]+]] = load_borrow [[KLASS_ADDR]]
2227-
// CHECK: end_borrow [[KLASS]]
22282232
// CHECK: [[ANY_ADDR:%[^,]+]] = tuple_element_addr [[INSTANCE]] {{.*}}, 1
22292233
// CHECK: apply undef<Any>([[ANY_ADDR]])
2234+
// CHECK: end_borrow [[KLASS]]
22302235
// CHECK: destroy_addr [[INSTANCE]]
22312236
// CHECK-LABEL: } // end sil function 'test_destructure_tuple_1_guaranteed'
22322237
sil [ossa] @test_destructure_tuple_1_guaranteed : $@convention(thin) () -> () {
@@ -2249,6 +2254,62 @@ exit:
22492254
return %505 : $()
22502255
}
22512256

2257+
// Check that the end_borrows created on behalf of the destructure_tuple end
2258+
// after the use of the load_borrow of the projection of the terminator result
2259+
// of the switch_enum that is guaranteed by that load_borrow.
2260+
//
2261+
// CHECK-LABEL: sil [ossa] @test_destructure_tuple_load_borrow_projected_box : {{.*}} {
2262+
// CHECK: {{bb[0-9]+}}([[OUTER:%[^,]+]] :
2263+
// CHECK: switch_enum [[OUTER]] : $RE<T>, case #RE.recursive!enumelt: [[OUTER_RECURSIVE:bb[0-9]+]], case #RE.other!enumelt: [[OUTER_OTHER:bb[0-9]+]]
2264+
// CHECK: [[OUTER_OTHER]]({{%[^,]+}} :
2265+
// CHECK: br [[EXIT:bb[0-9]+]]
2266+
// CHECK: [[OUTER_RECURSIVE]]([[TUPLE_BOX:%[^,]+]] :
2267+
// CHECK-NEXT: [[TUPLE_ADDR:%[^,]+]] = project_box [[TUPLE_BOX]]
2268+
// CHECK: [[INNER_ADDR:%[^,]+]] = tuple_element_addr [[TUPLE_ADDR]]
2269+
// CHECK: [[INNER:%[^,]+]] = load_borrow [[INNER_ADDR]]
2270+
// CHECK: switch_enum [[INNER]] : $RE<T>, case #RE.recursive!enumelt: [[INNER_RECURSIVE:bb[0-9]+]], case #RE.other!enumelt: [[INNER_OTHER:bb[0-9]+]]
2271+
// CHECK: [[INNER_OTHER]]([[OTHER_BOX:%[^,]+]] :
2272+
// CHECK: [[OTHER_ADDR:%[^,]+]] = project_box [[OTHER_BOX]]
2273+
// CHECK: [[OTHER:%[^,]+]] = load_borrow [[OTHER_ADDR]]
2274+
// CHECK: [[OTHER_COPY:%[^,]+]] = copy_value [[OTHER]]
2275+
// CHECK: end_borrow [[OTHER]]
2276+
// CHECK: end_borrow [[INNER]]
2277+
// CHECK: destroy_value [[OTHER_COPY]]
2278+
// CHECK: br [[EXIT]]
2279+
// CHECK: [[INNER_RECURSIVE]]
2280+
// CHECK: end_borrow [[INNER]]
2281+
// CHECK: br [[EXIT]]
2282+
// CHECK-LABEL: } // end sil function 'test_destructure_tuple_load_borrow_projected_box'
2283+
sil [ossa] @test_destructure_tuple_load_borrow_projected_box : $@convention(thin) <T> (@guaranteed RE<T>) -> () {
2284+
entry(%o : @guaranteed $RE<T>):
2285+
switch_enum %o : $RE<T>, case #RE.recursive!enumelt: outer_recursive, case #RE.other!enumelt: outer_other
2286+
2287+
outer_other(%other : @guaranteed $<τ_0_0> { var AnyObject } <T>):
2288+
br exit
2289+
2290+
outer_recursive(%tuple_box : @guaranteed $<Tau> { var (RE<Tau>, Tau) } <T>):
2291+
%tuple_addr = project_box %tuple_box : $<Tau> { var (RE<Tau>, Tau) } <T>, 0
2292+
%tuple = load_borrow %tuple_addr : $*(RE<T>, T)
2293+
(%i, %c) = destructure_tuple %tuple : $(RE<T>, T)
2294+
switch_enum %i : $RE<T>, case #RE.recursive!enumelt: inner_recursive, case #RE.other!enumelt: inner_other
2295+
2296+
inner_recursive(%in_tuple_box : @guaranteed $<Tau> { var (RE<Tau>, Tau) } <T>):
2297+
end_borrow %tuple : $(RE<T>, T)
2298+
br exit
2299+
2300+
inner_other(%s_box : @guaranteed $<Tau> { var AnyObject } <T>):
2301+
%s_addr = project_box %s_box : $<Tau> { var AnyObject } <T>, 0
2302+
%s = load_borrow %s_addr : $*AnyObject
2303+
%sc = copy_value %s : $AnyObject
2304+
end_borrow %s : $AnyObject
2305+
end_borrow %tuple : $(RE<T>, T)
2306+
destroy_value %sc : $AnyObject
2307+
br exit
2308+
2309+
exit:
2310+
%retval = tuple ()
2311+
return %retval : $()
2312+
}
22522313

22532314
// CHECK-LABEL: sil hidden [ossa] @test_store_1 : {{.*}} {
22542315
// CHECK: [[MAYBE_ADDR:%[^,]+]] = alloc_stack $Optional<Self>

0 commit comments

Comments
 (0)