Skip to content

Fix DCE of begin_borrow with @guaranteed operand #62881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 7, 2023
Merged

Fix DCE of begin_borrow with @guaranteed operand #62881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 15 additions & 19 deletions lib/SILOptimizer/Transforms/DeadCodeElimination.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,37 +283,33 @@ void DCE::markLive() {
break;
}
case SILInstructionKind::BeginBorrowInst: {
// Currently we only support borrows of owned values.
// Nested borrow handling can be complex in the presence of reborrows.
// So it is not handled currently.
auto *borrowInst = cast<BeginBorrowInst>(&I);
// Populate guaranteedPhiDependencies for this borrowInst
findGuaranteedPhiDependencies(BorrowedValue(borrowInst));
auto disableBorrowDCE = [&](SILValue borrow) {
visitTransitiveEndBorrows(borrow, [&](EndBorrowInst *endBorrow) {
markInstructionLive(endBorrow);
});
};
// If we have a begin_borrow of a @guaranteed operand, disable DCE'ing
// of parent borrow scopes. Dead reborrows needs complex handling, whuch
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Piggyback the typo in another PR

// is why it is disabled for now.
if (borrowInst->getOperand()->getOwnershipKind() ==
OwnershipKind::Guaranteed) {
// Visit the end_borrows of all the borrow scopes that this
// begin_borrow could be borrowing.
SmallVector<SILValue, 4> roots;
findGuaranteedReferenceRoots(borrowInst->getOperand(),
/*lookThroughNestedBorrows=*/false,
roots);
// Visit the end_borrows of all the borrow scopes that this
// begin_borrow could be borrowing, and mark them live.
for (auto root : roots) {
visitTransitiveEndBorrows(root,
[&](EndBorrowInst *endBorrow) {
markInstructionLive(endBorrow);
});
disableBorrowDCE(root);
}
continue;
}
// Populate guaranteedPhiDependencies for this borrow
findGuaranteedPhiDependencies(BorrowedValue(borrowInst));
// Don't optimize a borrow scope if it is lexical or has a pointer
// escape.
// If we have a lexical borrow scope or a pointer escape, disable DCE.
if (borrowInst->isLexical() ||
hasPointerEscape(BorrowedValue(borrowInst))) {
// Visit all end_borrows and mark them live
visitTransitiveEndBorrows(borrowInst, [&](EndBorrowInst *endBorrow) {
markInstructionLive(endBorrow);
});
continue;
disableBorrowDCE(borrowInst);
}
break;
}
Expand Down
42 changes: 42 additions & 0 deletions test/SILOptimizer/dead_code_elimination_nontrivial_ossa.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,48 @@ bb1(%newborrowi : @guaranteed $Klass, %newborrowo : @guaranteed $Klass):
return %res : $()
}


sil [ossa] @dce_nestedborrowlifetime4 : $@convention(thin) (@guaranteed Wrapper1, @guaranteed Wrapper1) -> () {
bb0(%0 : @guaranteed $Wrapper1, %1 : @guaranteed $Wrapper1):
cond_br undef, bb1, bb2

bb1:
%outer1 = begin_borrow %0 : $Wrapper1
%ex1 = struct_extract %outer1 : $Wrapper1, #Wrapper1.val1
%ex11 = struct_extract %ex1 : $Wrapper2, #Wrapper2.val2
br bb3(%ex11 : $Klass, %outer1 : $Wrapper1)

bb2:
%outer2 = begin_borrow %1 : $Wrapper1
%ex2 = struct_extract %outer2 : $Wrapper1, #Wrapper1.val1
%ex21 = struct_extract %ex2 : $Wrapper2, #Wrapper2.val2
br bb3(%ex21 : $Klass, %outer2 : $Wrapper1)

bb3(%phi1 : @guaranteed $Klass, %phi2 : @guaranteed $Wrapper1):
%f = function_ref @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
apply %f(%phi1) : $@convention(thin) (@guaranteed Klass) -> ()
end_borrow %phi2 : $Wrapper1
%9999 = tuple()
return %9999 : $()
}

sil [ossa] @dce_nestedborrowlifetime5 : $@convention(thin) (@guaranteed Wrapper1) -> () {
bb0(%0 : @guaranteed $Wrapper1):
%outer1 = begin_borrow %0 : $Wrapper1
%inner1 = begin_borrow %outer1 : $Wrapper1
%ex1 = struct_extract %inner1 : $Wrapper1, #Wrapper1.val1
%ex11 = struct_extract %ex1 : $Wrapper2, #Wrapper2.val2
br bb2(%ex11 : $Klass, %inner1 : $Wrapper1)

bb2(%phi1 : @guaranteed $Klass, %phi2 : @guaranteed $Wrapper1):
%f = function_ref @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
apply %f(%phi1) : $@convention(thin) (@guaranteed Klass) -> ()
end_borrow %phi2 : $Wrapper1
end_borrow %outer1 : $Wrapper1
%9999 = tuple()
return %9999 : $()
}

// CHECK-LABEL: sil [ossa] @infinite_loop :
// CHECK-NOT: copy_value
// CHECK-LABEL: } // end sil function 'infinite_loop'
Expand Down