Skip to content

[semantic-arc-opts] Use the LiveRange abstraction to find destroys instead of iterating over direct uses so we handle forwarding uses. #29002

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 4, 2020
Merged

[semantic-arc-opts] Use the LiveRange abstraction to find destroys instead of iterating over direct uses so we handle forwarding uses. #29002

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
73 changes: 17 additions & 56 deletions lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,6 @@ STATISTIC(NumEliminatedInsts, "number of removed instructions");
STATISTIC(NumLoadCopyConvertedToLoadBorrow,
"number of load_copy converted to load_borrow");

//===----------------------------------------------------------------------===//
// Utility
//===----------------------------------------------------------------------===//

static bool isConsumingOwnedUse(Operand *use) {
assert(use->get().getOwnershipKind() == ValueOwnershipKind::Owned);

if (use->isTypeDependent())
return false;

// We know that a copy_value produces an @owned value. Look through all of
// our uses and classify them as either invalidating or not
// invalidating. Make sure that all of the invalidating ones are
// destroy_value since otherwise the live_range is not complete.
auto map = use->getOwnershipKindMap();
auto constraint = map.getLifetimeConstraint(ValueOwnershipKind::Owned);
return constraint == UseLifetimeConstraint::MustBeInvalidated;
}

//===----------------------------------------------------------------------===//
// Live Range Modeling
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -325,7 +306,7 @@ struct SemanticARCOptVisitor
FORWARDING_TERM(CondBranch)
#undef FORWARDING_TERM

bool isWrittenTo(LoadInst *li, ArrayRef<SILInstruction *> destroys);
bool isWrittenTo(LoadInst *li, const LiveRange &lr);

bool processWorklist();

Expand Down Expand Up @@ -695,22 +676,20 @@ class StorageGuaranteesLoadVisitor
{
// The outer SemanticARCOptVisitor.
SemanticARCOptVisitor &ARCOpt;
// The original load instruction.
LoadInst *Load;

// The live range of the original load.
const LiveRange &liveRange;

// The current address being visited.
SILValue currentAddress;

Optional<bool> isWritten;

ArrayRef<SILInstruction *> destroyValues;

public:
StorageGuaranteesLoadVisitor(SemanticARCOptVisitor &arcOpt, LoadInst *load,
ArrayRef<SILInstruction *> destroyValues)
: ARCOpt(arcOpt), Load(load), currentAddress(load->getOperand()),
destroyValues(destroyValues) {}
const LiveRange &liveRange)
: ARCOpt(arcOpt), liveRange(liveRange),
currentAddress(load->getOperand()) {}

void answer(bool written) {
currentAddress = nullptr;
Expand Down Expand Up @@ -793,28 +772,11 @@ class StorageGuaranteesLoadVisitor
llvm::copy(borrowInst->getUsersOfType<EndBorrowInst>(),
std::back_inserter(baseEndBorrows));

SmallVector<SILInstruction *, 4> valueDestroys;
for (auto *use : Load->getUses()) {
// If this load is not consuming, skip it.
if (!isConsumingOwnedUse(use)) {
continue;
}

// Otherwise, if this isn't a destroy_value, we have a consuming use we
// don't understand. Return conservatively that this memory location may
// not be guaranteed.
auto *user = use->getUser();
if (!isa<DestroyValueInst>(user)) {
return answer(true);
}
valueDestroys.emplace_back(user);
}

SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
LinearLifetimeChecker checker(visitedBlocks, ARCOpt.getDeadEndBlocks());
// Returns true on success. So we invert.
bool foundError =
!checker.validateLifetime(baseObject, baseEndBorrows, valueDestroys);
bool foundError = !checker.validateLifetime(baseObject, baseEndBorrows,
liveRange.getDestroys());
return answer(foundError);
}

Expand Down Expand Up @@ -850,9 +812,9 @@ class StorageGuaranteesLoadVisitor
SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
LinearLifetimeChecker checker(visitedBlocks, ARCOpt.getDeadEndBlocks());
// Returns true on success. So we invert.
bool foundError =
!checker.validateLifetime(stack, destroyAddrs /*consuming users*/,
destroyValues /*non consuming users*/);
bool foundError = !checker.validateLifetime(
stack, destroyAddrs /*consuming users*/,
liveRange.getDestroys() /*non consuming users*/);
return answer(foundError);
}

Expand All @@ -866,9 +828,8 @@ class StorageGuaranteesLoadVisitor

} // namespace

bool SemanticARCOptVisitor::isWrittenTo(LoadInst *load,
ArrayRef<SILInstruction *> destroys) {
StorageGuaranteesLoadVisitor visitor(*this, load, destroys);
bool SemanticARCOptVisitor::isWrittenTo(LoadInst *load, const LiveRange &lr) {
StorageGuaranteesLoadVisitor visitor(*this, load, lr);
return visitor.doIt();
}

Expand All @@ -892,8 +853,7 @@ bool SemanticARCOptVisitor::visitLoadInst(LoadInst *li) {
// Then check if our address is ever written to. If it is, then we cannot use
// the load_borrow because the stored value may be released during the loaded
// value's live range.
auto destroyValues = lr.getDestroys();
if (isWrittenTo(li, destroyValues))
if (isWrittenTo(li, lr))
return false;

// Ok, we can perform our optimization. Convert the load [copy] into a
Expand All @@ -906,6 +866,7 @@ bool SemanticARCOptVisitor::visitLoadInst(LoadInst *li) {
// to find the post-dominating block set of these destroy value to ensure that
// we do not insert multiple end_borrow.
assert(lifetimeFrontier.empty());
auto destroyValues = lr.getDestroys();
ValueLifetimeAnalysis analysis(li, destroyValues);
bool foundCriticalEdges = !analysis.computeFrontier(
lifetimeFrontier, ValueLifetimeAnalysis::DontModifyCFG,
Expand Down
10 changes: 2 additions & 8 deletions test/SILOptimizer/semantic-arc-opts.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,11 +589,8 @@ bb0(%x : @owned $ClassLet):
return undef : $()
}

// We do not support this today, but we will once forwarding is ignored when
// checking if the load [copy] is a dead live range.
//
// CHECK-LABEL: sil [ossa] @dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase :
// CHECK: load [copy]
// CHECK: load_borrow
// CHECK: } // end sil function 'dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase'
sil [ossa] @dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase : $@convention(thin) (@owned ClassLet) -> () {
bb0(%x : @owned $ClassLet):
Expand All @@ -613,11 +610,8 @@ bb0(%x : @owned $ClassLet):
return undef : $()
}

// We do not support this today, but we will once forwarding is ignored when
// checking if the load [copy] is a dead live range.
//
// CHECK-LABEL: sil [ossa] @dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase_2 :
// CHECK: load [copy]
// CHECK: load_borrow
// CHECK: } // end sil function 'dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase_2'
sil [ossa] @dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase_2 : $@convention(thin) (@owned ClassLet) -> () {
bb0(%x : @owned $ClassLet):
Expand Down