Skip to content

Commit cdeb55a

Browse files
committed
Fix DCE of begin_borrow with @guaranteed operand
For the lower most begin_borrows with @guaranteed operands, there was no @guaranteed phi computation, restructure the code and fix this case.
1 parent 0144fb6 commit cdeb55a

File tree

2 files changed

+57
-19
lines changed

2 files changed

+57
-19
lines changed

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -283,37 +283,33 @@ void DCE::markLive() {
283283
break;
284284
}
285285
case SILInstructionKind::BeginBorrowInst: {
286-
// Currently we only support borrows of owned values.
287-
// Nested borrow handling can be complex in the presence of reborrows.
288-
// So it is not handled currently.
289286
auto *borrowInst = cast<BeginBorrowInst>(&I);
287+
// Populate guaranteedPhiDependencies for this borrowInst
288+
findGuaranteedPhiDependencies(BorrowedValue(borrowInst));
289+
auto disableBorrowDCE = [&](SILValue borrow) {
290+
visitTransitiveEndBorrows(borrow, [&](EndBorrowInst *endBorrow) {
291+
markInstructionLive(endBorrow);
292+
});
293+
};
294+
// If we have a begin_borrow of a @guaranteed operand, disable DCE'ing
295+
// of parent borrow scopes. Dead reborrows needs complex handling, whuch
296+
// is why it is disabled for now.
290297
if (borrowInst->getOperand()->getOwnershipKind() ==
291298
OwnershipKind::Guaranteed) {
292-
// Visit the end_borrows of all the borrow scopes that this
293-
// begin_borrow could be borrowing.
294299
SmallVector<SILValue, 4> roots;
295300
findGuaranteedReferenceRoots(borrowInst->getOperand(),
296301
/*lookThroughNestedBorrows=*/false,
297302
roots);
303+
// Visit the end_borrows of all the borrow scopes that this
304+
// begin_borrow could be borrowing, and mark them live.
298305
for (auto root : roots) {
299-
visitTransitiveEndBorrows(root,
300-
[&](EndBorrowInst *endBorrow) {
301-
markInstructionLive(endBorrow);
302-
});
306+
disableBorrowDCE(root);
303307
}
304-
continue;
305308
}
306-
// Populate guaranteedPhiDependencies for this borrow
307-
findGuaranteedPhiDependencies(BorrowedValue(borrowInst));
308-
// Don't optimize a borrow scope if it is lexical or has a pointer
309-
// escape.
309+
// If we have a lexical borrow scope or a pointer escape, disable DCE.
310310
if (borrowInst->isLexical() ||
311311
hasPointerEscape(BorrowedValue(borrowInst))) {
312-
// Visit all end_borrows and mark them live
313-
visitTransitiveEndBorrows(borrowInst, [&](EndBorrowInst *endBorrow) {
314-
markInstructionLive(endBorrow);
315-
});
316-
continue;
312+
disableBorrowDCE(borrowInst);
317313
}
318314
break;
319315
}

test/SILOptimizer/dead_code_elimination_nontrivial_ossa.sil

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,48 @@ bb1(%newborrowi : @guaranteed $Klass, %newborrowo : @guaranteed $Klass):
529529
return %res : $()
530530
}
531531

532+
533+
sil [ossa] @dce_nestedborrowlifetime4 : $@convention(thin) (@guaranteed Wrapper1, @guaranteed Wrapper1) -> () {
534+
bb0(%0 : @guaranteed $Wrapper1, %1 : @guaranteed $Wrapper1):
535+
cond_br undef, bb1, bb2
536+
537+
bb1:
538+
%outer1 = begin_borrow %0 : $Wrapper1
539+
%ex1 = struct_extract %outer1 : $Wrapper1, #Wrapper1.val1
540+
%ex11 = struct_extract %ex1 : $Wrapper2, #Wrapper2.val2
541+
br bb3(%ex11 : $Klass, %outer1 : $Wrapper1)
542+
543+
bb2:
544+
%outer2 = begin_borrow %1 : $Wrapper1
545+
%ex2 = struct_extract %outer2 : $Wrapper1, #Wrapper1.val1
546+
%ex21 = struct_extract %ex2 : $Wrapper2, #Wrapper2.val2
547+
br bb3(%ex21 : $Klass, %outer2 : $Wrapper1)
548+
549+
bb3(%phi1 : @guaranteed $Klass, %phi2 : @guaranteed $Wrapper1):
550+
%f = function_ref @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
551+
apply %f(%phi1) : $@convention(thin) (@guaranteed Klass) -> ()
552+
end_borrow %phi2 : $Wrapper1
553+
%9999 = tuple()
554+
return %9999 : $()
555+
}
556+
557+
sil [ossa] @dce_nestedborrowlifetime5 : $@convention(thin) (@guaranteed Wrapper1) -> () {
558+
bb0(%0 : @guaranteed $Wrapper1):
559+
%outer1 = begin_borrow %0 : $Wrapper1
560+
%inner1 = begin_borrow %outer1 : $Wrapper1
561+
%ex1 = struct_extract %inner1 : $Wrapper1, #Wrapper1.val1
562+
%ex11 = struct_extract %ex1 : $Wrapper2, #Wrapper2.val2
563+
br bb2(%ex11 : $Klass, %inner1 : $Wrapper1)
564+
565+
bb2(%phi1 : @guaranteed $Klass, %phi2 : @guaranteed $Wrapper1):
566+
%f = function_ref @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
567+
apply %f(%phi1) : $@convention(thin) (@guaranteed Klass) -> ()
568+
end_borrow %phi2 : $Wrapper1
569+
end_borrow %outer1 : $Wrapper1
570+
%9999 = tuple()
571+
return %9999 : $()
572+
}
573+
532574
// CHECK-LABEL: sil [ossa] @infinite_loop :
533575
// CHECK-NOT: copy_value
534576
// CHECK-LABEL: } // end sil function 'infinite_loop'

0 commit comments

Comments
 (0)