Skip to content

Commit 8579c19

Browse files
gottesmmgottesmm
committed
[move-only] Make sure that we serialize deinits and if we are not able to see the value witness function, call it via the value witness function.
Some notes: 1. I put in both a swiftpm like test case and a library evolution test case. I also updated the moveonly_deinit serialization swift test to show that we actually serialize the deinit. 2. I changed when we emit the deinit table to only be when we have a type with an actual value type destructor. Notably this doesn't include classes today so as a side-effect, we no longer attempt to devirtualize moveonly class deinits. This doesn't affect anything we are trying to actually do since we do not support noncopyable classes today. With that in mind, I changed one test that was showing that deinit devirtualization worked to use a struct with deinit instead of a class. rdar://109679168
1 parent 79a2ff5 commit 8579c19

14 files changed

+177
-16
lines changed

‎include/swift/SIL/SILMoveOnlyDeinit.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,12 @@ class SILMoveOnlyDeinit final : public SILAllocated<SILMoveOnlyDeinit> {
6363

6464
SILFunction *getImplementation() const { return funcImpl; }
6565

66-
bool isSerialized() const { return serialized; }
66+
IsSerialized_t isSerialized() const {
67+
return serialized ? IsSerialized : IsNotSerialized;
68+
}
69+
void setSerialized(IsSerialized_t inputSerialized) {
70+
serialized = inputSerialized ? 1 : 0;
71+
}
6772

6873
void print(llvm::raw_ostream &os, bool verbose) const;
6974
void dump() const;

‎lib/IRGen/GenType.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2890,19 +2890,25 @@ static bool tryEmitDeinitCall(IRGenFunction &IGF,
28902890
llvm::function_ref<void ()> indirectCleanup) {
28912891
auto ty = T.getASTType();
28922892
auto nominal = ty->getAnyNominal();
2893+
28932894
// We are only concerned with move-only type deinits here.
28942895
if (!nominal || !nominal->getValueTypeDestructor()) {
28952896
return false;
28962897
}
2897-
2898-
auto deinit = IGF.getSILModule().lookUpMoveOnlyDeinit(nominal);
2899-
assert(deinit && "type has a deinit declared in AST but SIL deinit record is not present!");
2900-
2898+
2899+
auto deinitTable = IGF.getSILModule().lookUpMoveOnlyDeinit(nominal);
2900+
2901+
// If we do not have a deinit table, call the value witness instead.
2902+
if (!deinitTable) {
2903+
irgen::emitDestroyCall(IGF, T, indirect());
2904+
return true;
2905+
}
2906+
29012907
// The deinit should take a single value parameter of the nominal type, either
29022908
// by @owned or indirect @in convention.
2903-
auto deinitFn = IGF.IGM.getAddrOfSILFunction(deinit->getImplementation(),
2909+
auto deinitFn = IGF.IGM.getAddrOfSILFunction(deinitTable->getImplementation(),
29042910
NotForDefinition);
2905-
auto deinitTy = deinit->getImplementation()->getLoweredFunctionType();
2911+
auto deinitTy = deinitTable->getImplementation()->getLoweredFunctionType();
29062912
auto deinitFP = FunctionPointer::forDirect(IGF.IGM, deinitFn,
29072913
nullptr, deinitTy);
29082914
assert(deinitTy->getNumParameters() == 1

‎lib/SILGen/SILGen.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,11 +1079,6 @@ void SILGenModule::emitFunctionDefinition(SILDeclRef constant, SILFunction *f) {
10791079
preEmitFunction(constant, f, loc);
10801080
PrettyStackTraceSILFunction X("silgen emitDeallocatingDestructor", f);
10811081
SILGenFunction(*this, *f, dd).emitDeallocatingDestructor(dd);
1082-
1083-
// If we have a move only type, create the table for this type.
1084-
if (nom->isMoveOnly())
1085-
SILMoveOnlyDeinit::create(f->getModule(), nom, IsNotSerialized, f);
1086-
10871082
postEmitFunction(constant, f);
10881083
return;
10891084
}

‎lib/SILGen/SILGen.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
459459
SILFunction *jvp, SILFunction *vjp,
460460
const DeclAttribute *diffAttr);
461461

462+
/// Emit a deinit table for a noncopyable type.
463+
void emitNonCopyableTypeDeinitTable(NominalTypeDecl *decl);
464+
462465
/// Known functions for bridging.
463466
SILDeclRef getStringToNSStringFn();
464467
SILDeclRef getNSStringToStringFn();

‎lib/SILGen/SILGenType.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,21 @@ void SILGenModule::emitDefaultWitnessTable(ProtocolDecl *protocol) {
10691069
defaultWitnesses->convertToDefinition(builder.DefaultWitnesses);
10701070
}
10711071

1072+
void SILGenModule::emitNonCopyableTypeDeinitTable(NominalTypeDecl *nom) {
1073+
auto *dd = nom->getValueTypeDestructor();
1074+
if (!dd)
1075+
return;
1076+
1077+
SILDeclRef constant(dd, SILDeclRef::Kind::Deallocator);
1078+
SILFunction *f = getFunction(constant, NotForDefinition);
1079+
auto serialized = IsSerialized_t::IsNotSerialized;
1080+
bool nomIsPublic = nom->getEffectiveAccess() >= AccessLevel::Public;
1081+
// We only serialize the deinit if the type is public and not resilient.
1082+
if (nomIsPublic && !nom->isResilient())
1083+
serialized = IsSerialized;
1084+
SILMoveOnlyDeinit::create(f->getModule(), nom, serialized, f);
1085+
}
1086+
10721087
namespace {
10731088

10741089
/// An ASTVisitor for generating SIL from method declarations
@@ -1100,6 +1115,13 @@ class SILGenType : public TypeMemberVisitor<SILGenType> {
11001115
genVTable.emitVTable();
11011116
}
11021117

1118+
// If this is a nominal type that is move only, emit a deinit table for it.
1119+
if (auto *nom = dyn_cast<NominalTypeDecl>(theType)) {
1120+
if (nom->isMoveOnly()) {
1121+
SGM.emitNonCopyableTypeDeinitTable(nom);
1122+
}
1123+
}
1124+
11031125
// Build a default witness table if this is a protocol that needs one.
11041126
if (auto protocol = dyn_cast<ProtocolDecl>(theType)) {
11051127
if (!protocol->isObjC() && protocol->isResilient()) {

‎lib/SILOptimizer/UtilityPasses/SerializeSILPass.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,10 @@ class SerializeSILPass : public SILModuleTransform {
473473
for (auto &VT : M.getVTables()) {
474474
VT->setSerialized(IsNotSerialized);
475475
}
476+
477+
for (auto &Deinit : M.getMoveOnlyDeinits()) {
478+
Deinit->setSerialized(IsNotSerialized);
479+
}
476480
}
477481

478482
public:

‎test/IRGen/Inputs/moveonly_split_module_source_input.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
2+
class Klass {
3+
var name = "John"
4+
}
5+
6+
#if TEST_LIBRARY_EVOLUTION
7+
public struct MoveOnly : ~Copyable {
8+
var k = Klass()
9+
var k2 = Klass()
10+
deinit {
11+
print("==> I am in the deinit resiliently!")
12+
print("==> My name is \(k.name)")
13+
}
14+
}
15+
#else
16+
struct MoveOnly : ~Copyable {
17+
var k = Klass()
18+
var k2 = Klass()
19+
deinit {
20+
print("==> I am in the deinit!")
21+
print("==> My name is \(k.name)")
22+
}
23+
}
24+
#endif

‎test/IRGen/moveonly_split_module_source_deinit.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swiftc_driver -emit-module -module-name server -emit-module-path %t/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift
3+
// RUN: %target-swift-frontend -module-name server -primary-file %s %S/Inputs/moveonly_split_module_source_input.swift -emit-ir -emit-module-path %t/server.swiftmodule | %FileCheck %s -check-prefix=REFERRING_MODULE
4+
// RUN: %target-swift-frontend -module-name server %s -primary-file %S/Inputs/moveonly_split_module_source_input.swift -emit-ir -emit-module-path %t/server.swiftmodule | %FileCheck %s -check-prefix=DEFINING_MODULE
5+
6+
// Make sure we call the deinit through the value witness table in the other module.
7+
8+
// REFERRING_MODULE-LABEL: define swiftcc void @"$s6serverAAV4mainyyKFZ"(%swift.refcounted* swiftself %0, %swift.error** noalias nocapture swifterror dereferenceable(8) %1) #0 {
9+
// REFERRING_MODULE: [[SERVER:%.*]] = alloca %T6server8MoveOnlyV
10+
// REFERRING_MODULE: [[VALUE_WITNESS_TABLE:%.*]] = getelementptr inbounds i8*, i8** %"$s6server8MoveOnlyVN.valueWitnesses"
11+
// REFERRING_MODULE: [[VALUE_WITNESS:%.*]] = load i8*, i8** [[VALUE_WITNESS_TABLE]]
12+
// REFERRING_MODULE: [[DESTROY:%.*]] = bitcast i8* [[VALUE_WITNESS]]
13+
// REFERRING_MODULE: [[CAST_SERVER:%.*]] = bitcast %T6server8MoveOnlyV* [[SERVER]]
14+
// REFERRING_MODULE: call void [[DESTROY]]({{%.*}} [[CAST_SERVER]], {{%.*}} @"$s6server8MoveOnlyVN")
15+
16+
// Make sure that in the other module, we do call the deinit directly from the value witness.
17+
// DEFINING_MODULE-LABEL: define internal void @"$s6server8MoveOnlyVwxx"(%swift.opaque* noalias %object, %swift.type* %MoveOnly) {{.*}} {
18+
// DEFINING_MODULE: [[SELF:%.*]] = bitcast %swift.opaque* [[ARG:%.*]] to %T6server8MoveOnlyV*
19+
// DEFINING_MODULE: [[VAR:%.*]] = getelementptr inbounds {{%.*}}, {{%.*}}* [[SELF]]
20+
// DEFINING_MODULE: [[LOADED_VAR:%.*]] = load {{%.*}}*, {{%.*}}** [[VAR]],
21+
// DEFINING_MODULE: [[VAR2:%.*]] = getelementptr inbounds {{%.*}}, {{%.*}}* [[SELF]]
22+
// DEFINING_MODULE: [[LOADED_VAR2:%.*]] = load {{%.*}}*, {{%.*}}** [[VAR2]],
23+
// DEFINING_MODULE: call swiftcc void @"$s6server8MoveOnlyVfD"({{%.*}}* [[LOADED_VAR]], {{%.*}}* [[LOADED_VAR2]])
24+
@main
25+
public struct server {
26+
public static func main() throws {
27+
let server = MoveOnly()
28+
_ = server
29+
}
30+
}

‎test/IRGen/moveonly_split_module_source_deinit_library_evolution.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift-dylib(%t/%target-library-name(MoveOnlySplit)) -enable-library-evolution %S/Inputs/moveonly_split_module_source_input.swift -emit-module -emit-module-path %t/MoveOnlySplit.swiftmodule -module-name MoveOnlySplit -DTEST_LIBRARY_EVOLUTION
3+
// RUN: %target-codesign %t/%target-library-name(MoveOnlySplit)
4+
5+
// RUN: %target-build-swift %s -lMoveOnlySplit -I %t -L %t -o %t/main %target-rpath(%t)
6+
// RUN: %target-codesign %t/main
7+
// RUN: %target-run %t/main %t/%target-library-name(MoveOnlySplit) | %FileCheck -check-prefix=CHECK-LIBRARY-EVOLUTION %s
8+
9+
// REQUIRES: executable_test
10+
11+
import MoveOnlySplit
12+
13+
func main() {
14+
let server = MoveOnly() // CHECK-LIBRARY-EVOLUTION: ==> I am in the deinit resiliently!
15+
}
16+
17+
main()

‎test/Interpreter/Inputs/moveonly_split_module_source_input.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
2+
#if TEST_LIBRARY_EVOLUTION
3+
public struct MoveOnly : ~Copyable {
4+
var name = "John"
5+
public init() {}
6+
deinit {
7+
print("==> I am in the deinit resiliently!")
8+
print("==> My name is: \(name)!")
9+
}
10+
}
11+
#else
12+
struct MoveOnly : ~Copyable {
13+
var name = "John"
14+
deinit {
15+
print("==> I am in the deinit!")
16+
print("==> My name is: \(name)!")
17+
}
18+
}
19+
#endif

‎test/Interpreter/moveonly_split_module_source_deinit.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Normal test.
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: %target-swiftc_driver -emit-module -module-name server -emit-module-path %t/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift
5+
// RUN: %target-swiftc_driver -emit-executable -module-name server -emit-module-path %t/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift -o %t/server -Xlinker -add_ast_path -Xlinker %t/server.swiftmodule -Xlinker -alias -Xlinker _server_main -Xlinker _main
6+
// RUN: %target-codesign %t/server
7+
// RUN: %target-run %t/server | %FileCheck %s
8+
9+
// REQUIRES: executable_test
10+
11+
@main
12+
public struct server {
13+
public static func main() throws {
14+
let server = MoveOnly() // CHECK: ==> I am in the deinit!
15+
}
16+
}

‎test/Interpreter/moveonly_split_module_source_deinit_library_evolution.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift-dylib(%t/%target-library-name(MoveOnlySplit)) -enable-library-evolution %S/Inputs/moveonly_split_module_source_input.swift -emit-module -emit-module-path %t/MoveOnlySplit.swiftmodule -module-name MoveOnlySplit -DTEST_LIBRARY_EVOLUTION
3+
// RUN: %target-codesign %t/%target-library-name(MoveOnlySplit)
4+
5+
// RUN: %target-build-swift %s -lMoveOnlySplit -I %t -L %t -o %t/main %target-rpath(%t)
6+
// RUN: %target-codesign %t/main
7+
// RUN: %target-run %t/main %t/%target-library-name(MoveOnlySplit) | %FileCheck -check-prefix=CHECK-LIBRARY-EVOLUTION %s
8+
9+
// REQUIRES: executable_test
10+
11+
import MoveOnlySplit
12+
13+
func main() {
14+
let server = MoveOnly() // CHECK-LIBRARY-EVOLUTION: ==> I am in the deinit resiliently!
15+
}
16+
17+
main()

‎test/SILOptimizer/moveonly_lifetime.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
// RUN: %target-swift-emit-sil -sil-verify-all -module-name moveonly_lifetime -o /dev/null -Xllvm -sil-print-canonical-module -Onone -verify -enable-experimental-feature MoveOnlyClasses %s 2>&1 | %FileCheck %s
22

3-
@_moveOnly
4-
class C {}
3+
struct C : ~Copyable {
4+
deinit {}
5+
}
56

67
@_silgen_name("getC")
78
func getC() -> C
@@ -37,7 +38,7 @@ func something()
3738
// CHECK: apply [[BORROW_C]]([[INSTANCE]])
3839
//
3940
// TODO: Once we maximize lifetimes this should be below something.
40-
// CHECK: [[DESTROY_C:%[^,]+]] = function_ref @$s17moveonly_lifetime1CCfD
41+
// CHECK: [[DESTROY_C:%[^,]+]] = function_ref @$s17moveonly_lifetime1CVfD
4142
// CHECK: [[INSTANCE:%.*]] = load [take] [[STACK]]
4243
// CHECK: apply [[DESTROY_C]]([[INSTANCE]])
4344
//

‎test/Serialization/moveonly_deinit.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
// RUN: %empty-directory(%t)
2-
// TODO: re-enable the simplification passes once rdar://104875010 is fixed
32
// RUN: %target-swift-frontend -enable-experimental-feature MoveOnlyEnumDeinits -Xllvm -sil-disable-pass=simplification -g -emit-module -module-name OtherModule %S/Inputs/moveonly_deinit.swift -emit-module-path %t/OtherModule.swiftmodule
43
// RUN: %target-swift-frontend -enable-experimental-feature MoveOnlyEnumDeinits -Xllvm -sil-disable-pass=simplification -g -I %t %s -emit-silgen
4+
// RUN: %target-sil-opt -enable-experimental-feature MoveOnlyEnumDeinits %t/OtherModule.swiftmodule | %FileCheck -check-prefix=CHECK-SERIALIZED %s
55

66
// Make sure we can deserialize deinits of both enums and structs.
77

88
import OtherModule
99

10+
// CHECK-SERIALIZED: sil_moveonlydeinit [serialized] MoveOnlyStruct {
11+
// CHECK-SERIALIZED: sil_moveonlydeinit [serialized] MoveOnlyEnum {
1012
let s = MoveOnlyStruct(desc: 5)
1113
let e = MoveOnlyEnum.lhs(5)

0 commit comments

Comments
 (0)