Skip to content

Commit 4e9da29

Browse files
committed
[move-only] Verify drop_deinit
drop_deinit only exists in ownership SIL. Remove IRGen support. A drop_deinit can only ever be destroyed or destructured. A destructure of a struct-with-deinit requires a drop_deinit operand. (cherry picked from commit 3efd122)
1 parent 7b55a14 commit 4e9da29

File tree

4 files changed

+69
-5
lines changed

4 files changed

+69
-5
lines changed

include/swift/SIL/InstructionUtils.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ SILValue stripCastsWithoutMarkDependence(SILValue V);
4040
/// begin_borrow instructions.
4141
SILValue lookThroughOwnershipInsts(SILValue v);
4242

43+
/// Reverse of lookThroughOwnershipInsts.
44+
///
45+
/// Return true if \p visitor returned true for all uses.
46+
bool visitNonOwnershipUses(SILValue value,
47+
function_ref<bool(Operand *)> visitor);
48+
4349
/// Return the underlying SILValue after looking through all copy_value
4450
/// instructions.
4551
SILValue lookThroughCopyValueInsts(SILValue v);

lib/IRGen/IRGenSIL.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,8 +1249,7 @@ class IRGenSILFunction :
12491249
setLoweredExplosion(i, e);
12501250
}
12511251
void visitDropDeinitInst(DropDeinitInst *i) {
1252-
auto e = getLoweredExplosion(i->getOperand());
1253-
setLoweredExplosion(i, e);
1252+
llvm_unreachable("only valid in ownership SIL");
12541253
}
12551254
void visitMarkMustCheckInst(MarkMustCheckInst *i) {
12561255
llvm_unreachable("Invalid in Lowered SIL");

lib/SIL/Utils/InstructionUtils.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,30 @@ SILValue swift::lookThroughOwnershipInsts(SILValue v) {
3939
}
4040
}
4141

42+
bool swift::visitNonOwnershipUses(SILValue value,
43+
function_ref<bool(Operand *)> visitor) {
44+
// All ownership insts have a single operand, so a recursive walk is
45+
// sufficient and cannot revisit operands.
46+
for (Operand *use : value->getUses()) {
47+
auto *user = use->getUser();
48+
switch (user->getKind()) {
49+
default:
50+
if (!visitor(use))
51+
return false;
52+
53+
break;
54+
case SILInstructionKind::MoveValueInst:
55+
case SILInstructionKind::CopyValueInst:
56+
case SILInstructionKind::BeginBorrowInst:
57+
if (!visitNonOwnershipUses(cast<SingleValueInstruction>(user), visitor))
58+
return false;
59+
60+
break;
61+
}
62+
}
63+
return true;
64+
}
65+
4266
SILValue swift::lookThroughCopyValueInsts(SILValue val) {
4367
while (auto *cvi =
4468
dyn_cast_or_null<CopyValueInst>(val->getDefiningInstruction())) {

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3547,6 +3547,13 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
35473547
"destructure with none ownership kind operand and non-none "
35483548
"ownership kind result?!");
35493549
}
3550+
if (operandTy.getNominalOrBoundGenericNominal()
3551+
->getValueTypeDestructor()) {
3552+
require(
3553+
isa<DropDeinitInst>(lookThroughOwnershipInsts(DSI->getOperand())),
3554+
"a destructure of a move-only-type-with-deinit requires a "
3555+
"drop_deinit");
3556+
}
35503557
}
35513558
}
35523559

@@ -5968,12 +5975,40 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
59685975
"Result and operand must have the same type, today.");
59695976
}
59705977

5978+
// check that a drop_deinit can only ever be destroyed or destructured
5979+
void checkDropDeinitUses(DropDeinitInst *ddi) {
5980+
// Address-type drop_deinit has no special structural requirements. It just
5981+
// sits there and blocks optimization on the allocation and downstream uses
5982+
// of the address. If we want to optimize around address-type drop_deinit,
5983+
// then we need a seperate verifier for its requirements.
5984+
if (ddi->getType().isAddress())
5985+
return;
5986+
5987+
visitNonOwnershipUses(ddi, [&](Operand *use) {
5988+
auto *user = use->getUser();
5989+
require(isa<DestroyValueInst>(user)
5990+
|| isa<EndLifetimeInst>(user)
5991+
|| isa<DestructureStructInst>(user)
5992+
|| isa<SwitchEnumInst>(user),
5993+
"A drop_deinit can only be destroyed or destructured");
5994+
return true;
5995+
});
5996+
}
5997+
59715998
void checkDropDeinitInst(DropDeinitInst *ddi) {
5972-
require(ddi->getType() == ddi->getOperand()->getType(),
5999+
require(F.hasOwnership(), "drop_deinit only allowed in OSSA");
6000+
6001+
auto type = ddi->getType();
6002+
require(type == ddi->getOperand()->getType(),
59736003
"Result and operand must have the same type.");
5974-
require(ddi->getType().isMoveOnlyNominalType(),
6004+
require(type.isMoveOnlyNominalType(),
59756005
"drop_deinit only allowed for move-only types");
5976-
require(F.hasOwnership(), "drop_deinit only allowed in OSSA");
6006+
require(type.getNominalOrBoundGenericNominal()
6007+
->getValueTypeDestructor(), "drop_deinit only allowed for "
6008+
"struct/enum types that define a deinit");
6009+
assert(!type.isTrivial(F) && "a type with a deinit is nontrivial");
6010+
6011+
checkDropDeinitUses(ddi);
59776012
}
59786013

59796014
void checkMarkMustCheckInst(MarkMustCheckInst *i) {

0 commit comments

Comments
 (0)