Skip to content

Commit 017f003

Browse files
committed
[OSSA] Fix borrowCopyOverGuaranteedUsers dead-ends.
The utility was computing the boundary of the `copy_value` with the `end_borrow`s as users. But there may not be any `end_borrow`s thanks to dead-ends. Instead, consider both guaranteed users and any `end_borrow`s. Fixes a miscompile in CSE. rdar://158353230
1 parent 1fe65b0 commit 017f003

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -969,7 +969,14 @@ BeginBorrowInst *OwnershipLifetimeExtender::borrowCopyOverGuaranteedUsers(
969969
// Create destroys at the end of copy's lifetime. This only needs to consider
970970
// uses that end the borrow scope.
971971
{
972-
ValueLifetimeAnalysis lifetimeAnalysis(copy, borrow->getEndBorrows());
972+
SmallVector<SILInstruction *, 16> users;
973+
for (auto *user : guaranteedUsers) {
974+
users.push_back(user);
975+
}
976+
for (auto *user : borrow->getEndBorrows()) {
977+
users.push_back(user);
978+
}
979+
ValueLifetimeAnalysis lifetimeAnalysis(copy, users);
973980
ValueLifetimeBoundary copyBoundary;
974981
lifetimeAnalysis.computeLifetimeBoundary(copyBoundary);
975982

test/SILOptimizer/cse_ossa_nontrivial.sil

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,3 +1164,38 @@ bb0:
11641164
%13 = tuple ()
11651165
return %13
11661166
}
1167+
1168+
/// A scenario where a borrow of a copy is introduced but the cfg is such that
1169+
/// no end_borrows are inserted for the borrow. The copy must still be
1170+
/// destroyed.
1171+
// CHECK-LABEL: sil [ossa] @no_boundary_for_end_borrows : {{.*}} {
1172+
// TODO: There shouldn't actually be a copy_value here, but lifetime transfer
1173+
// APIs are written and OSSARAUWHelper is in use, there will be.
1174+
// Even then, this test case shouldn't crash.
1175+
// CHECK: copy_value
1176+
// CHECK: destroy_value
1177+
// CHECK-LABEL: } // end sil function 'no_boundary_for_end_borrows'
1178+
sil [ossa] @no_boundary_for_end_borrows : $@convention(thin) (@guaranteed NonTrivialStruct) -> () {
1179+
entry(%s : @guaranteed $NonTrivialStruct):
1180+
%c1 = struct_extract %s : $NonTrivialStruct, #NonTrivialStruct.val
1181+
cond_br undef, loop_entry, exit
1182+
1183+
exit:
1184+
apply undef(%c1) : $@convention(thin) (@guaranteed Klass) -> ()
1185+
return undef : $()
1186+
1187+
loop_entry:
1188+
%c2 = struct_extract %s : $NonTrivialStruct, #NonTrivialStruct.val
1189+
apply undef(%c2) : $@convention(thin) (@guaranteed Klass) -> ()
1190+
br loop_header
1191+
1192+
loop_header:
1193+
apply undef(%c2) : $@convention(thin) (@guaranteed Klass) -> ()
1194+
cond_br undef, die, loop_backedge
1195+
1196+
loop_backedge:
1197+
br loop_header
1198+
1199+
die:
1200+
unreachable
1201+
}

0 commit comments

Comments
 (0)