Skip to content

[move-only] Add support for trivial move only var/mutating self/inout. #61143

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
21 changes: 17 additions & 4 deletions include/swift/SIL/PrunedLiveness.h
Original file line number Diff line number Diff line change
Expand Up @@ -752,17 +752,30 @@ struct TypeTreeLeafTypeRange {
SubElementNumber startEltOffset;
SubElementNumber endEltOffset;

private:
TypeTreeLeafTypeRange(SubElementNumber start, SubElementNumber end)
: startEltOffset(start), endEltOffset(end) {}

public:
/// The leaf type range for the entire type tree.
TypeTreeLeafTypeRange(SILValue rootAddress)
: startEltOffset(0), endEltOffset(TypeSubElementCount(rootAddress)) {}

/// The leaf type sub-range of the type tree of \p rootAddress, consisting of
/// \p projectedAddress and all of \p projectedAddress's descendent fields in
/// the type tree.
TypeTreeLeafTypeRange(SILValue projectedAddress, SILValue rootAddress)
: startEltOffset(
*SubElementNumber::compute(projectedAddress, rootAddress)),
endEltOffset(startEltOffset + TypeSubElementCount(projectedAddress)) {}
///
/// \returns None if we are unable to understand the path in between \p
/// projectedAddress and \p rootAddress.
static Optional<TypeTreeLeafTypeRange> get(SILValue projectedAddress,
SILValue rootAddress) {
auto startEltOffset =
SubElementNumber::compute(projectedAddress, rootAddress);
if (!startEltOffset)
return None;
return {{*startEltOffset,
*startEltOffset + TypeSubElementCount(projectedAddress)}};
}

/// Is the given leaf type specified by \p singleLeafElementNumber apart of
/// our \p range of leaf type values in the our larger type.
Expand Down
14 changes: 6 additions & 8 deletions lib/SIL/Utils/PrunedLiveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -507,14 +507,12 @@ SubElementNumber::compute(SILValue projectionDerivedFromRoot,
continue;
}

#ifndef NDEBUG
if (!isa<InitExistentialAddrInst>(projectionDerivedFromRoot)) {
llvm::errs() << "Unknown access path instruction!\n";
llvm::errs() << "Value: " << *projectionDerivedFromRoot;
llvm_unreachable("standard error");
}
#endif
// Cannot promote loads and stores from within an existential projection.
// If we do not know how to handle this case, just return None.
//
// NOTE: We use to assert here, but since this is used for diagnostics, we
// really do not want to abort. Instead, our caller can choose to abort if
// they get back a None. This ensures that we do not abort in cases where we
// just want to emit to the user a "I do not understand" error.
return None;
}
}
Expand Down
81 changes: 62 additions & 19 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -841,17 +841,22 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {

// TODO: What about copy_addr of itself. We really should just pre-process
// those maybe.
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
if (!leafRange)
return false;

assert(!useState.initInsts.count(user));
useState.initInsts.insert(
{user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
useState.initInsts.insert({user, *leafRange});
return true;
}

if (::memInstMustReinitialize(op)) {
LLVM_DEBUG(llvm::dbgs() << "Found reinit: " << *user);
assert(!useState.reinitInsts.count(user));
useState.reinitInsts.insert(
{user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
if (!leafRange)
return false;
useState.reinitInsts.insert({user, *leafRange});
return true;
}

Expand Down Expand Up @@ -891,9 +896,12 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
return true;
}

auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
if (!leafRange)
return false;

LLVM_DEBUG(llvm::dbgs() << "Found take: " << *user);
useState.takeInsts.insert(
{user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
useState.takeInsts.insert({user, *leafRange});
return true;
}

Expand Down Expand Up @@ -935,9 +943,13 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {

// If set, this will tell the checker that we can change this load into
// a load_borrow.
auto leafRange =
TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
if (!leafRange)
return false;

LLVM_DEBUG(llvm::dbgs() << "Found potential borrow: " << *user);
useState.borrows.insert(
{user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
useState.borrows.insert({user, *leafRange});
return true;
}

Expand All @@ -963,33 +975,41 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {

// Then if we had any final consuming uses, mark that this liveness use is
// a take and if not, mark this as a borrow.
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
if (!leafRange)
return false;

if (moveChecker.finalConsumingUses.empty()) {
LLVM_DEBUG(llvm::dbgs() << "Found borrow inst: " << *user);
useState.borrows.insert(
{user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
useState.borrows.insert({user, *leafRange});
} else {
LLVM_DEBUG(llvm::dbgs() << "Found take inst: " << *user);
useState.takeInsts.insert(
{user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
useState.takeInsts.insert({user, *leafRange});
}
return true;
}
}

// Now that we have handled or loadTakeOrCopy, we need to now track our Takes.
if (::memInstMustConsume(op)) {
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
if (!leafRange)
return false;
LLVM_DEBUG(llvm::dbgs() << "Pure consuming use: " << *user);
useState.takeInsts.insert(
{user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
useState.takeInsts.insert({user, *leafRange});
return true;
}

if (auto fas = FullApplySite::isa(op->getUser())) {
switch (fas.getArgumentConvention(*op)) {
case SILArgumentConvention::Indirect_In_Guaranteed:
useState.livenessUses.insert(
{user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
case SILArgumentConvention::Indirect_In_Guaranteed: {
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
if (!leafRange)
return false;

useState.livenessUses.insert({user, *leafRange});
return true;
}

case SILArgumentConvention::Indirect_Inout:
case SILArgumentConvention::Indirect_InoutAliasable:
Expand All @@ -1006,6 +1026,10 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
// If we don't fit into any of those categories, just track as a liveness
// use. We assume all such uses must only be reads to the memory. So we assert
// to be careful.
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
if (!leafRange)
return false;

LLVM_DEBUG(llvm::dbgs() << "Found liveness use: " << *user);
#ifndef NDEBUG
if (user->mayWriteToMemory()) {
Expand All @@ -1014,8 +1038,7 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
llvm_unreachable("standard failure");
}
#endif
useState.livenessUses.insert(
{user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
useState.livenessUses.insert({user, *leafRange});

return true;
}
Expand All @@ -1036,8 +1059,26 @@ void MoveOnlyChecker::searchForCandidateMarkMustChecks() {
// Skip any alloc_box due to heap to stack failing on a box capture. This
// will just cause an error.
if (auto *pbi = dyn_cast<ProjectBoxInst>(mmci->getOperand())) {
if (isa<AllocBoxInst>(pbi->getOperand())) {
LLVM_DEBUG(
llvm::dbgs()
<< "Early emitting diagnostic for unsupported alloc box!\n");
if (mmci->getType().isMoveOnlyWrapped()) {
diagnose(fn->getASTContext(), mmci->getLoc().getSourceLoc(),
diag::sil_moveonlychecker_not_understand_no_implicit_copy);
} else {
diagnose(fn->getASTContext(), mmci->getLoc().getSourceLoc(),
diag::sil_moveonlychecker_not_understand_moveonly);
}
valuesWithDiagnostics.insert(mmci);
continue;
}

if (auto *bbi = dyn_cast<BeginBorrowInst>(pbi->getOperand())) {
if (isa<AllocBoxInst>(bbi->getOperand())) {
LLVM_DEBUG(
llvm::dbgs()
<< "Early emitting diagnostic for unsupported alloc box!\n");
if (mmci->getType().isMoveOnlyWrapped()) {
diagnose(
fn->getASTContext(), mmci->getLoc().getSourceLoc(),
Expand Down Expand Up @@ -1604,6 +1645,8 @@ bool MoveOnlyChecker::check() {

// Perform our address check.
if (!performSingleCheck(markedValue)) {
LLVM_DEBUG(llvm::dbgs()
<< "Failed to perform single check! Emitting error!\n");
// If we fail the address check in some way, set the diagnose!
if (markedValue->getType().isMoveOnlyWrapped()) {
diagnose(fn->getASTContext(), markedValue->getLoc().getSourceLoc(),
Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/Mandatory/MoveOnlyObjectChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ bool MoveOnlyChecker::searchForCandidateMarkMustChecks() {
auto *mmci = dyn_cast<MarkMustCheckInst>(&*ii);
++ii;

if (!mmci || !mmci->hasMoveCheckerKind())
if (!mmci || !mmci->hasMoveCheckerKind() || !mmci->getType().isObject())
continue;

// Handle guaranteed/owned move arguments and values.
Expand Down
15 changes: 4 additions & 11 deletions test/SILOptimizer/moveonly_addresschecker_diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1836,7 +1836,6 @@ public func closureClassUseAfterConsumeArg(_ argX: inout Klass) {
public func closureCaptureClassUseAfterConsume() {
var x2 = Klass()
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
x2 = Klass()
let f = {
classUseMoveOnlyWithoutEscaping(x2)
Expand All @@ -1849,7 +1848,6 @@ public func closureCaptureClassUseAfterConsume() {
public func closureCaptureClassUseAfterConsumeError() {
var x2 = Klass()
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
x2 = Klass()
let f = {
classUseMoveOnlyWithoutEscaping(x2)
Expand Down Expand Up @@ -1925,9 +1923,8 @@ public func deferCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
public func closureAndDeferCaptureClassUseAfterConsume() {
var x2 = Klass()
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-3 {{'x2' consumed but not reinitialized before end of function}}
// expected-error @-4 {{'x2' consumed more than once}}
// expected-error @-2 {{'x2' consumed but not reinitialized before end of function}}
// expected-error @-3 {{'x2' consumed more than once}}
x2 = Klass()
let f = {
defer {
Expand All @@ -1946,8 +1943,7 @@ public func closureAndDeferCaptureClassUseAfterConsume2() {
var x2 = Klass()
// expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-3 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-4 {{'x2' consumed more than once}}
// expected-error @-3 {{'x2' consumed more than once}}
x2 = Klass()
let f = {
classConsume(x2)
Expand All @@ -1967,8 +1963,7 @@ public func closureAndDeferCaptureClassUseAfterConsume3() {
var x2 = Klass()
// expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-3 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-4 {{'x2' consumed more than once}}
// expected-error @-3 {{'x2' consumed more than once}}
x2 = Klass()
let f = {
classConsume(x2)
Expand Down Expand Up @@ -2008,7 +2003,6 @@ public func closureAndDeferCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
public func closureAndClosureCaptureClassUseAfterConsume() {
var x2 = Klass()
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
x2 = Klass()
let f = {
let g = {
Expand All @@ -2024,7 +2018,6 @@ public func closureAndClosureCaptureClassUseAfterConsume() {
public func closureAndClosureCaptureClassUseAfterConsume2() {
var x2 = Klass()
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
x2 = Klass()
let f = {
let g = {
Expand Down
Loading