Skip to content

Commit 2e54880

Browse files
committed
[move-function-value] Improve support for uses that are due to binding a moved value to a different value.
Most of the tests that I wrote used full on non-consuming/consuming uses rather than initialization of a variable. This commit fixes that hole in the model and adds some tests that exercise this behavior. Address moves did not have this problem due to different codegen.
1 parent 8a68a35 commit 2e54880

File tree

2 files changed

+61
-8
lines changed

2 files changed

+61
-8
lines changed

lib/SILOptimizer/Mandatory/MoveKillsCopyableValuesChecker.cpp

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,22 @@ bool CheckerLivenessInfo::compute() {
133133
break;
134134
case OperandOwnership::Borrow: {
135135
if (auto *bbi = dyn_cast<BeginBorrowInst>(user)) {
136-
// Only add borrows to liveness if the borrow isn't lexical. If it is
137-
// a lexical borrow, we have created an entirely new source level
138-
// binding that should be tracked separately.
139-
if (!bbi->isLexical()) {
136+
// If we have a lexical begin_borrow, we are going to check its uses
137+
// separately and emit diagnostics for it. So we just need to add the
138+
// liveness of the begin_borrow.
139+
//
140+
// NOTE: We know that semantically the use lexical lifetime must have
141+
// a separate lifetime from the base lexical lifetime that we are
142+
// processing. We do not want to include those uses as transitive uses
143+
// of our base lexical lifetime. We just want to treat the formation
144+
// of the new variable as a use. Thus we only include the begin_borrow
145+
// itself as the use.
146+
if (bbi->isLexical()) {
147+
liveness.updateForUse(bbi, false /*lifetime ending*/);
148+
} else {
149+
// Otherwise, try to update liveness for a borrowing operand
150+
// use. This will make it so that we add the end_borrows of the
151+
// liveness use. If we have a reborrow here, we will bail.
140152
bool failed = !liveness.updateForBorrowingOperand(use);
141153
if (failed)
142154
return false;
@@ -351,24 +363,32 @@ bool MoveKillsCopyableValuesChecker::check() {
351363
SmallSetVector<SILValue, 32> valuesToCheck;
352364

353365
for (auto *arg : fn->getEntryBlock()->getSILFunctionArguments()) {
354-
if (arg->getOwnershipKind() == OwnershipKind::Owned)
366+
if (arg->getOwnershipKind() == OwnershipKind::Owned) {
367+
LLVM_DEBUG(llvm::dbgs() << "Found owned arg to check: " << *arg);
355368
valuesToCheck.insert(arg);
369+
}
356370
}
357371

358372
for (auto &block : *fn) {
359373
for (auto &ii : block) {
360374
if (auto *bbi = dyn_cast<BeginBorrowInst>(&ii)) {
361-
if (bbi->isLexical())
375+
if (bbi->isLexical()) {
376+
LLVM_DEBUG(llvm::dbgs()
377+
<< "Found lexical lifetime to check: " << *bbi);
362378
valuesToCheck.insert(bbi);
379+
}
363380
continue;
364381
}
365382
}
366383
}
367384

368-
if (valuesToCheck.empty())
385+
if (valuesToCheck.empty()) {
386+
LLVM_DEBUG(llvm::dbgs() << "No values to check! Exiting early!\n");
369387
return false;
388+
}
370389

371-
LLVM_DEBUG(llvm::dbgs() << "Visiting Function: " << fn->getName() << "\n");
390+
LLVM_DEBUG(llvm::dbgs()
391+
<< "Found at least one value to check, performing checking.\n");
372392
auto valuesToProcess =
373393
llvm::makeArrayRef(valuesToCheck.begin(), valuesToCheck.end());
374394
auto &mod = fn->getModule();
@@ -475,6 +495,9 @@ class MoveKillsCopyableValuesCheckerPass : public SILFunctionTransform {
475495
assert(fn->getModule().getStage() == SILStage::Raw &&
476496
"Should only run on Raw SIL");
477497

498+
LLVM_DEBUG(llvm::dbgs() << "*** Checking moved values in fn: "
499+
<< getFunction()->getName() << '\n');
500+
478501
MoveKillsCopyableValuesChecker checker(getFunction());
479502

480503
// If we already had dominance or loop info generated, update them when

test/SILOptimizer/move_function_kills_copyable_values.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,36 @@ public func loopUse1(_ x: Klass) {
4747
}
4848
}
4949

50+
//===---
51+
// Let + Non Consuming Assignment
52+
//
53+
54+
public func simpleLinearUseAssignment(_ x: __owned Klass) {
55+
let y = x // expected-error {{'y' used after being moved}}
56+
let _ = _move(y) // expected-note {{move here}}
57+
let m = y // expected-note {{use here}}
58+
let _ = m
59+
}
60+
61+
public func conditionalUse1Assignment(_ x: Klass) {
62+
let y = x
63+
if booleanValue {
64+
let _ = _move(y)
65+
} else {
66+
let m = y
67+
let _ = m
68+
}
69+
}
70+
71+
public func loopUse1Assignment(_ x: Klass) {
72+
let y = x // expected-error {{'y' used after being moved}}
73+
let _ = _move(y) // expected-note {{move here}}
74+
for _ in 0..<1024 {
75+
let m = y // expected-note {{use here}}
76+
let _ = m
77+
}
78+
}
79+
5080
//===---
5181
// Let + Consuming Use
5282
//

0 commit comments

Comments
 (0)