From 7d70a70acfa9c1339fea3fc9a6731060cae56c08 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Thu, 6 Apr 2023 16:09:45 +0200 Subject: [PATCH 1/5] SIL: add the `drop_deinit` instruction his instruction is a marker for a following destroy instruction to suppress the call of the move-only type's deinitializer. --- .../Sources/SIL/Instruction.swift | 4 ++++ .../Sources/SIL/Registration.swift | 1 + docs/SIL.rst | 23 +++++++++++++++++++ include/swift/SIL/MemAccessUtils.h | 1 + include/swift/SIL/SILBuilder.h | 8 +++++++ include/swift/SIL/SILCloner.h | 11 +++++++++ include/swift/SIL/SILInstruction.h | 9 ++++++++ include/swift/SIL/SILNodes.def | 2 ++ lib/IRGen/IRGenSIL.cpp | 4 ++++ lib/SIL/IR/OperandOwnership.cpp | 6 +++++ lib/SIL/IR/SILPrinter.cpp | 4 ++++ lib/SIL/IR/ValueOwnership.cpp | 4 ++++ lib/SIL/Parser/ParseSIL.cpp | 9 ++++++++ lib/SIL/Utils/InstructionUtils.cpp | 1 + lib/SIL/Utils/MemAccessUtils.cpp | 1 + lib/SIL/Verifier/SILVerifier.cpp | 8 +++++++ .../Mandatory/OwnershipModelEliminator.cpp | 4 ++++ .../UtilityPasses/SerializeSILPass.cpp | 1 + lib/SILOptimizer/Utils/SILInliner.cpp | 1 + lib/Serialization/DeserializeSIL.cpp | 8 +++++++ lib/Serialization/ModuleFormat.h | 2 +- lib/Serialization/SerializeSIL.cpp | 1 + test/SIL/Parser/basic.sil | 16 +++++++++++++ test/SIL/Serialization/basic.sil | 16 +++++++++++++ 24 files changed, 144 insertions(+), 1 deletion(-) diff --git a/SwiftCompilerSources/Sources/SIL/Instruction.swift b/SwiftCompilerSources/Sources/SIL/Instruction.swift index 7cd4c84797ad9..74471ab4f4ab0 100644 --- a/SwiftCompilerSources/Sources/SIL/Instruction.swift +++ b/SwiftCompilerSources/Sources/SIL/Instruction.swift @@ -647,6 +647,10 @@ final public class MoveValueInst : SingleValueInstruction, UnaryInstruction { public var fromValue: Value { operand.value } } +final public class DropDeinitInst : SingleValueInstruction, UnaryInstruction { + public var fromValue: Value { operand.value } +} + final public class StrongCopyUnownedValueInst : SingleValueInstruction, UnaryInstruction {} final public class StrongCopyUnmanagedValueInst : SingleValueInstruction, UnaryInstruction {} diff --git a/SwiftCompilerSources/Sources/SIL/Registration.swift b/SwiftCompilerSources/Sources/SIL/Registration.swift index 39193380a9a70..cfdb890b9827f 100644 --- a/SwiftCompilerSources/Sources/SIL/Registration.swift +++ b/SwiftCompilerSources/Sources/SIL/Registration.swift @@ -125,6 +125,7 @@ public func registerSILClasses() { register(ProjectBoxInst.self) register(CopyValueInst.self) register(MoveValueInst.self) + register(DropDeinitInst.self) register(EndCOWMutationInst.self) register(ClassifyBridgeObjectInst.self) register(PartialApplyInst.self) diff --git a/docs/SIL.rst b/docs/SIL.rst index 8b59f4e5662ab..a14de96147aa7 100644 --- a/docs/SIL.rst +++ b/docs/SIL.rst @@ -6117,6 +6117,29 @@ a type `T` into the move only value space. The ``lexical`` attribute specifies that the value corresponds to a local variable in the Swift source. + +drop_deinit +``````````` + +:: + + sil-instruction ::= 'drop_deinit' sil-operand + + %1 = drop_deinit %0 : $T + // T must be a move-only type + // %1 is an @owned T + %3 = drop_deinit %2 : $*T + // T must be a move-only type + // %2 has type *T + +This instruction is a marker for a following destroy instruction to suppress +the call of the move-only type's deinitializer. +The instruction accepts an object or address type. +If its argument is an object type it takes in an `@owned T` and produces a new +`@owned T`. If its argument is an address type, it's an identity projection. + +The instruction is only valid in ownership SIL. + release_value ````````````` diff --git a/include/swift/SIL/MemAccessUtils.h b/include/swift/SIL/MemAccessUtils.h index bfae95eb68c1d..1b87602105841 100644 --- a/include/swift/SIL/MemAccessUtils.h +++ b/include/swift/SIL/MemAccessUtils.h @@ -1632,6 +1632,7 @@ inline bool isAccessStorageIdentityCast(SingleValueInstruction *svi) { // Simply pass-thru the incoming address. case SILInstructionKind::MarkUninitializedInst: case SILInstructionKind::MarkMustCheckInst: + case SILInstructionKind::DropDeinitInst: case SILInstructionKind::MarkUnresolvedReferenceBindingInst: case SILInstructionKind::MarkDependenceInst: case SILInstructionKind::CopyValueInst: diff --git a/include/swift/SIL/SILBuilder.h b/include/swift/SIL/SILBuilder.h index 0506fa059a8a4..61fae5cf0a3b3 100644 --- a/include/swift/SIL/SILBuilder.h +++ b/include/swift/SIL/SILBuilder.h @@ -1355,6 +1355,14 @@ class SILBuilder { operand, isLexical)); } + DropDeinitInst *createDropDeinit(SILLocation loc, SILValue operand) { + assert(getFunction().hasOwnership()); + assert(!operand->getType().isTrivial(getFunction()) && + "Should not be passing trivial values to this api."); + return insert(new (getModule()) DropDeinitInst(getSILDebugLocation(loc), + operand)); + } + MarkUnresolvedMoveAddrInst *createMarkUnresolvedMoveAddr(SILLocation loc, SILValue srcAddr, SILValue takeAddr) { diff --git a/include/swift/SIL/SILCloner.h b/include/swift/SIL/SILCloner.h index bcbcca0ef54a5..4ba596c70e204 100644 --- a/include/swift/SIL/SILCloner.h +++ b/include/swift/SIL/SILCloner.h @@ -1888,6 +1888,17 @@ void SILCloner::visitMoveValueInst(MoveValueInst *Inst) { recordClonedInstruction(Inst, MVI); } +template +void SILCloner::visitDropDeinitInst(DropDeinitInst *Inst) { + getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope())); + if (!getBuilder().hasOwnership()) { + return recordFoldedValue(Inst, getOpValue(Inst->getOperand())); + } + auto *MVI = getBuilder().createDropDeinit(getOpLocation(Inst->getLoc()), + getOpValue(Inst->getOperand())); + recordClonedInstruction(Inst, MVI); +} + template void SILCloner::visitMarkMustCheckInst(MarkMustCheckInst *Inst) { getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope())); diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index d09045c17d204..c0bc46b004361 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -8268,6 +8268,15 @@ class MoveValueInst void removeIsLexical() { lexical = false; } }; +class DropDeinitInst + : public UnaryInstructionBase { + friend class SILBuilder; + + DropDeinitInst(SILDebugLocation DebugLoc, SILValue operand) + : UnaryInstructionBase(DebugLoc, operand, operand->getType()) {} +}; + /// Equivalent to a copy_addr to [init] except that it is used for diagnostics /// and should not be pattern matched. During the diagnostic passes, the "move /// function" checker for addresses always converts this to a copy_addr [init] diff --git a/include/swift/SIL/SILNodes.def b/include/swift/SIL/SILNodes.def index eb7798942794e..1ef15f6fc6012 100644 --- a/include/swift/SIL/SILNodes.def +++ b/include/swift/SIL/SILNodes.def @@ -461,6 +461,8 @@ ABSTRACT_VALUE_AND_INST(SingleValueInstruction, ValueBase, SILInstruction) // effects relative to other OSSA values like copy_value. SINGLE_VALUE_INST(MoveValueInst, move_value, SingleValueInstruction, None, DoesNotRelease) + SINGLE_VALUE_INST(DropDeinitInst, drop_deinit, SingleValueInstruction, None, + DoesNotRelease) // A canary value inserted by a SIL generating frontend to signal to the move // checker to check a specific value. Valid only in Raw SIL. The relevant // checkers should remove the mark_must_check instruction after successfully diff --git a/lib/IRGen/IRGenSIL.cpp b/lib/IRGen/IRGenSIL.cpp index 6a1e0fe1234f6..4e11c5d8ccd6a 100644 --- a/lib/IRGen/IRGenSIL.cpp +++ b/lib/IRGen/IRGenSIL.cpp @@ -1222,6 +1222,10 @@ class IRGenSILFunction : auto e = getLoweredExplosion(i->getOperand()); setLoweredExplosion(i, e); } + void visitDropDeinitInst(DropDeinitInst *i) { + auto e = getLoweredExplosion(i->getOperand()); + setLoweredExplosion(i, e); + } void visitMarkMustCheckInst(MarkMustCheckInst *i) { llvm_unreachable("Invalid in Lowered SIL"); } diff --git a/lib/SIL/IR/OperandOwnership.cpp b/lib/SIL/IR/OperandOwnership.cpp index eff862b2478d3..6155967241cb5 100644 --- a/lib/SIL/IR/OperandOwnership.cpp +++ b/lib/SIL/IR/OperandOwnership.cpp @@ -455,6 +455,12 @@ OperandOwnershipClassifier::visitStoreBorrowInst(StoreBorrowInst *i) { return OperandOwnership::TrivialUse; } +OperandOwnership +OperandOwnershipClassifier::visitDropDeinitInst(DropDeinitInst *i) { + return i->getType().isAddress() ? OperandOwnership::TrivialUse + : OperandOwnership::ForwardingConsume; +} + // Get the OperandOwnership for instantaneous apply, yield, and return uses. // This does not apply to uses that begin an explicit borrow scope in the // caller, such as begin_apply. diff --git a/lib/SIL/IR/SILPrinter.cpp b/lib/SIL/IR/SILPrinter.cpp index 2077a1b38dda8..98b5ea2f8f57f 100644 --- a/lib/SIL/IR/SILPrinter.cpp +++ b/lib/SIL/IR/SILPrinter.cpp @@ -1996,6 +1996,10 @@ class SILPrinter : public SILInstructionVisitor { *this << getIDAndType(I->getOperand()); } + void visitDropDeinitInst(DropDeinitInst *I) { + *this << getIDAndType(I->getOperand()); + } + void visitMarkMustCheckInst(MarkMustCheckInst *I) { using CheckKind = MarkMustCheckInst::CheckKind; switch (I->getCheckKind()) { diff --git a/lib/SIL/IR/ValueOwnership.cpp b/lib/SIL/IR/ValueOwnership.cpp index e9889cd1325ea..1d5898202fa69 100644 --- a/lib/SIL/IR/ValueOwnership.cpp +++ b/lib/SIL/IR/ValueOwnership.cpp @@ -354,6 +354,10 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitLoadInst(LoadInst *LI) { llvm_unreachable("Unhandled LoadOwnershipQualifier in switch."); } +ValueOwnershipKind ValueOwnershipKindClassifier::visitDropDeinitInst(DropDeinitInst *ddi) { + return ddi->getType().isAddress() ? OwnershipKind::None : OwnershipKind::Owned; +} + ValueOwnershipKind ValueOwnershipKindClassifier::visitPartialApplyInst(PartialApplyInst *PA) { // partial_apply instructions are modeled as creating an owned value during diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp index 5e7cdb997f904..c7c0a6bf49095 100644 --- a/lib/SIL/Parser/ParseSIL.cpp +++ b/lib/SIL/Parser/ParseSIL.cpp @@ -3698,6 +3698,15 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B, break; } + case SILInstructionKind::DropDeinitInst: { + if (parseTypedValueRef(Val, B)) + return true; + if (parseSILDebugLocation(InstLoc, B)) + return true; + ResultVal = B.createDropDeinit(InstLoc, Val); + break; + } + case SILInstructionKind::MarkMustCheckInst: { StringRef AttrName; if (!parseSILOptional(AttrName, *this)) { diff --git a/lib/SIL/Utils/InstructionUtils.cpp b/lib/SIL/Utils/InstructionUtils.cpp index 66c34911cdfb6..fef40e264b471 100644 --- a/lib/SIL/Utils/InstructionUtils.cpp +++ b/lib/SIL/Utils/InstructionUtils.cpp @@ -441,6 +441,7 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType) case SILInstructionKind::ValueToBridgeObjectInst: case SILInstructionKind::MarkDependenceInst: case SILInstructionKind::MoveValueInst: + case SILInstructionKind::DropDeinitInst: case SILInstructionKind::MarkMustCheckInst: case SILInstructionKind::MarkUnresolvedReferenceBindingInst: case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst: diff --git a/lib/SIL/Utils/MemAccessUtils.cpp b/lib/SIL/Utils/MemAccessUtils.cpp index 367b37ff79531..4656fefa37c80 100644 --- a/lib/SIL/Utils/MemAccessUtils.cpp +++ b/lib/SIL/Utils/MemAccessUtils.cpp @@ -1864,6 +1864,7 @@ AccessPathDefUseTraversal::visitSingleValueUser(SingleValueInstruction *svi, return IgnoredUse; } + case SILInstructionKind::DropDeinitInst: case SILInstructionKind::MarkMustCheckInst: { // Mark must check goes on the project_box, so it isn't a ref. assert(!dfs.isRef()); diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp index d7d614a335ba6..74cd73d68aae5 100644 --- a/lib/SIL/Verifier/SILVerifier.cpp +++ b/lib/SIL/Verifier/SILVerifier.cpp @@ -5915,6 +5915,14 @@ class SILVerifier : public SILVerifierBase { "Result and operand must have the same type, today."); } + void checkDropDeinitInst(DropDeinitInst *ddi) { + require(ddi->getType() == ddi->getOperand()->getType(), + "Result and operand must have the same type."); + require(ddi->getType().isMoveOnlyNominalType(), + "drop_deinit only allowed for move-only types"); + require(F.hasOwnership(), "drop_deinit only allowed in OSSA"); + } + void checkMarkMustCheckInst(MarkMustCheckInst *i) { require(i->getModule().getStage() == SILStage::Raw, "Only valid in Raw SIL! Should have been eliminated by /some/ " diff --git a/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp b/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp index 53373140a7129..029f0979b3794 100644 --- a/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp +++ b/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp @@ -141,6 +141,10 @@ struct OwnershipModelEliminatorVisitor eraseInstructionAndRAUW(mvi, mvi->getOperand()); return true; } + bool visitDropDeinitInst(DropDeinitInst *ddi) { + eraseInstructionAndRAUW(ddi, ddi->getOperand()); + return true; + } bool visitBeginBorrowInst(BeginBorrowInst *bbi) { eraseInstructionAndRAUW(bbi, bbi->getOperand()); return true; diff --git a/lib/SILOptimizer/UtilityPasses/SerializeSILPass.cpp b/lib/SILOptimizer/UtilityPasses/SerializeSILPass.cpp index fc9c41d4eab09..f3359e9ee6098 100644 --- a/lib/SILOptimizer/UtilityPasses/SerializeSILPass.cpp +++ b/lib/SILOptimizer/UtilityPasses/SerializeSILPass.cpp @@ -204,6 +204,7 @@ static bool hasOpaqueArchetype(TypeExpansionContext context, case SILInstructionKind::CopyValueInst: case SILInstructionKind::ExplicitCopyValueInst: case SILInstructionKind::MoveValueInst: + case SILInstructionKind::DropDeinitInst: case SILInstructionKind::MarkMustCheckInst: case SILInstructionKind::MarkUnresolvedReferenceBindingInst: case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst: diff --git a/lib/SILOptimizer/Utils/SILInliner.cpp b/lib/SILOptimizer/Utils/SILInliner.cpp index 5daa6861f2401..1861648efecd2 100644 --- a/lib/SILOptimizer/Utils/SILInliner.cpp +++ b/lib/SILOptimizer/Utils/SILInliner.cpp @@ -876,6 +876,7 @@ InlineCost swift::instructionInlineCost(SILInstruction &I) { case SILInstructionKind::BindMemoryInst: case SILInstructionKind::RebindMemoryInst: case SILInstructionKind::MoveValueInst: + case SILInstructionKind::DropDeinitInst: case SILInstructionKind::MarkMustCheckInst: case SILInstructionKind::MarkUnresolvedReferenceBindingInst: case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst: diff --git a/lib/Serialization/DeserializeSIL.cpp b/lib/Serialization/DeserializeSIL.cpp index a5042ff1ea7ad..5fff4c599a7b8 100644 --- a/lib/Serialization/DeserializeSIL.cpp +++ b/lib/Serialization/DeserializeSIL.cpp @@ -2200,6 +2200,14 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn, break; } + case SILInstructionKind::DropDeinitInst: { + auto Ty = MF->getType(TyID); + ResultInst = Builder.createDropDeinit( + Loc, + getLocalValue(ValID, getSILType(Ty, (SILValueCategory)TyCategory, Fn))); + break; + } + case SILInstructionKind::MarkUnresolvedReferenceBindingInst: { using Kind = MarkUnresolvedReferenceBindingInst::Kind; auto ty = MF->getType(TyID); diff --git a/lib/Serialization/ModuleFormat.h b/lib/Serialization/ModuleFormat.h index de32222315e48..121dd2c5103c8 100644 --- a/lib/Serialization/ModuleFormat.h +++ b/lib/Serialization/ModuleFormat.h @@ -58,7 +58,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0; /// describe what change you made. The content of this comment isn't important; /// it just ensures a conflict if two people change the module format. /// Don't worry about adhering to the 80-column limit for this line. -const uint16_t SWIFTMODULE_VERSION_MINOR = 757; // expanded macro definitions +const uint16_t SWIFTMODULE_VERSION_MINOR = 758; // drop_deinit instruction /// A standard hash seed used for all string hashes in a serialized module. /// diff --git a/lib/Serialization/SerializeSIL.cpp b/lib/Serialization/SerializeSIL.cpp index 116a97ff413f5..f72ce4e6a2b65 100644 --- a/lib/Serialization/SerializeSIL.cpp +++ b/lib/Serialization/SerializeSIL.cpp @@ -1482,6 +1482,7 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) { case SILInstructionKind::CopyValueInst: case SILInstructionKind::ExplicitCopyValueInst: case SILInstructionKind::MoveValueInst: + case SILInstructionKind::DropDeinitInst: case SILInstructionKind::MarkUnresolvedReferenceBindingInst: case SILInstructionKind::MoveOnlyWrapperToCopyableValueInst: case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst: diff --git a/test/SIL/Parser/basic.sil b/test/SIL/Parser/basic.sil index 6cacc004693b0..de8f8621a2f7b 100644 --- a/test/SIL/Parser/basic.sil +++ b/test/SIL/Parser/basic.sil @@ -48,6 +48,12 @@ class Class2 { init() } +@_moveOnly struct MoveOnlyStruct { + @_hasStorage var i: Int + deinit +} + + sil @type_ref1 : $(Class1, Int) -> () // CHECK-LABEL: sil @type_ref1 : $@convention(thin) (Class1, Int) // Instructions @@ -1659,6 +1665,16 @@ bb0(%0 : @guaranteed $Builtin.NativeObject): return undef : $() } +// CHECK-LABEL: sil [ossa] @test_drop_deinit : +sil [ossa] @test_drop_deinit : $@convention(thin) (@owned MoveOnlyStruct) -> () { +bb0(%0 : @owned $MoveOnlyStruct): + // CHECK: drop_deinit %0 : $MoveOnlyStruct + %1 = drop_deinit %0 : $MoveOnlyStruct + destroy_value %1 : $MoveOnlyStruct + %3 = tuple () + return %3 : $() +} + sil @test_destructure_struct_tuple : $@convention(thin) (@owned (Builtin.NativeObject, Builtin.Int32), @owned TestArray2) -> @owned (Builtin.NativeObject, Builtin.Int32, TestArrayStorage, Int32, TestArrayStorage) { bb0(%0 : $(Builtin.NativeObject, Builtin.Int32), %1 : $TestArray2): (%2, %3) = destructure_tuple %0 : $(Builtin.NativeObject, Builtin.Int32) diff --git a/test/SIL/Serialization/basic.sil b/test/SIL/Serialization/basic.sil index 7bcf2de049068..46c16a21d9136 100644 --- a/test/SIL/Serialization/basic.sil +++ b/test/SIL/Serialization/basic.sil @@ -23,6 +23,11 @@ struct Int32 { struct EmptyStruct {} +@_moveOnly struct MoveOnlyStruct { + @_hasStorage var i: Int32 + deinit +} + // CHECK-LABEL: sil @async_test : $@convention(thin) @async sil @async_test : $@async () -> () { bb0: @@ -44,6 +49,17 @@ bb0(%0 : @owned $(Builtin.NativeObject, Builtin.Int32), %1 : @owned $TestArray2) return %7 : $(Builtin.NativeObject, Builtin.Int32, TestArrayStorage, Int32, TestArrayStorage) } +// CHECK-LABEL: sil [ossa] @test_drop_deinit : +// CHECK: %1 = drop_deinit %0 : $MoveOnlyStruct +// CHECK-LABEL: } // end sil function 'test_drop_deinit' +sil [ossa] @test_drop_deinit : $@convention(thin) (@owned MoveOnlyStruct) -> () { +bb0(%0 : @owned $MoveOnlyStruct): + %1 = drop_deinit %0 : $MoveOnlyStruct + destroy_value %1 : $MoveOnlyStruct + %3 = tuple () + return %3 : $() +} + sil @test_empty_destructure : $@convention(thin) () -> () { bb0: %0 = struct $EmptyStruct() From c4ae7fe9e667ee4eb2bed8574ab686daaaf3916d Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Thu, 6 Apr 2023 19:27:13 +0200 Subject: [PATCH 2/5] move-only: support deinits which get the self argument as indirect argument This happens for address-only move-only types. --- lib/SIL/IR/SILFunctionType.cpp | 2 +- lib/SIL/Verifier/SILVerifier.cpp | 1 + lib/SILGen/SILGenProlog.cpp | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp index 82bd77375ea3b..ca9603ee27440 100644 --- a/lib/SIL/IR/SILFunctionType.cpp +++ b/lib/SIL/IR/SILFunctionType.cpp @@ -2385,7 +2385,7 @@ struct DeallocatorConventions : Conventions { ParameterConvention getIndirectSelfParameter(const AbstractionPattern &type) const override { - llvm_unreachable("Deallocators do not have indirect self parameters"); + return ParameterConvention::Indirect_In; } static bool classof(const Conventions *C) { diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp index 74cd73d68aae5..ff3b25ac33a66 100644 --- a/lib/SIL/Verifier/SILVerifier.cpp +++ b/lib/SIL/Verifier/SILVerifier.cpp @@ -667,6 +667,7 @@ struct ImmutableAddressUseVerifier { case SILInstructionKind::IndexAddrInst: case SILInstructionKind::TailAddrInst: case SILInstructionKind::IndexRawPointerInst: + case SILInstructionKind::MarkMustCheckInst: // Add these to our worklist. for (auto result : inst->getResults()) { llvm::copy(result->getUses(), std::back_inserter(worklist)); diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp index 0cc7854adcb2e..20623c662118e 100644 --- a/lib/SILGen/SILGenProlog.cpp +++ b/lib/SILGen/SILGenProlog.cpp @@ -40,7 +40,7 @@ static void diagnose(ASTContext &Context, SourceLoc loc, Diag diag, SILValue SILGenFunction::emitSelfDeclForDestructor(VarDecl *selfDecl) { // Emit the implicit 'self' argument. - SILType selfType = getLoweredLoadableType(selfDecl->getType()); + SILType selfType = getLoweredType(selfDecl->getType()); SILValue selfValue = F.begin()->createFunctionArgument(selfType, selfDecl); // If we have a move only type, then mark it with mark_must_check so we can't From dfa46c61291470ec43ddd8da209ce3b1c3451417 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Thu, 6 Apr 2023 20:12:13 +0200 Subject: [PATCH 3/5] SILGen: emit drop_deinit in move-only destructors --- lib/SILGen/SILGenDestructor.cpp | 1 + test/SILGen/forget.swift | 12 ++++++++---- test/SILGen/moveonly_deinits.swift | 12 ++++++++---- test/SILGen/non_loadable_move_only.swift | 18 ++++++++++++++++++ 4 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 test/SILGen/non_loadable_move_only.swift diff --git a/lib/SILGen/SILGenDestructor.cpp b/lib/SILGen/SILGenDestructor.cpp index 61735e36d7741..68f297f919a90 100644 --- a/lib/SILGen/SILGenDestructor.cpp +++ b/lib/SILGen/SILGenDestructor.cpp @@ -490,6 +490,7 @@ void SILGenFunction::emitMoveOnlyMemberDestruction(SILValue selfValue, NominalTypeDecl *nom, CleanupLocation cleanupLoc, SILBasicBlock *finishBB) { + selfValue = B.createDropDeinit(cleanupLoc, selfValue); if (selfValue->getType().isAddress()) { if (auto *structDecl = dyn_cast(nom)) { for (VarDecl *vd : nom->getStoredProperties()) { diff --git a/test/SILGen/forget.swift b/test/SILGen/forget.swift index d4d209bcb943d..744a237c57bc4 100644 --- a/test/SILGen/forget.swift +++ b/test/SILGen/forget.swift @@ -22,7 +22,8 @@ func invokedDeinit() {} // CHECK: store {{.*}} to [init] // CHECK: [[SELF_MMC:%.*]] = mark_must_check [no_consume_or_assign] [[SELF_REF]] : $*MaybeFile // CHECK: [[SELF_VAL:%.*]] = load [copy] [[SELF_MMC]] : $*MaybeFile - // CHECK: switch_enum [[SELF_VAL]] : $MaybeFile, case #MaybeFile.some!enumelt: bb1, case #MaybeFile.none!enumelt: bb2 + // CHECK: [[DD:%.*]] = drop_deinit [[SELF_VAL]] : $MaybeFile + // CHECK: switch_enum [[DD]] : $MaybeFile, case #MaybeFile.some!enumelt: bb1, case #MaybeFile.none!enumelt: bb2 // // CHECK: bb1([[FILE:%.*]] : @owned $File): // CHECK: destroy_value [[FILE]] : $File @@ -50,7 +51,8 @@ func invokedDeinit() {} // CHECK: load_borrow {{.*}} : $*File // CHECK: [[SELF_MMC:%.*]] = mark_must_check [no_consume_or_assign] [[SELF_REF]] : $*File // CHECK: [[SELF_VAL:%.*]] = load [copy] [[SELF_MMC]] : $*File - // CHECK: end_lifetime [[SELF_VAL]] : $File + // CHECK: [[DD:%.*]] = drop_deinit [[SELF_VAL]] : $File + // CHECK: end_lifetime [[DD]] : $File deinit { invokedDeinit() @@ -90,7 +92,8 @@ func invokedDeinit() {} // CHECK: [[MMC:%.*]] = mark_must_check [no_consume_or_assign] [[ACCESS]] : $*PointerTree // CHECK: [[COPIED_SELF:%.*]] = load [copy] [[MMC]] : $*PointerTree // CHECK: end_access [[ACCESS]] : $*PointerTree -// CHECK: ([[LEFT:%.*]], [[FILE:%.*]], {{%.*}}, [[RIGHT:%.*]]) = destructure_struct [[COPIED_SELF]] : $PointerTree +// CHECK: [[DD:%.*]] = drop_deinit [[COPIED_SELF]] +// CHECK: ([[LEFT:%.*]], [[FILE:%.*]], {{%.*}}, [[RIGHT:%.*]]) = destructure_struct [[DD]] : $PointerTree // CHECK: destroy_value [[LEFT]] : $Ptr // CHECK: destroy_value [[FILE]] : $File // CHECK: destroy_value [[RIGHT]] : $Ptr @@ -167,7 +170,8 @@ final class Wallet { // CHECK: [[SELF_MMC:%.*]] = mark_must_check [no_consume_or_assign] [[SELF_ACCESS]] // CHECK: [[SELF_COPY:%.*]] = load [copy] [[SELF_MMC]] : $*Ticket // CHECK: end_access [[SELF_ACCESS:%.*]] : $*Ticket - // CHECK: switch_enum [[SELF_COPY]] : $Ticket, case #Ticket.empty!enumelt: bb4, case #Ticket.within!enumelt: bb5 + // CHECK: [[DD:%.*]] = drop_deinit [[SELF_COPY]] : $Ticket + // CHECK: switch_enum [[DD]] : $Ticket, case #Ticket.empty!enumelt: bb4, case #Ticket.within!enumelt: bb5 // CHECK: bb4: // CHECK: br bb6 // CHECK: bb5([[PREV_SELF_WALLET:%.*]] : @owned $Wallet): diff --git a/test/SILGen/moveonly_deinits.swift b/test/SILGen/moveonly_deinits.swift index e22462e2e01b5..a483078e3681c 100644 --- a/test/SILGen/moveonly_deinits.swift +++ b/test/SILGen/moveonly_deinits.swift @@ -66,7 +66,8 @@ var value: Bool { false } // SILGEN-LABEL: sil hidden [ossa] @$s16moveonly_deinits19KlassPairWithDeinitVfD : $@convention(method) (@owned KlassPairWithDeinit) -> () { // SILGEN: bb0([[ARG:%.*]] : // SILGEN: [[MARK:%.*]] = mark_must_check [consumable_and_assignable] [[ARG]] -// SILGEN: ([[LHS:%.*]], [[RHS:%.*]]) = destructure_struct [[MARK]] +// SILGEN: [[DD:%.*]] = drop_deinit [[MARK]] +// SILGEN: ([[LHS:%.*]], [[RHS:%.*]]) = destructure_struct [[DD]] // SILGEN: destroy_value [[LHS]] // SILGEN: destroy_value [[RHS]] // SILGEN: } // end sil function '$s16moveonly_deinits19KlassPairWithDeinitVfD' @@ -74,7 +75,8 @@ var value: Bool { false } // SILGEN-LABEL: sil hidden [ossa] @$s16moveonly_deinits17IntPairWithDeinitVfD : $@convention(method) (@owned IntPairWithDeinit) -> () { // SILGEN: bb0([[ARG:%.*]] : // SILGEN: [[MARKED:%.*]] = mark_must_check [consumable_and_assignable] [[ARG]] -// SILGEN: end_lifetime [[MARKED]] +// SILGEN: [[DD:%.*]] = drop_deinit [[MARKED]] +// SILGEN: end_lifetime [[DD]] // SILGEN: } // end sil function '$s16moveonly_deinits17IntPairWithDeinitVfD' //////////////////////// @@ -330,7 +332,8 @@ func consumeKlassEnumPairWithDeinit(_ x: __owned KlassEnumPairWithDeinit) { } // SILGEN-LABEL: sil hidden [ossa] @$s16moveonly_deinits23KlassEnumPairWithDeinitOfD : $@convention(method) (@owned KlassEnumPairWithDeinit) -> () { // SILGEN: bb0([[ARG:%.*]] : // SILGEN: [[MARK:%.*]] = mark_must_check [consumable_and_assignable] [[ARG]] -// SILGEN: switch_enum [[MARK]] : $KlassEnumPairWithDeinit, case #KlassEnumPairWithDeinit.lhs!enumelt: [[BB_LHS:bb[0-9]+]], case #KlassEnumPairWithDeinit.rhs!enumelt: [[BB_RHS:bb[0-9]+]] +// SILGEN: [[DD:%.*]] = drop_deinit [[MARK]] +// SILGEN: switch_enum [[DD]] : $KlassEnumPairWithDeinit, case #KlassEnumPairWithDeinit.lhs!enumelt: [[BB_LHS:bb[0-9]+]], case #KlassEnumPairWithDeinit.rhs!enumelt: [[BB_RHS:bb[0-9]+]] // // SILGEN: [[BB_LHS]]([[ARG:%.*]] : // SILGEN-NEXT: destroy_value [[ARG]] @@ -348,7 +351,8 @@ func consumeKlassEnumPairWithDeinit(_ x: __owned KlassEnumPairWithDeinit) { } // SILGEN-LABEL: sil hidden [ossa] @$s16moveonly_deinits21IntEnumPairWithDeinitOfD : $@convention(method) (@owned IntEnumPairWithDeinit) -> () { // SILGEN: bb0([[ARG:%.*]] : // SILGEN: [[MARK:%.*]] = mark_must_check [consumable_and_assignable] [[ARG]] -// SILGEN: switch_enum [[MARK]] : $IntEnumPairWithDeinit, case #IntEnumPairWithDeinit.lhs!enumelt: [[BB_LHS:bb[0-9]+]], case #IntEnumPairWithDeinit.rhs!enumelt: [[BB_RHS:bb[0-9]+]] +// SILGEN: [[DD:%.*]] = drop_deinit [[MARK]] +// SILGEN: switch_enum [[DD]] : $IntEnumPairWithDeinit, case #IntEnumPairWithDeinit.lhs!enumelt: [[BB_LHS:bb[0-9]+]], case #IntEnumPairWithDeinit.rhs!enumelt: [[BB_RHS:bb[0-9]+]] // // SILGEN: [[BB_LHS]]([[ARG:%.*]] : // SILGEN-NEXT: br [[BB_CONT:bb[0-9]+]] diff --git a/test/SILGen/non_loadable_move_only.swift b/test/SILGen/non_loadable_move_only.swift new file mode 100644 index 0000000000000..706b836cae084 --- /dev/null +++ b/test/SILGen/non_loadable_move_only.swift @@ -0,0 +1,18 @@ +// RUN: %target-swift-emit-silgen -module-name=test -primary-file %s | %FileCheck %s + +@_moveOnly +public struct GenericMoveOnly { + var i: Int + var s: T + + // CHECK-LABEL: sil [ossa] @$s4test15GenericMoveOnlyVfD : $@convention(method) (@in GenericMoveOnly) -> () + // CHECK: [[DD:%.*]] = drop_deinit %0 : $*GenericMoveOnly + // CHECK: [[SE:%.*]] = struct_element_addr %2 : $*GenericMoveOnly, #GenericMoveOnly.s + // CHECK: [[A:%.*]] = begin_access [deinit] [static] %3 : $*T + // CHECK: destroy_addr [[A]] : $*T + // CHECK: } // end sil function '$s4test15GenericMoveOnlyVfD' + deinit { + } +} + + From 33694e041fbbbc6151f27bcf8c957b77e0bf6550 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Thu, 6 Apr 2023 20:13:37 +0200 Subject: [PATCH 4/5] MoveOnlyDeinitInsertion: don't call the deinit if the destroy is preceeded by a drop_deinit rdar://105798769 --- .../Mandatory/MoveOnlyDeinitInsertion.cpp | 6 ++- .../moveonly_deinit_insertion.sil | 42 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyDeinitInsertion.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyDeinitInsertion.cpp index 62bb4057cf075..50518ac489c8b 100644 --- a/lib/SILOptimizer/Mandatory/MoveOnlyDeinitInsertion.cpp +++ b/lib/SILOptimizer/Mandatory/MoveOnlyDeinitInsertion.cpp @@ -62,7 +62,8 @@ static bool performTransform(SILFunction &fn) { if (auto *dvi = dyn_cast(inst)) { auto destroyType = dvi->getOperand()->getType(); - if (destroyType.isMoveOnlyNominalType()) { + if (destroyType.isMoveOnlyNominalType() && + !isa(lookThroughOwnershipInsts(dvi->getOperand()))) { LLVM_DEBUG(llvm::dbgs() << "Handling: " << *dvi); auto *nom = destroyType.getNominalOrBoundGenericNominal(); assert(nom); @@ -88,7 +89,8 @@ static bool performTransform(SILFunction &fn) { if (auto *dai = dyn_cast(inst)) { auto destroyType = dai->getOperand()->getType(); - if (destroyType.isLoadable(fn) && destroyType.isMoveOnlyNominalType()) { + if (destroyType.isLoadable(fn) && destroyType.isMoveOnlyNominalType() && + !isa(dai->getOperand())) { LLVM_DEBUG(llvm::dbgs() << "Handling: " << *dai); auto *nom = destroyType.getNominalOrBoundGenericNominal(); assert(nom); diff --git a/test/SILOptimizer/moveonly_deinit_insertion.sil b/test/SILOptimizer/moveonly_deinit_insertion.sil index 040071c35c0aa..4e148eb4911f9 100644 --- a/test/SILOptimizer/moveonly_deinit_insertion.sil +++ b/test/SILOptimizer/moveonly_deinit_insertion.sil @@ -309,6 +309,48 @@ bb6: return %14 : $() } +//===----------------------------------------------------------------------===// +// drop_deinit Tests +//===----------------------------------------------------------------------===// + +// CHECK-LABEL: sil [ossa] @dropDeinitOnStruct : $@convention(thin) (@owned TrivialStruct) -> () { +// CHECK: %1 = drop_deinit %0 +// CHECK-NEXT: destroy_value %1 +// CHECK: } // end sil function 'dropDeinitOnStruct' +sil [ossa] @dropDeinitOnStruct : $@convention(thin) (@owned TrivialStruct) -> () { +bb0(%0 : @owned $TrivialStruct): + %1 = drop_deinit %0 : $TrivialStruct + destroy_value %1 : $TrivialStruct + %9999 = tuple() + return %9999 : $() +} + +// CHECK-LABEL: sil [ossa] @dropDeinitOnMovedStruct : $@convention(thin) (@owned TrivialStruct) -> () { +// CHECK: %1 = drop_deinit %0 +// CHECK-NEXT: %2 = move_value %1 +// CHECK-NEXT: destroy_value %2 +// CHECK: } // end sil function 'dropDeinitOnMovedStruct' +sil [ossa] @dropDeinitOnMovedStruct : $@convention(thin) (@owned TrivialStruct) -> () { +bb0(%0 : @owned $TrivialStruct): + %1 = drop_deinit %0 : $TrivialStruct + %2 = move_value %1 : $TrivialStruct + destroy_value %2 : $TrivialStruct + %9999 = tuple() + return %9999 : $() +} + +// CHECK-LABEL: sil [ossa] @dropDeinitOnIndirectStruct : $@convention(thin) (@in TrivialStruct) -> () { +// CHECK: %1 = drop_deinit %0 +// CHECK-NEXT: destroy_addr %1 +// CHECK: } // end sil function 'dropDeinitOnIndirectStruct' +sil [ossa] @dropDeinitOnIndirectStruct : $@convention(thin) (@in TrivialStruct) -> () { +bb0(%0 : $*TrivialStruct): + %1 = drop_deinit %0 : $*TrivialStruct + destroy_addr %1 : $*TrivialStruct + %9999 = tuple() + return %9999 : $() +} + sil @$s4main5KlassCfD : $@convention(method) (@owned Klass) -> () sil @$s4main5KlassCACycfc : $@convention(method) (@owned Klass) -> @owned Klass sil @$s4main5KlassCfd : $@convention(method) (@guaranteed Klass) -> @owned Builtin.NativeObject From 6b44d5a4d2210b80bf89563e94dbb52491020a38 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Fri, 7 Apr 2023 10:04:56 +0200 Subject: [PATCH 5/5] tests: re-enable simplification passes in SILGen/moveonly_deinits.swift --- test/SILGen/moveonly_deinits.swift | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/SILGen/moveonly_deinits.swift b/test/SILGen/moveonly_deinits.swift index a483078e3681c..40a9f1749b6ce 100644 --- a/test/SILGen/moveonly_deinits.swift +++ b/test/SILGen/moveonly_deinits.swift @@ -1,6 +1,5 @@ -// TODO: re-enable the simplification passes once rdar://104875010 is fixed -// RUN: %target-swift-emit-silgen -enable-experimental-feature MoveOnlyEnumDeinits -Xllvm -sil-disable-pass=simplification %s | %FileCheck -check-prefix=SILGEN %s -// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyEnumDeinits -Xllvm -sil-disable-pass=simplification %s | %FileCheck -check-prefix=SIL %s +// RUN: %target-swift-emit-silgen -enable-experimental-feature MoveOnlyEnumDeinits %s | %FileCheck -check-prefix=SILGEN %s +// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyEnumDeinits %s | %FileCheck -check-prefix=SIL %s // Test that makes sure that throughout the pipeline we properly handle // conditional releases for trivial and non-trivial move only types.