Skip to content

SemanticARCOpts fix - remove deadEndBlocks checks #40500

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.

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
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion include/swift/SIL/OwnershipUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -566,8 +566,11 @@ struct BorrowedValue {
/// This ignores reborrows. The assumption is that, since \p uses are
/// dominated by this local scope, checking the extended borrow scope should
/// not be necessary to determine they are within the scope.
///
/// \p deadEndBlocks is optional during transition. It will be completely
/// removed in an upcoming commit.
bool areUsesWithinLocalScope(ArrayRef<Operand *> uses,
DeadEndBlocks &deadEndBlocks) const;
DeadEndBlocks *deadEndBlocks) const;

/// Given a local borrow scope introducer, visit all non-forwarding consuming
/// users. This means that this looks through guaranteed block arguments. \p
Expand Down
3 changes: 2 additions & 1 deletion include/swift/SIL/PrunedLiveness.h
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,9 @@ class PrunedLiveness {
/// client already knows that inst occurs after the start of liveness.
bool isWithinBoundary(SILInstruction *inst) const;

/// \p deadEndBlocks is optional.
bool areUsesWithinBoundary(ArrayRef<Operand *> uses,
DeadEndBlocks &deadEndBlocks) const;
DeadEndBlocks *deadEndBlocks) const;

/// Compute liveness for a single SSA definition.
void computeSSALiveness(SILValue def);
Expand Down
6 changes: 3 additions & 3 deletions lib/SIL/Utils/OwnershipUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ void BorrowedValue::computeLiveness(PrunedLiveness &liveness) const {
}

bool BorrowedValue::areUsesWithinLocalScope(
ArrayRef<Operand *> uses, DeadEndBlocks &deadEndBlocks) const {
ArrayRef<Operand *> uses, DeadEndBlocks *deadEndBlocks) const {
// First make sure that we actually have a local scope. If we have a non-local
// scope, then we have something (like a SILFunctionArgument) where a larger
// semantic construct (in the case of SILFunctionArgument, the function
Expand Down Expand Up @@ -906,14 +906,14 @@ bool AddressOwnership::areUsesWithinLifetime(
SILValue root = base.getOwnershipReferenceRoot();
BorrowedValue borrow(root);
if (borrow)
return borrow.areUsesWithinLocalScope(uses, deadEndBlocks);
return borrow.areUsesWithinLocalScope(uses, &deadEndBlocks);

// --- A reference no borrow scope. Currently happens for project_box.

// Compute the reference value's liveness.
PrunedLiveness liveness;
liveness.computeSSALiveness(root);
return liveness.areUsesWithinBoundary(uses, deadEndBlocks);
return liveness.areUsesWithinBoundary(uses, &deadEndBlocks);
}

//===----------------------------------------------------------------------===//
Expand Down
8 changes: 6 additions & 2 deletions lib/SIL/Utils/PrunedLiveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,14 @@ bool PrunedLiveness::isWithinBoundary(SILInstruction *inst) const {
}

bool PrunedLiveness::areUsesWithinBoundary(ArrayRef<Operand *> uses,
DeadEndBlocks &deadEndBlocks) const {
DeadEndBlocks *deadEndBlocks) const {
auto checkDeadEnd = [deadEndBlocks](SILInstruction *inst) {
return deadEndBlocks && deadEndBlocks->isDeadEnd(inst->getParent());
};

for (auto *use : uses) {
auto *user = use->getUser();
if (!isWithinBoundary(user) && !deadEndBlocks.isDeadEnd(user->getParent()))
if (!isWithinBoundary(user) && !checkDeadEnd(user))
return false;
}
return true;
Expand Down
21 changes: 12 additions & 9 deletions lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(
// dead end blocks that use the value in a non-consuming way.
//
// TODO: There may be some way of sinking this into the loop below.
//
// FIXME: The haveAnyLocalScopes and destroy.empty() checks are relics of
// attempts to handle dead end blocks during areUsesWithinLocalScope. If we
// don't use dead end blocks at all, they should not be relevant.
bool haveAnyLocalScopes =
llvm::any_of(borrowScopeIntroducers, [](BorrowedValue borrowScope) {
return borrowScope.isLocalScope();
Expand All @@ -177,17 +181,16 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(
// anyways, we want to be conservative here and optimize only if we do not
// need to insert an end_borrow since all of our borrow introducers are
// non-local scopes.
//
// The call to areUsesWithinLocalScope cannot consider dead-end blocks. A
// local borrow scope requires all its inner uses to be inside the borrow
// scope, regardless of whether the end of the scope is inside a dead-end
// block.
{
bool foundNonDeadEnd = false;
for (auto *d : destroys) {
foundNonDeadEnd |= !getDeadEndBlocks().isDeadEnd(d->getParentBlock());
}
if (!foundNonDeadEnd && haveAnyLocalScopes)
return false;
SmallVector<Operand *, 8> scratchSpace;
if (llvm::any_of(borrowScopeIntroducers, [&](BorrowedValue borrowScope) {
return !borrowScope.areUsesWithinLocalScope(lr.getAllConsumingUses(),
getDeadEndBlocks());
return !borrowScope.areUsesWithinLocalScope(lr.getAllConsumingUses(),
nullptr);
})) {
return false;
}
Expand Down Expand Up @@ -216,7 +219,7 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(

if (llvm::any_of(borrowScopeIntroducers, [&](BorrowedValue borrowScope) {
return !borrowScope.areUsesWithinLocalScope(
phiArgLR.getAllConsumingUses(), getDeadEndBlocks());
phiArgLR.getAllConsumingUses(), nullptr);
})) {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/SILOptimizer/Utils/OwnershipOptUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ GuaranteedOwnershipExtension::checkBorrowExtension(
assert(guaranteedLiveness.empty());
borrow.computeLiveness(guaranteedLiveness);

if (guaranteedLiveness.areUsesWithinBoundary(newUses, deBlocks))
if (guaranteedLiveness.areUsesWithinBoundary(newUses, &deBlocks))
return Valid; // reuse the borrow scope as-is

beginBorrow = dyn_cast<BeginBorrowInst>(borrow.value);
Expand Down Expand Up @@ -224,7 +224,7 @@ GuaranteedOwnershipExtension::checkLifetimeExtension(
ownedConsumeBlocks.push_back(user->getParent());
}
}
if (ownedLifetime.areUsesWithinBoundary(newUses, deBlocks))
if (ownedLifetime.areUsesWithinBoundary(newUses, &deBlocks))
return Valid;

return ExtendLifetime; // Can't cover newUses without destroy sinking.
Expand Down