Skip to content

Commit d62bf7d

Browse files
authored
Merge pull request swiftlang#39691 from meg-gupta/temprvofix
2 parents 5f60aad + 308704d commit d62bf7d

File tree

2 files changed

+74
-3
lines changed

2 files changed

+74
-3
lines changed

lib/SILOptimizer/Transforms/TempRValueElimination.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@
1818

1919
#define DEBUG_TYPE "sil-temp-rvalue-opt"
2020

21-
#include "swift/SIL/DebugUtils.h"
2221
#include "swift/SIL/BasicBlockUtils.h"
22+
#include "swift/SIL/DebugUtils.h"
2323
#include "swift/SIL/MemAccessUtils.h"
24+
#include "swift/SIL/OwnershipUtils.h"
2425
#include "swift/SIL/SILArgument.h"
2526
#include "swift/SIL/SILBuilder.h"
2627
#include "swift/SIL/SILVisitor.h"
@@ -276,9 +277,24 @@ collectLoads(Operand *addressUse, CopyAddrInst *originalCopy,
276277
loadInsts.insert(user);
277278
return true;
278279
}
279-
case SILInstructionKind::LoadBorrowInst:
280+
case SILInstructionKind::LoadBorrowInst: {
280281
loadInsts.insert(user);
281-
return true;
282+
BorrowedValue borrow(cast<LoadBorrowInst>(user));
283+
auto visitEndScope = [&](Operand *op) -> bool {
284+
auto *opUser = op->getUser();
285+
if (auto *endBorrow = dyn_cast<EndBorrowInst>(opUser)) {
286+
if (endBorrow->getParent() != block)
287+
return false;
288+
loadInsts.insert(endBorrow);
289+
return true;
290+
}
291+
// Don't look further if we see a reborrow.
292+
assert(cast<BranchInst>(opUser));
293+
return false;
294+
};
295+
auto res = borrow.visitLocalScopeEndingUses(visitEndScope);
296+
return res;
297+
}
282298
case SILInstructionKind::FixLifetimeInst:
283299
// If we have a fixed lifetime on our alloc_stack, we can just treat it like
284300
// a load and re-write it so that it is on the old memory or old src object.

test/SILOptimizer/temp_rvalue_opt_ossa.sil

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@ struct Two {
2121
var b: Klass
2222
}
2323

24+
struct NonTrivialStruct {
25+
var val:Klass
26+
}
27+
28+
public enum FakeOptional<T> {
29+
case none
30+
case some(T)
31+
}
32+
2433
sil @unknown : $@convention(thin) () -> ()
2534

2635
sil [ossa] @guaranteed_user : $@convention(thin) (@guaranteed Klass) -> ()
@@ -1410,3 +1419,49 @@ bb2:
14101419
unwind
14111420
}
14121421

1422+
// This does not get optimized correctly because of the conservative treatment of load_borrow/end_borrow in MemBehavior
1423+
// CHECK-LABEL: sil [ossa] @test_temprvoborrowboundary1 :
1424+
// CHECK: copy_addr
1425+
// TODO-CHECK-NOT: alloc_stack
1426+
// TODO-CHECK-NOT: copy_addr
1427+
// TODO-CHECK: [[ADDR:%.*]] = unchecked_take_enum_data_addr %0 : $*FakeOptional<NonTrivialStruct>, #FakeOptional.some!enumelt
1428+
// TODO-CHECK: [[ELE:%.*]] = struct_element_addr [[ADDR]] : $*NonTrivialStruct, #NonTrivialStruct.val
1429+
// TODO-CHECK: [[LD:%.*]] = load_borrow [[ELE]] : $*Klass
1430+
// TODO-CHECK: end_borrow [[LD]] : $Klass
1431+
// TODO-CHECK: destroy_addr [[ADDR]]
1432+
// CHECK: } // end sil function 'test_temprvoborrowboundary1'
1433+
sil [ossa] @test_temprvoborrowboundary1 : $@convention(thin) (@in FakeOptional<NonTrivialStruct>) -> () {
1434+
bb0(%0 : $*FakeOptional<NonTrivialStruct>):
1435+
%1 = alloc_stack $NonTrivialStruct
1436+
%2 = unchecked_take_enum_data_addr %0 : $*FakeOptional<NonTrivialStruct>, #FakeOptional.some!enumelt
1437+
copy_addr [take] %2 to [initialization] %1 : $*NonTrivialStruct
1438+
%4 = struct_element_addr %1 : $*NonTrivialStruct, #NonTrivialStruct.val
1439+
%5 = load_borrow %4 : $*Klass
1440+
end_borrow %5 : $Klass
1441+
destroy_addr %1 : $*NonTrivialStruct
1442+
dealloc_stack %1 : $*NonTrivialStruct
1443+
%res = tuple ()
1444+
return %res : $()
1445+
}
1446+
1447+
// This does not get optimized because of the end borrow in a different block
1448+
// CHECK-LABEL: sil [ossa] @test_temprvoborrowboundary2 :
1449+
// CHECK: copy_addr
1450+
// CHECK: } // end sil function 'test_temprvoborrowboundary2'
1451+
sil [ossa] @test_temprvoborrowboundary2 : $@convention(thin) (@in FakeOptional<NonTrivialStruct>) -> () {
1452+
bb0(%0 : $*FakeOptional<NonTrivialStruct>):
1453+
%1 = alloc_stack $NonTrivialStruct
1454+
%2 = unchecked_take_enum_data_addr %0 : $*FakeOptional<NonTrivialStruct>, #FakeOptional.some!enumelt
1455+
copy_addr [take] %2 to [initialization] %1 : $*NonTrivialStruct
1456+
%4 = struct_element_addr %1 : $*NonTrivialStruct, #NonTrivialStruct.val
1457+
%5 = load_borrow %4 : $*Klass
1458+
br bb1(%5 : $Klass)
1459+
1460+
bb1(%6 : @guaranteed $Klass):
1461+
end_borrow %6 : $Klass
1462+
destroy_addr %1 : $*NonTrivialStruct
1463+
dealloc_stack %1 : $*NonTrivialStruct
1464+
%res = tuple ()
1465+
return %res : $()
1466+
}
1467+

0 commit comments

Comments
 (0)