Skip to content

Commit c7b91a9

Browse files
aschwaighoferaschwaighofer
committed
Address review comments
1 parent ad6f6de commit c7b91a9

File tree

4 files changed

+50
-46
lines changed

4 files changed

+50
-46
lines changed

‎lib/IRGen/GenEnum.cpp

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -659,8 +659,7 @@ namespace {
659659
void consume(IRGenFunction &IGF, Explosion &src,
660660
Atomicity atomicity,
661661
SILType T) const override {
662-
if (ElementsAreABIAccessible &&
663-
tryEmitConsumeUsingDeinit(IGF, src, T)) {
662+
if (tryEmitConsumeUsingDeinit(IGF, src, T)) {
664663
return;
665664
}
666665

@@ -682,8 +681,7 @@ namespace {
682681

683682
void destroy(IRGenFunction &IGF, Address addr, SILType T,
684683
bool isOutlined) const override {
685-
if (ElementsAreABIAccessible &&
686-
tryEmitDestroyUsingDeinit(IGF, addr, T)) {
684+
if (tryEmitDestroyUsingDeinit(IGF, addr, T)) {
687685
return;
688686
}
689687

@@ -2858,8 +2856,7 @@ namespace {
28582856

28592857
void consume(IRGenFunction &IGF, Explosion &src,
28602858
Atomicity atomicity, SILType T) const override {
2861-
if (ElementsAreABIAccessible &&
2862-
tryEmitConsumeUsingDeinit(IGF, src, T)) {
2859+
if (tryEmitConsumeUsingDeinit(IGF, src, T)) {
28632860
return;
28642861
}
28652862

@@ -2987,8 +2984,7 @@ namespace {
29872984

29882985
void destroy(IRGenFunction &IGF, Address addr, SILType T,
29892986
bool isOutlined) const override {
2990-
if (ElementsAreABIAccessible &&
2991-
tryEmitDestroyUsingDeinit(IGF, addr, T)) {
2987+
if (tryEmitDestroyUsingDeinit(IGF, addr, T)) {
29922988
return;
29932989
}
29942990

@@ -4899,8 +4895,7 @@ namespace {
48994895

49004896
void consume(IRGenFunction &IGF, Explosion &src,
49014897
Atomicity atomicity, SILType T) const override {
4902-
if (ElementsAreABIAccessible &&
4903-
tryEmitConsumeUsingDeinit(IGF, src, T)) {
4898+
if (tryEmitConsumeUsingDeinit(IGF, src, T)) {
49044899
return;
49054900
}
49064901

@@ -5259,8 +5254,7 @@ namespace {
52595254

52605255
void destroy(IRGenFunction &IGF, Address addr, SILType T,
52615256
bool isOutlined) const override {
5262-
if (ElementsAreABIAccessible &&
5263-
tryEmitDestroyUsingDeinit(IGF, addr, T)) {
5257+
if (tryEmitDestroyUsingDeinit(IGF, addr, T)) {
52645258
return;
52655259
}
52665260

@@ -6919,11 +6913,8 @@ SingletonEnumImplStrategy::completeEnumTypeLayout(TypeConverter &TC,
69196913
auto alignment = fixedEltTI.getFixedAlignment();
69206914
applyLayoutAttributes(TC.IGM, theEnum, /*fixed*/true, alignment);
69216915

6922-
IsABIAccessible_t isABIAccessible = IsABIAccessible;
6923-
if (Type.getASTType()->isNoncopyable() &&
6924-
!IGM.getSILModule().isTypeMetadataAccessible(Type.getASTType()))
6925-
isABIAccessible = IsNotABIAccessible;
6926-
6916+
auto isABIAccessible = isTypeABIAccessibleIfFixedSize(TC.IGM,
6917+
Type.getASTType());
69276918
return getFixedEnumTypeInfo(enumTy,
69286919
fixedEltTI.getFixedSize(),
69296920
fixedEltTI.getSpareBits(),
@@ -7086,11 +7077,8 @@ TypeInfo *SinglePayloadEnumImplStrategy::completeFixedLayout(
70867077
auto copyable = !theEnum->canBeCopyable()
70877078
? IsNotCopyable : IsCopyable;
70887079

7089-
IsABIAccessible_t isABIAccessible = IsABIAccessible;
7090-
if (Type.getASTType()->isNoncopyable() &&
7091-
!IGM.getSILModule().isTypeMetadataAccessible(Type.getASTType()))
7092-
isABIAccessible = IsNotABIAccessible;
7093-
7080+
auto isABIAccessible = isTypeABIAccessibleIfFixedSize(TC.IGM,
7081+
Type.getASTType());
70947082
getFixedEnumTypeInfo(
70957083
enumTy, Size(sizeWithTag), spareBits.build(), alignment,
70967084
deinit & payloadTI.isTriviallyDestroyable(ResilienceExpansion::Maximal),
@@ -7303,11 +7291,8 @@ MultiPayloadEnumImplStrategy::completeFixedLayout(TypeConverter &TC,
73037291

73047292
applyLayoutAttributes(TC.IGM, theEnum, /*fixed*/ true, worstAlignment);
73057293

7306-
IsABIAccessible_t isABIAccessible = IsABIAccessible;
7307-
if (Type.getASTType()->isNoncopyable() &&
7308-
!IGM.getSILModule().isTypeMetadataAccessible(Type.getASTType()))
7309-
isABIAccessible = IsNotABIAccessible;
7310-
7294+
auto isABIAccessible = isTypeABIAccessibleIfFixedSize(TC.IGM,
7295+
Type.getASTType());
73117296
getFixedEnumTypeInfo(enumTy, Size(sizeWithTag), std::move(spareBits),
73127297
worstAlignment, isTriviallyDestroyable, isBT,
73137298
isCopyable, isABIAccessible);

‎lib/IRGen/GenStruct.cpp

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -277,14 +277,14 @@ namespace {
277277
void destroy(IRGenFunction &IGF, Address address, SILType T,
278278
bool isOutlined) const override {
279279

280-
if (!asImpl().areFieldsABIAccessible()) {
281-
emitDestroyCall(IGF, T, address);
282-
return;
283-
}
284-
285280
// If the struct has a deinit declared, then call it to destroy the
286281
// value.
287282
if (!tryEmitDestroyUsingDeinit(IGF, address, T)) {
283+
if (!asImpl().areFieldsABIAccessible()) {
284+
emitDestroyCall(IGF, T, address);
285+
return;
286+
}
287+
288288
// Otherwise, perform elementwise destruction of the value.
289289
super::destroy(IGF, address, T, isOutlined);
290290
}
@@ -953,19 +953,19 @@ namespace {
953953

954954
void consume(IRGenFunction &IGF, Explosion &explosion,
955955
Atomicity atomicity, SILType T) const override {
956+
// If the struct has a deinit declared, then call it to consume the
957+
// value.
958+
if (tryEmitConsumeUsingDeinit(IGF, explosion, T)) {
959+
return;
960+
}
961+
956962
if (!areFieldsABIAccessible()) {
957963
auto temporary = allocateStack(IGF, T, "deinit.arg").getAddress();
958964
initialize(IGF, explosion, temporary, /*outlined*/false);
959965
emitDestroyCall(IGF, T, temporary);
960966
return;
961967
}
962968

963-
// If the struct has a deinit declared, then call it to consume the
964-
// value.
965-
if (tryEmitConsumeUsingDeinit(IGF, explosion, T)) {
966-
return;
967-
}
968-
969969
// Otherwise, do elementwise destruction of the value.
970970
return super::consume(IGF, explosion, atomicity, T);
971971
}
@@ -1267,10 +1267,7 @@ namespace {
12671267
FieldsAreABIAccessible_t areFieldsABIAccessible,
12681268
StructLayout &&layout,
12691269
unsigned explosionSize) {
1270-
IsABIAccessible_t isABIAccessible = IsABIAccessible;
1271-
if (TheStruct->isNoncopyable() &&
1272-
!IGM.getSILModule().isTypeMetadataAccessible(TheStruct))
1273-
isABIAccessible = IsNotABIAccessible;
1270+
auto isABIAccessible = isTypeABIAccessibleIfFixedSize(IGM, TheStruct);
12741271
return LoadableStructTypeInfo::create(fields,
12751272
areFieldsABIAccessible,
12761273
explosionSize,
@@ -1287,10 +1284,7 @@ namespace {
12871284
FixedStructTypeInfo *createFixed(ArrayRef<StructFieldInfo> fields,
12881285
FieldsAreABIAccessible_t areFieldsABIAccessible,
12891286
StructLayout &&layout) {
1290-
IsABIAccessible_t isABIAccessible = IsABIAccessible;
1291-
if (TheStruct->isNoncopyable() &&
1292-
!IGM.getSILModule().isTypeMetadataAccessible(TheStruct))
1293-
isABIAccessible = IsNotABIAccessible;
1287+
auto isABIAccessible = isTypeABIAccessibleIfFixedSize(IGM, TheStruct);
12941288
return FixedStructTypeInfo::create(fields, areFieldsABIAccessible,
12951289
layout.getType(),
12961290
layout.getSize(),

‎lib/IRGen/GenType.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3040,3 +3040,22 @@ bool irgen::tryEmitDestroyUsingDeinit(IRGenFunction &IGF, Address address,
30403040
// Indirect parameter teardown
30413041
[&]{ /* nothing to do */ });
30423042
}
3043+
3044+
IsABIAccessible_t irgen::isTypeABIAccessibleIfFixedSize(IRGenModule &IGM,
3045+
CanType ty) {
3046+
3047+
// Copyable types currently are always fixed size.
3048+
if (!ty->isNoncopyable())
3049+
return IsABIAccessible;
3050+
3051+
// Check for a deinit. If this type does not define a deinit it is ABI
3052+
// accessible because we can just project onto its sub elements.
3053+
auto nom = ty->getAnyNominal();
3054+
if (!nom || !nom->getValueTypeDestructor())
3055+
return IsABIAccessible;
3056+
3057+
if (IGM.getSILModule().isTypeMetadataAccessible(ty))
3058+
return IsABIAccessible;
3059+
3060+
return IsNotABIAccessible;
3061+
}

‎lib/IRGen/GenType.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,12 @@ bool tryEmitConsumeUsingDeinit(IRGenFunction &IGF,
407407
Explosion &explosion,
408408
SILType T);
409409

410+
/// Most fixed size types currently are always ABI accessible (value operations
411+
/// can be done without metadata). One notable exception is non-copyable types
412+
/// with a deinit. Their type metadata is required to call destroy if the deinit
413+
/// function is not available to the current SIL module.
414+
IsABIAccessible_t isTypeABIAccessibleIfFixedSize(IRGenModule &IGM, CanType ty);
415+
410416
} // end namespace irgen
411417
} // end namespace swift
412418

0 commit comments

Comments
 (0)