Skip to content

Commit 72523cf

Browse files
committed
[move-only] Fix a few bugs around MoveOnlyWrapperToCopyableValue.
The main fixes are: 1. MoveOnlyWrapperToCopyableValue needed to be marked as a FirstArgOwnershipForwardingSingleValueInst instead of just as being an Ownership mixin. I discovered that in certain cases I was treating it that way (in the isa check for FirstArgOwnershipForwardingSingleValueInst), but we were inconsistent. Now we are consistent. 2. MoveOnlyWrapperToCopyableValue is always specified as being initialized as owned or guaranteed. What is key to understand though is that the owned/guaranteed property here is more a semantic property around whether the lifetime of the move only value is ending or if we are allowing it to escape as an moveonlywrapped unwrapped guaranteed parameter to a function. The main implication of this is that we can not just use the actual ownership kind to determine the type of moveonlywrapper_to_copyable we are using. This is b/c after ownership lowering, the resulting ownership kind will be none, meaning the instruction will be in an invalid state. Thus the need to represent this as a separate bit in the instruction. It may make sense to rename the forms of this instruction to be `[lifetime end]` and `[guaranteed function arg]` that way it is semantically clear. But I am going to do that change at another time.
1 parent 4eebe7c commit 72523cf

File tree

3 files changed

+27
-20
lines changed

3 files changed

+27
-20
lines changed

include/swift/SIL/SILBuilder.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,14 +1299,16 @@ class SILBuilder {
12991299
MoveOnlyWrapperToCopyableValueInst *
13001300
createOwnedMoveOnlyWrapperToCopyableValue(SILLocation loc, SILValue src) {
13011301
return insert(new (getModule()) MoveOnlyWrapperToCopyableValueInst(
1302-
*F, getSILDebugLocation(loc), src, OwnershipKind::Owned));
1302+
*F, getSILDebugLocation(loc), src,
1303+
MoveOnlyWrapperToCopyableValueInst::Owned));
13031304
}
13041305

13051306
MoveOnlyWrapperToCopyableValueInst *
13061307
createGuaranteedMoveOnlyWrapperToCopyableValue(SILLocation loc,
13071308
SILValue src) {
13081309
return insert(new (getModule()) MoveOnlyWrapperToCopyableValueInst(
1309-
*F, getSILDebugLocation(loc), src, OwnershipKind::Guaranteed));
1310+
*F, getSILDebugLocation(loc), src,
1311+
MoveOnlyWrapperToCopyableValueInst::Guaranteed));
13101312
}
13111313

13121314
UnconditionalCheckedCastInst *

include/swift/SIL/SILInstruction.h

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7627,19 +7627,29 @@ class CopyableToMoveOnlyWrapperValueInst
76277627
class MoveOnlyWrapperToCopyableValueInst
76287628
: public UnaryInstructionBase<
76297629
SILInstructionKind::MoveOnlyWrapperToCopyableValueInst,
7630-
SingleValueInstruction>,
7631-
public OwnershipForwardingMixin {
7630+
FirstArgOwnershipForwardingSingleValueInst> {
7631+
public:
7632+
enum InitialKind {
7633+
Guaranteed,
7634+
Owned,
7635+
};
7636+
7637+
private:
76327638
friend class SILBuilder;
76337639

7640+
InitialKind initialKind;
7641+
76347642
MoveOnlyWrapperToCopyableValueInst(const SILFunction &fn,
76357643
SILDebugLocation DebugLoc,
7636-
SILValue operand,
7637-
OwnershipKind forwardingOwnershipKind)
7638-
: UnaryInstructionBase(DebugLoc, operand,
7639-
operand->getType().removingMoveOnlyWrapper()),
7640-
OwnershipForwardingMixin(
7641-
SILInstructionKind::MoveOnlyWrapperToCopyableValueInst,
7642-
forwardingOwnershipKind) {}
7644+
SILValue operand, InitialKind kind)
7645+
: UnaryInstructionBase(
7646+
DebugLoc, operand, operand->getType().removingMoveOnlyWrapper(),
7647+
kind == InitialKind::Guaranteed ? OwnershipKind::Guaranteed
7648+
: OwnershipKind::Owned),
7649+
initialKind(kind) {}
7650+
7651+
public:
7652+
InitialKind getInitialKind() const { return initialKind; }
76437653
};
76447654

76457655
/// Given an object reference, return true iff it is non-nil and refers
@@ -9783,8 +9793,7 @@ inline bool OwnershipForwardingMixin::isa(SILInstructionKind kind) {
97839793
OwnershipForwardingConversionInst::classof(kind) ||
97849794
OwnershipForwardingSelectEnumInstBase::classof(kind) ||
97859795
OwnershipForwardingMultipleValueInstruction::classof(kind) ||
9786-
kind == SILInstructionKind::MarkMustCheckInst ||
9787-
kind == SILInstructionKind::MoveOnlyWrapperToCopyableValueInst;
9796+
kind == SILInstructionKind::MarkMustCheckInst;
97889797
}
97899798

97909799
inline OwnershipForwardingMixin *

lib/SIL/IR/SILPrinter.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1921,15 +1921,11 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
19211921

19221922
void visitMoveOnlyWrapperToCopyableValueInst(
19231923
MoveOnlyWrapperToCopyableValueInst *I) {
1924-
switch (I->getForwardingOwnershipKind()) {
1925-
case OwnershipKind::None:
1926-
case OwnershipKind::Any:
1927-
case OwnershipKind::Unowned:
1928-
llvm_unreachable("Move only values are always non-trivial");
1929-
case OwnershipKind::Owned:
1924+
switch (I->getInitialKind()) {
1925+
case MoveOnlyWrapperToCopyableValueInst::Owned:
19301926
*this << "[owned] ";
19311927
break;
1932-
case OwnershipKind::Guaranteed:
1928+
case MoveOnlyWrapperToCopyableValueInst::Guaranteed:
19331929
*this << "[guaranteed] ";
19341930
break;
19351931
}

0 commit comments

Comments
 (0)