Skip to content

[no-implicit-copy] Update SILGen/move checker to work with new patterns from copyable_to_moveonly and friends. #59611

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 3 commits into from
Jun 28, 2022
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
13 changes: 13 additions & 0 deletions include/swift/SIL/SILValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,19 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {

bool isLexical() const;

/// Unsafely eliminate moveonly from this value's type. Returns true if the
/// value's underlying type was move only and thus was changed. Returns false
/// otherwise.
///
/// NOTE: Please do not use this directly! It is only meant to be used by the
/// optimizer pass: SILMoveOnlyTypeEliminator.
bool unsafelyEliminateMoveOnlyWrapper() {
if (!Type.isMoveOnlyWrapped())
return false;
Type = Type.removingMoveOnlyWrapper();
return true;
}

static bool classof(SILNodePointer node) {
return node->getKind() >= SILNodeKind::First_ValueBase &&
node->getKind() <= SILNodeKind::Last_ValueBase;
Expand Down
4 changes: 4 additions & 0 deletions include/swift/SILOptimizer/PassManager/Passes.def
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,10 @@ PASS(MoveOnlyChecker, "sil-move-only-checker",
PASS(MoveKillsCopyableValuesChecker, "sil-move-kills-copyable-values-checker",
"Pass that checks that any copyable (non-move only) value that is passed "
"to _move do not have any uses later than the _move")
PASS(TrivialMoveOnlyTypeEliminator, "sil-trivial-move-only-type-eliminator",
"Pass that rewrites SIL to remove move only types from values of trivial type")
PASS(MoveOnlyTypeEliminator, "sil-move-only-type-eliminator",
"Pass that rewrites SIL to remove move only types from all values")
PASS(LexicalLifetimeEliminator, "sil-lexical-lifetime-eliminator",
"Pass that removes lexical lifetime markers from borrows and alloc stack")
PASS(MoveKillsCopyableAddressesChecker, "sil-move-kills-copyable-addresses-checker",
Expand Down
41 changes: 39 additions & 2 deletions lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1784,8 +1784,33 @@ static void emitRawApply(SILGenFunction &SGF,

// Gather the arguments.
for (auto i : indices(args)) {
auto argValue = (inputParams[i].isConsumed() ? args[i].forward(SGF)
: args[i].getValue());
SILValue argValue;
if (inputParams[i].isConsumed()) {
argValue = args[i].forward(SGF);
if (argValue->getType().isMoveOnlyWrapped()) {
argValue =
SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(loc, argValue);
}
} else {
ManagedValue arg = args[i];

// Move only is not represented in the Swift level type system, so if we
// have a move only value, convert it to a non-move only value. The
// move/is no escape checkers will ensure that it is legal to do this or
// will error. At this point we just want to make sure that the emitted
// types line up.
if (arg.getType().isMoveOnlyWrapped()) {
// We need to borrow so that we can convert from $@moveOnly T -> $T. Use
// a formal access borrow to ensure that we have tight scopes like we do
// when we borrow fn.
if (!arg.isPlusZero())
arg = arg.formalAccessBorrow(SGF, loc);
arg = SGF.B.createGuaranteedMoveOnlyWrapperToCopyableValue(loc, arg);
}

argValue = arg.getValue();
}

#ifndef NDEBUG
auto inputTy =
substFnConv.getSILType(inputParams[i], SGF.getTypeExpansionContext());
Expand Down Expand Up @@ -3057,6 +3082,18 @@ class ArgEmitter {

// If it's not already in memory, put it there.
if (!result.getType().isAddress()) {
// If we have a move only wrapped type, we need to unwrap before we
// materialize. We will forward as appropriate so it will show up as a
// consuming use or a guaranteed use as appropriate.
if (result.getType().isMoveOnlyWrapped()) {
if (result.isPlusOne(SGF)) {
result =
SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(loc, result);
} else {
result = SGF.B.createGuaranteedMoveOnlyWrapperToCopyableValue(
loc, result);
}
}
result = result.materialize(SGF, loc);
}

Expand Down
39 changes: 39 additions & 0 deletions lib/SILGen/SILGenBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include "SILGenBuilder.h"
#include "ArgumentSource.h"
#include "Cleanup.h"
#include "RValue.h"
#include "SILGenFunction.h"
#include "Scope.h"
Expand Down Expand Up @@ -450,6 +451,9 @@ static ManagedValue createInputFunctionArgument(SILGenBuilder &B, SILType type,
case SILArgumentConvention::Direct_Unowned:
// Unowned parameters are only guaranteed at the instant of the call, so we
// must retain them even if we're in a context that can accept a +0 value.
//
// NOTE: If we have a trivial value, the copy will do nothing, so this is
// just a convenient way to avoid writing conditional code.
return ManagedValue::forUnmanaged(arg).copy(SGF, loc);

case SILArgumentConvention::Direct_Owned:
Expand Down Expand Up @@ -882,3 +886,38 @@ ManagedValue SILGenBuilder::createMarkDependence(SILLocation loc,
base.forward(getSILGenFunction()));
return cloner.clone(mdi);
}

ManagedValue SILGenBuilder::createBeginBorrow(SILLocation loc,
ManagedValue value,
bool isLexical) {
auto *newValue =
SILBuilder::createBeginBorrow(loc, value.getValue(), isLexical);
SGF.emitManagedBorrowedRValueWithCleanup(newValue);
return ManagedValue::forUnmanaged(newValue);
}

ManagedValue SILGenBuilder::createMoveValue(SILLocation loc,
ManagedValue value) {
assert(value.isPlusOne(SGF) && "Must be +1 to be moved!");
CleanupCloner cloner(*this, value);
auto *mdi = createMoveValue(loc, value.forward(getSILGenFunction()));
return cloner.clone(mdi);
}

ManagedValue
SILGenBuilder::createOwnedMoveOnlyWrapperToCopyableValue(SILLocation loc,
ManagedValue value) {
assert(value.isPlusOne(SGF) && "Argument must be at +1!");
CleanupCloner cloner(*this, value);
auto *mdi = createOwnedMoveOnlyWrapperToCopyableValue(
loc, value.forward(getSILGenFunction()));
return cloner.clone(mdi);
}

ManagedValue SILGenBuilder::createGuaranteedMoveOnlyWrapperToCopyableValue(
SILLocation loc, ManagedValue value) {
auto *mdi =
createGuaranteedMoveOnlyWrapperToCopyableValue(loc, value.getValue());
assert(mdi->getOperand()->getType().isObject() && "Expected an object?!");
return ManagedValue::forUnmanaged(mdi);
}
33 changes: 31 additions & 2 deletions lib/SILGen/SILGenBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@
#include "JumpDest.h"
#include "ManagedValue.h"
#include "RValue.h"
#include "swift/SIL/SILBuilder.h"
#include "swift/Basic/ProfileCounter.h"
#include "swift/SIL/SILBuilder.h"
#include "swift/SIL/SILLocation.h"

namespace swift {
namespace Lowering {
Expand Down Expand Up @@ -362,7 +363,19 @@ class SILGenBuilder : public SILBuilder {
BranchInst *createBranch(SILLocation Loc, SILBasicBlock *TargetBlock,
ArrayRef<ManagedValue> Args);

using SILBuilder::createReturn;
ReturnInst *createReturn(SILLocation Loc, SILValue ReturnValue) {
// If we have a move only type as our "result type", convert it back to
// being a copyable type. Move only types are never returned today and we
// will rely on the SIL level move only and no escape checker to validate
// that this is a correct usage. So just make the types line up.
if (ReturnValue->getType().isMoveOnlyWrapped()) {
auto cvtLoc = RegularLocation::getAutoGeneratedLocation();
ReturnValue =
createOwnedMoveOnlyWrapperToCopyableValue(cvtLoc, ReturnValue);
}
return SILBuilder::createReturn(Loc, ReturnValue);
}

ReturnInst *createReturn(SILLocation Loc, ManagedValue ReturnValue);

ReturnInst *createReturn(SILLocation Loc, SILValue ReturnValue,
Expand All @@ -385,6 +398,22 @@ class SILGenBuilder : public SILBuilder {
using SILBuilder::createMarkDependence;
ManagedValue createMarkDependence(SILLocation loc, ManagedValue value,
ManagedValue base);

using SILBuilder::createBeginBorrow;
ManagedValue createBeginBorrow(SILLocation loc, ManagedValue value,
bool isLexical = false);

using SILBuilder::createMoveValue;
ManagedValue createMoveValue(SILLocation loc, ManagedValue value);

using SILBuilder::createOwnedMoveOnlyWrapperToCopyableValue;
ManagedValue createOwnedMoveOnlyWrapperToCopyableValue(SILLocation loc,
ManagedValue value);

using SILBuilder::createGuaranteedMoveOnlyWrapperToCopyableValue;
ManagedValue
createGuaranteedMoveOnlyWrapperToCopyableValue(SILLocation loc,
ManagedValue value);
};

} // namespace Lowering
Expand Down
Loading