Skip to content

[SIL] Make owned function arguments lexical. #41228

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
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
63 changes: 63 additions & 0 deletions docs/SIL.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2364,6 +2364,69 @@ The current list of interior pointer SIL instructions are:
(*) We still need to finish adding support for project_box, but all other
interior pointers are guarded already.

Variable Lifetimes
~~~~~~~~~~~~~~~~~~

In order for programmer intended lifetimes to be maintained under optimization,
the lifetimes of SIL values which correspond to named source-level values can
only be modified in limited ways. Generally, the behavior is that the lifetime
of a named source-level value cannot _observably_ end before the end of the
lexical scope in which that value is defined. Specifically, code motion may
not move the ends of these lifetimes across a **deinit barrier**.

A few sorts of SIL value have lifetimes that are constrained that way:

1: `begin_borrow [lexical]`
2: `move_value [lexical]`
3: @owned function arguments
4: `alloc_stack [lexical]`

That these three have constrained lifetimes is encoded in ValueBase::isLexical,
which should be checked before changing the lifetime of a value.

The reason that only @owned function arguments are constrained is that a
@guaranteed function argument is guaranteed by the function's caller to live for
the full duration of the function already. Optimization of the function alone
can't shorten it. When such a function is inlined into its caller, though, a
lexical borrow scope is added for each of its @guaranteed arguments, ensuring
that the lifetime of the corresponding source-level value is not shortened in a
way that doesn't respect deinit barriers.

Unlike the other sorts, `alloc_stack [lexical]` isn't a SILValue. Instead, it
constrains the lifetime of an addressable variable. Since the constraint is
applied to the in-memory representation, no additional lexical SILValue is
required.

Deinit Barriers
```````````````

Deinit barriers (see swift::isDeinitBarrier) are instructions which would be
affected by the side effects of deinitializers. To maintain the order of
effects that is visible to the programmer, destroys of lexical values cannot be
reordered with respect to them. There are three kinds:

1. synchronization points (locks, memory barriers, syscalls, etc.)
2. loads of weak or unowned values
3. accesses of pointers

Examples:

1. Given an instance of a class which owns a file handle and closes the file
handle on deinit, writing to the file handle and then deallocating the
instance would result in changes being written. If the destroy of the
instance were hoisted above the call to write to the file handle, an error
would be raised instead.
2. Given an instance `c` of a class `C` which weakly references an instance `d`
of a second class `D`, if `d` is referenced via a local variable `v`, then
loading that weak reference from `c` within the variable scope should return
a non-nil reference to `d`. Hoisting the destroy of `v` above the weak load
from `c`, however, would result in the destruction of `d` before that load
and a nil weak reference to `D`.
3. Given an instance of a class which owns a buffer and deallocates it on
deinitialization, accessing the pointer and then deallocating the instance
is defined behavior. Hoisting the destroy of the instance above the access
to the memory would result in accessing a freed pointer.

Memory Lifetime
~~~~~~~~~~~~~~~

Expand Down
4 changes: 2 additions & 2 deletions include/swift/SIL/MemAccessUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ inline bool accessKindMayConflict(SILAccessKind a, SILAccessKind b) {

/// Return true if \p instruction is a deinitialization barrier.
///
/// Deinitialization barriers constrain variable lifetimes. Lexical end_borrow
/// and destroy_addr cannot be hoisted above them.
/// Deinitialization barriers constrain variable lifetimes. Lexical end_borrow,
/// destroy_value, and destroy_addr cannot be hoisted above them.
bool isDeinitBarrier(SILInstruction *instruction);

} // end namespace swift
Expand Down
8 changes: 8 additions & 0 deletions include/swift/SIL/OwnershipUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,14 @@ bool findExtendedTransitiveGuaranteedUses(
SILValue guaranteedValue,
SmallVectorImpl<Operand *> &usePoints);

/// Find non-transitive uses of a simple (i.e. without looking through
/// reborrows) value.
///
/// The scope-ending use of borrows of the value are included. If a borrow of
/// the value is reborrowed, returns false.
bool findUsesOfSimpleValue(SILValue value,
SmallVectorImpl<Operand *> *usePoints = nullptr);

/// An operand that forwards ownership to one or more results.
class ForwardingOperand {
Operand *use = nullptr;
Expand Down
7 changes: 6 additions & 1 deletion include/swift/SILOptimizer/Utils/OwnershipOptUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ class OwnershipRAUWHelper {
/// their value ownership. This ignores any current uses of \p oldValue. To
/// determine whether \p oldValue can be replaced as-is with it's existing
/// uses, create an instance of OwnershipRAUWHelper and check its validity.
static bool hasValidRAUWOwnership(SILValue oldValue, SILValue newValue);
static bool hasValidRAUWOwnership(SILValue oldValue, SILValue newValue,
ArrayRef<Operand *> oldUses);

private:
OwnershipFixupContext *ctx;
Expand Down Expand Up @@ -283,6 +284,10 @@ class OwnershipRAUWHelper {
SILValue getReplacementAddress();
};

/// Whether the provided uses lie within the current liveness of the
/// specified lexical value.
bool areUsesWithinLexicalValueLifetime(SILValue, ArrayRef<Operand *>);

/// A utility composed ontop of OwnershipFixupContext that knows how to replace
/// a single use of a value with another value with a different ownership. We
/// allow for the values to have different types.
Expand Down
6 changes: 2 additions & 4 deletions lib/SIL/IR/SILValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,8 @@ ValueBase::getDefiningInstructionResult() {
}

bool ValueBase::isLexical() const {
// TODO: Eventually, rather than SILGen'ing a borrow scope for owned
// arguments, we will just have this check here.
// if (auto *argument = dyn_cast<SILArgument>(this))
// return argument->getOwnershipKind() == OwnershipKind::Owned;
if (auto *argument = dyn_cast<SILFunctionArgument>(this))
return argument->getOwnershipKind() == OwnershipKind::Owned;
if (auto *bbi = dyn_cast<BeginBorrowInst>(this))
return bbi->isLexical();
if (auto *mvi = dyn_cast<MoveValueInst>(this))
Expand Down
19 changes: 19 additions & 0 deletions lib/SIL/Utils/OwnershipUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,25 @@ bool swift::findExtendedUsesOfSimpleBorrowedValue(
return true;
}

bool swift::findUsesOfSimpleValue(SILValue value,
SmallVectorImpl<Operand *> *usePoints) {
for (auto *use : value->getUses()) {
if (use->getOperandOwnership() == OperandOwnership::Borrow) {
if (!BorrowingOperand(use).visitScopeEndingUses([&](Operand *end) {
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
return false;
}
usePoints->push_back(use);
return true;
})) {
return false;
}
}
usePoints->push_back(use);
}
return true;
}

// Find all use points of \p guaranteedValue within its borrow scope. All use
// points will be dominated by \p guaranteedValue.
//
Expand Down
67 changes: 23 additions & 44 deletions lib/SILOptimizer/Utils/LexicalDestroyFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ struct BorroweeUsage final {
/// %lifetime. In detail, PrunedLiveness::isWithinBoundary relies on
/// clients to know that instructions are after the start of liveness. We
/// determine this via the dominance tree.
bool findBorroweeUsage(Context &, BorroweeUsage &);
bool findBorroweeUsage(Context const &, BorroweeUsage &);

/// Sift scope ends of %lifetime for those that CAN be folded.
class FilterCandidates final {
Expand All @@ -308,9 +308,6 @@ class FilterCandidates final {

/// Determines whether each candidate is viable for folding.
///
/// Requiring findUsers to have been called, this uses the more expensive
/// isViableMatch.
///
/// returns true if any candidates were viable
/// false otherwise
bool run(Candidates &candidates);
Expand All @@ -334,8 +331,7 @@ class FilterCandidates final {
///
/// If there are, we can't fold the apply. Specifically, we can't introduce
/// a move_value [lexical] %borrowee because that value still needs to be used
/// in those locations. While updateSSA would happily rewrite those uses to
/// be uses of the move, that is not a semantically valid transformation.
/// in those locations.
///
/// For example, given the following SIL
/// %borrowee : @owned
Expand Down Expand Up @@ -410,11 +406,7 @@ class Rewriter final {
/// possible)
/// (2) hoist the end_borrow
/// (3) delete the destroy_value
void fold(Match, SmallVectorImpl<int> const &rewritableArgumentIndices);

// Update SSA to reflect that fact that %move and %borrowee are two
// defs for the "same value".
void updateSSA();
void fold(Match, ArrayRef<int> rewritableArgumentIndices);

// The move_value [lexical] instruction that was added during the run.
//
Expand Down Expand Up @@ -470,25 +462,31 @@ bool run(Context &context) {
void Rewriter::run() {
bool foldedAny = false;
(void)foldedAny;
for (unsigned index = 0, count = candidates.vector.size(); index < count;
++index) {
auto size = candidates.vector.size();
for (unsigned index = 0; index < size; ++index) {
auto candidate = candidates.vector[index];
if (!candidate.viable)
continue;
createMove();
if (!candidate.viable) {
// Nonviable candidates still end with the pattern
//
// end_borrow %lifetime
// ...
// destroy_value %borrowee
//
// Now that the new move_value [lexical] dominates all candidates, the
// every candidate's destroy_value %borrowee is dominated by it, so every
// one is dominated by another consuming use which is illegal. Rewrite
// each such destroy_value to be a destroy_value of the move.
candidate.match.dvi->setOperand(mvi);
continue;
}

fold(candidate.match, candidate.argumentIndices);
#ifndef NDEBUG
foldedAny = true;
#endif
}
assert(foldedAny && "rewriting without anything to rewrite!?");

// There are now "two defs for %borrowee":
// - %borrowee
// - %move
// We need to update SSA.
updateSSA();
}

bool Rewriter::createMove() {
Expand All @@ -505,8 +503,7 @@ bool Rewriter::createMove() {
return true;
}

void Rewriter::fold(Match candidate,
SmallVectorImpl<int> const &rewritableArgumentIndices) {
void Rewriter::fold(Match candidate, ArrayRef<int> rewritableArgumentIndices) {
// First, rewrite the apply in terms of the move_value.
unsigned argumentNumber = 0;
for (auto index : rewritableArgumentIndices) {
Expand Down Expand Up @@ -551,26 +548,6 @@ void Rewriter::fold(Match candidate,
context.deleter.forceDelete(candidate.dvi);
}

void Rewriter::updateSSA() {
SILSSAUpdater updater;
updater.initialize(context.borrowee->getType(),
context.borrowee.getOwnershipKind());
updater.addAvailableValue(context.borrowee->getParentBlock(),
context.borrowee);
updater.addAvailableValue(mvi->getParentBlock(), mvi);

SmallVector<Operand *, 16> uses;
for (auto use : context.borrowee->getUses()) {
if (use->getUser() == mvi)
continue;
uses.push_back(use);
}

for (auto use : uses) {
updater.rewriteUse(*use);
}
}

//===----------------------------------------------------------------------===//
// MARK: Lookups
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -638,8 +615,10 @@ bool FilterCandidates::run(Candidates &candidates) {
return anyViable;
}

bool findBorroweeUsage(Context &context, BorroweeUsage &usage) {
bool findBorroweeUsage(Context const &context, BorroweeUsage &usage) {
auto recordUse = [&](Operand *use) {
// Ignore uses that aren't dominated by the introducer. PrunedLiveness
// relies on us doing this check.
if (!context.dominanceTree.dominates(context.introducer, use->getUser()))
return;
usage.uses.push_back(use);
Expand Down
Loading