Skip to content

Commit 3316a58

Browse files
committed
Allow move_value of trivial values.
Required for SIL level local variable scopes.
1 parent 075d3a4 commit 3316a58

File tree

5 files changed

+21
-9
lines changed

5 files changed

+21
-9
lines changed

include/swift/SIL/SILBuilder.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1459,7 +1459,7 @@ class SILBuilder {
14591459
HasPointerEscape_t hasPointerEscape = DoesNotHavePointerEscape,
14601460
IsFromVarDecl_t fromVarDecl = IsNotFromVarDecl) {
14611461
assert(getFunction().hasOwnership());
1462-
assert(!operand->getType().isTrivial(getFunction()) &&
1462+
assert(fromVarDecl || !operand->getType().isTrivial(getFunction()) &&
14631463
"Should not be passing trivial values to this api. Use instead "
14641464
"emitMoveValueOperation");
14651465
return insert(new (getModule())

include/swift/SIL/SILValue.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -610,9 +610,6 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
610610
bool hasDebugTrace() const;
611611

612612
/// Does this SILValue begin a VarDecl scope? Only true in OSSA.
613-
///
614-
/// If this is true, the value must be a SingleValueInstruction whose first
615-
/// operand is the variable's initial value.
616613
bool isFromVarDecl();
617614

618615
static bool classof(SILNodePointer node) {

lib/SIL/IR/TypeLowering.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,6 +1061,15 @@ namespace {
10611061

10621062
void emitDestroyValue(SILBuilder &B, SILLocation loc,
10631063
SILValue value) const override {
1064+
if (B.getFunction().hasOwnership()
1065+
&& B.getModule().getStage() == SILStage::Raw
1066+
&& value->isFromVarDecl()) {
1067+
// Do not use destroy_value for trivial values. The lifetime introducer
1068+
// may be implicitly copied and used outside of its original scope,
1069+
// which violates the invariants of destroy_value.
1070+
B.createExtendLifetime(loc, value);
1071+
return;
1072+
}
10641073
// Trivial
10651074
}
10661075
};

lib/SIL/IR/ValueOwnership.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ CONSTANT_OWNERSHIP_INST(Owned, CopyBlock)
8585
CONSTANT_OWNERSHIP_INST(Owned, CopyBlockWithoutEscaping)
8686
CONSTANT_OWNERSHIP_INST(Owned, CopyValue)
8787
CONSTANT_OWNERSHIP_INST(Owned, ExplicitCopyValue)
88-
CONSTANT_OWNERSHIP_INST(Owned, MoveValue)
8988
CONSTANT_OWNERSHIP_INST(Owned, EndCOWMutation)
9089
CONSTANT_OWNERSHIP_INST(Owned, EndInitLetRef)
9190
CONSTANT_OWNERSHIP_INST(Owned, BeginDeallocRef)
@@ -212,7 +211,9 @@ CONSTANT_OR_NONE_OWNERSHIP_INST(Guaranteed, OpenExistentialBoxValue)
212211
// value).
213212
CONSTANT_OR_NONE_OWNERSHIP_INST(Owned, MarkUninitialized)
214213

215-
// unchecked_bitwise_cast is a bitwise copy. It produces a trivial or unowned
214+
// In raw SIL, a MoveValue delimits the scope of trivial variables.
215+
CONSTANT_OR_NONE_OWNERSHIP_INST(Owned, MoveValue)
216+
216217
// result.
217218
//
218219
// If the operand is nontrivial and the result is trivial, then it is the

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2702,8 +2702,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
27022702
}
27032703

27042704
void checkExtendLifetimeInst(ExtendLifetimeInst *I) {
2705-
require(!I->getOperand()->getType().isTrivial(*I->getFunction()),
2706-
"Source value should be non-trivial");
2705+
require(!I->getOperand()->getType().isTrivial(*I->getFunction())
2706+
|| F.getModule().getStage() == SILStage::Raw,
2707+
"Source value should be non-trivial after diagnostics");
27072708
require(F.hasOwnership(),
27082709
"extend_lifetime is only valid in functions with qualified "
27092710
"ownership");
@@ -3403,7 +3404,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
34033404
void checkDestroyValueInst(DestroyValueInst *I) {
34043405
require(I->getOperand()->getType().isObject(),
34053406
"Source value should be an object value");
3406-
require(!I->getOperand()->getType().isTrivial(*I->getFunction()),
3407+
require(!I->getOperand()->getType().isTrivial(*I->getFunction())
3408+
|| I->getOperand()->isFromVarDecl(),
34073409
"Source value should be non-trivial");
34083410
require(!fnConv.useLoweredAddresses() || F.hasOwnership(),
34093411
"destroy_value is only valid in functions with qualified "
@@ -6779,6 +6781,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
67796781

67806782
} else if (i.isDeallocatingStack()) {
67816783
SILValue op = i.getOperand(0);
6784+
while (auto *mvi = dyn_cast<MoveValueInst>(op)) {
6785+
op = mvi->getOperand();
6786+
}
67826787
require(!state.Stack.empty(),
67836788
"stack dealloc with empty stack");
67846789
if (op != state.Stack.back()) {

0 commit comments

Comments
 (0)