Skip to content

Commit 6e5b49a

Browse files
committed
no_consume_or_assign ref_element_addr should not have any destroys
This fixes an issue when doing move-checking on a read accessor, where the field is only borrowed. After the MoveOnlyAddressChecker ran on it, it'd inject a destroy that didn't get "claimed": ``` %2 = ref_element_addr %0 : $ListOfFiles, #ListOfFiles.file %3 = mark_must_check [no_consume_or_assign] %2 : $*File %4 = begin_access [read] [dynamic] %3 : $*File %5 = load_borrow %4 : $*File yield %5 : $File, resume bb1, unwind bb2 bb1: end_borrow %5 : $File end_access %4 : $*File destroy_addr %2 : $*File // BAD %9 = tuple () return %9 : $() ``` The approach of this fix is to recognize that at the point we're injecting destroys, we would have emitted diagnostics and stopped already if there were any consuming uses that we need to clean-up after, since we're in `no_consume_or_assign` checking mode here when just reading the field.
1 parent 355b7dc commit 6e5b49a

File tree

1 file changed

+16
-2
lines changed

1 file changed

+16
-2
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,8 @@ struct MoveOnlyAddressCheckerPImpl {
959959

960960
bool performSingleCheck(MarkMustCheckInst *markedValue);
961961

962-
void insertDestroysOnBoundary(FieldSensitiveMultiDefPrunedLiveRange &liveness,
962+
void insertDestroysOnBoundary(MarkMustCheckInst *markedValue,
963+
FieldSensitiveMultiDefPrunedLiveRange &liveness,
963964
FieldSensitivePrunedLivenessBoundary &boundary);
964965

965966
void rewriteUses(FieldSensitiveMultiDefPrunedLiveRange &liveness,
@@ -1845,10 +1846,23 @@ static void insertDestroyBeforeInstruction(UseState &addressUseState,
18451846
}
18461847

18471848
void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
1849+
MarkMustCheckInst *markedValue,
18481850
FieldSensitiveMultiDefPrunedLiveRange &liveness,
18491851
FieldSensitivePrunedLivenessBoundary &boundary) {
18501852
using IsInterestingUser = FieldSensitivePrunedLiveness::IsInterestingUser;
18511853
LLVM_DEBUG(llvm::dbgs() << "Inserting destroys on boundary!\n");
1854+
1855+
// If we're in no_consume_or_assign mode, we don't insert destroys, as we've
1856+
// already checked that there are no consumes. There can only be borrow uses,
1857+
// which means no destruction is needed at all.
1858+
if (markedValue->getCheckKind() ==
1859+
MarkMustCheckInst::CheckKind::NoConsumeOrAssign) {
1860+
LLVM_DEBUG(llvm::dbgs()
1861+
<< " Skipping destroy insertion b/c no_consume_or_assign\n");
1862+
consumes.finishRecordingFinalConsumes();
1863+
return;
1864+
}
1865+
18521866
LLVM_DEBUG(llvm::dbgs() << " Visiting users!\n");
18531867

18541868
for (auto &pair : boundary.getLastUsers()) {
@@ -2113,7 +2127,7 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
21132127
SWIFT_DEFER { consumes.clear(); };
21142128
FieldSensitivePrunedLivenessBoundary boundary(liveness.getNumSubElements());
21152129
liveness.computeBoundary(boundary);
2116-
insertDestroysOnBoundary(liveness, boundary);
2130+
insertDestroysOnBoundary(markedAddress, liveness, boundary);
21172131
rewriteUses(liveness, boundary);
21182132

21192133
return true;

0 commit comments

Comments
 (0)