Skip to content

[move-function-value] Improve support for uses that are due to binding a moved value to a different value. #58470

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
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
39 changes: 31 additions & 8 deletions lib/SILOptimizer/Mandatory/MoveKillsCopyableValuesChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,22 @@ bool CheckerLivenessInfo::compute() {
break;
case OperandOwnership::Borrow: {
if (auto *bbi = dyn_cast<BeginBorrowInst>(user)) {
// Only add borrows to liveness if the borrow isn't lexical. If it is
// a lexical borrow, we have created an entirely new source level
// binding that should be tracked separately.
if (!bbi->isLexical()) {
// If we have a lexical begin_borrow, we are going to check its uses
// separately and emit diagnostics for it. So we just need to add the
// liveness of the begin_borrow.
//
// NOTE: We know that semantically the use lexical lifetime must have
// a separate lifetime from the base lexical lifetime that we are
// processing. We do not want to include those uses as transitive uses
// of our base lexical lifetime. We just want to treat the formation
// of the new variable as a use. Thus we only include the begin_borrow
// itself as the use.
if (bbi->isLexical()) {
liveness.updateForUse(bbi, false /*lifetime ending*/);
} else {
// Otherwise, try to update liveness for a borrowing operand
// use. This will make it so that we add the end_borrows of the
// liveness use. If we have a reborrow here, we will bail.
bool failed = !liveness.updateForBorrowingOperand(use);
if (failed)
return false;
Expand Down Expand Up @@ -351,24 +363,32 @@ bool MoveKillsCopyableValuesChecker::check() {
SmallSetVector<SILValue, 32> valuesToCheck;

for (auto *arg : fn->getEntryBlock()->getSILFunctionArguments()) {
if (arg->getOwnershipKind() == OwnershipKind::Owned)
if (arg->getOwnershipKind() == OwnershipKind::Owned) {
LLVM_DEBUG(llvm::dbgs() << "Found owned arg to check: " << *arg);
valuesToCheck.insert(arg);
}
}

for (auto &block : *fn) {
for (auto &ii : block) {
if (auto *bbi = dyn_cast<BeginBorrowInst>(&ii)) {
if (bbi->isLexical())
if (bbi->isLexical()) {
LLVM_DEBUG(llvm::dbgs()
<< "Found lexical lifetime to check: " << *bbi);
valuesToCheck.insert(bbi);
}
continue;
}
}
}

if (valuesToCheck.empty())
if (valuesToCheck.empty()) {
LLVM_DEBUG(llvm::dbgs() << "No values to check! Exiting early!\n");
return false;
}

LLVM_DEBUG(llvm::dbgs() << "Visiting Function: " << fn->getName() << "\n");
LLVM_DEBUG(llvm::dbgs()
<< "Found at least one value to check, performing checking.\n");
auto valuesToProcess =
llvm::makeArrayRef(valuesToCheck.begin(), valuesToCheck.end());
auto &mod = fn->getModule();
Expand Down Expand Up @@ -473,6 +493,9 @@ class MoveKillsCopyableValuesCheckerPass : public SILFunctionTransform {
assert(fn->getModule().getStage() == SILStage::Raw &&
"Should only run on Raw SIL");

LLVM_DEBUG(llvm::dbgs() << "*** Checking moved values in fn: "
<< getFunction()->getName() << '\n');

MoveKillsCopyableValuesChecker checker(getFunction());

// If we already had dominance or loop info generated, update them when
Expand Down
30 changes: 30 additions & 0 deletions test/SILOptimizer/move_function_kills_copyable_values.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,36 @@ public func loopUse1(_ x: Klass) {
}
}

//===---
// Let + Non Consuming Assignment
//

public func simpleLinearUseAssignment(_ x: __owned Klass) {
let y = x // expected-error {{'y' used after being moved}}
let _ = _move(y) // expected-note {{move here}}
let m = y // expected-note {{use here}}
let _ = m
}

public func conditionalUse1Assignment(_ x: Klass) {
let y = x
if booleanValue {
let _ = _move(y)
} else {
let m = y
let _ = m
}
}

public func loopUse1Assignment(_ x: Klass) {
let y = x // expected-error {{'y' used after being moved}}
let _ = _move(y) // expected-note {{move here}}
for _ in 0..<1024 {
let m = y // expected-note {{use here}}
let _ = m
}
}

//===---
// Let + Consuming Use
//
Expand Down