Skip to content

Commit c3fbe24

Browse files
gottesmmgottesmm
authored
Merge pull request #66185 from gottesmm/rdar-109904633-109679168
[move-only] A few fixes around deinits
2 parents 79a2ff5 + a52302d commit c3fbe24

17 files changed

+607
-21
lines changed

‎include/swift/SIL/SILFunctionConventions.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,11 @@ class SILFunctionConventions {
371371
return getNumIndirectSILResults();
372372
}
373373

374+
/// Returns the index of self.
375+
unsigned getSILArgIndexOfSelf() const {
376+
return getSILArgIndexOfFirstParam() + getNumParameters() - 1;
377+
}
378+
374379
/// Get the index into formal indirect results corresponding to the given SIL
375380
/// indirect result argument index.
376381
unsigned getIndirectFormalResultIndexForSILArg(unsigned argIdx) const {

‎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/Mandatory/MoveOnlyDeinitInsertion.cpp

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,21 @@ static bool performTransform(SILFunction &fn) {
7878
auto subMap =
7979
astType->getContextSubstitutionMap(nom->getModuleContext(), nom);
8080
SILBuilderWithScope builder(dvi);
81+
82+
SILValue value = dvi->getOperand();
83+
auto conv = deinitFunc->getConventionsInContext();
84+
if (conv.getSILArgumentConvention(conv.getSILArgIndexOfSelf())
85+
.isIndirectConvention()) {
86+
auto *asi =
87+
builder.createAllocStack(dvi->getLoc(), value->getType());
88+
builder.emitStoreValueOperation(dvi->getLoc(), value, asi,
89+
StoreOwnershipQualifier::Init);
90+
value = asi;
91+
}
8192
auto *funcRef = builder.createFunctionRef(dvi->getLoc(), deinitFunc);
82-
builder.createApply(dvi->getLoc(), funcRef, subMap,
83-
dvi->getOperand());
93+
builder.createApply(dvi->getLoc(), funcRef, subMap, value);
94+
if (isa<AllocStackInst>(value))
95+
builder.createDeallocStack(dvi->getLoc(), value);
8496
dvi->eraseFromParent();
8597
changed = true;
8698
continue;
@@ -105,9 +117,15 @@ static bool performTransform(SILFunction &fn) {
105117
auto *funcRef = builder.createFunctionRef(dai->getLoc(), deinitFunc);
106118
auto subMap = destroyType.getASTType()->getContextSubstitutionMap(
107119
nom->getModuleContext(), nom);
108-
auto loadedValue = builder.emitLoadValueOperation(
109-
dai->getLoc(), dai->getOperand(), LoadOwnershipQualifier::Take);
110-
builder.createApply(dai->getLoc(), funcRef, subMap, loadedValue);
120+
121+
auto conv = deinitFunc->getConventionsInContext();
122+
auto argConv =
123+
conv.getSILArgumentConvention(conv.getSILArgIndexOfSelf());
124+
SILValue value = dai->getOperand();
125+
if (!argConv.isIndirectConvention())
126+
value = builder.emitLoadValueOperation(
127+
dai->getLoc(), dai->getOperand(), LoadOwnershipQualifier::Take);
128+
builder.createApply(dai->getLoc(), funcRef, subMap, value);
111129
dai->eraseFromParent();
112130
changed = true;
113131
continue;

‎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: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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+
public init() {}
11+
deinit {
12+
print("==> I am in the deinit resiliently!")
13+
print("==> My name is \(k.name)")
14+
}
15+
}
16+
#else
17+
public struct MoveOnly : ~Copyable {
18+
var k = Klass()
19+
var k2 = Klass()
20+
public init() {}
21+
deinit {
22+
print("==> I am in the deinit!")
23+
print("==> My name is \(k.name)")
24+
}
25+
}
26+
#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) {{.*}}{
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
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()

0 commit comments

Comments
 (0)