Skip to content

Commit d05f611

Browse files
aschwaighoferaschwaighofer
authored
Merge pull request #16759 from aschwaighofer/abi_inline_implies_bitwise_takable
ABI: Only store bitwise take-able values inline
2 parents 7139152 + 93fda91 commit d05f611

File tree

14 files changed

+243
-85
lines changed

14 files changed

+243
-85
lines changed

‎include/swift/ABI/MetadataValues.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ class TargetValueWitnessFlags {
138138
}
139139

140140
/// True if the type requires out-of-line allocation of its storage.
141+
/// This can be the case because the value requires more storage or if it is
142+
/// not bitwise takable.
141143
bool isInlineStorage() const { return !(Data & IsNonInline); }
142144
constexpr TargetValueWitnessFlags withInlineStorage(bool isInline) const {
143145
return TargetValueWitnessFlags((Data & ~IsNonInline) |

‎include/swift/ABI/ValueWitness.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ BEGIN_VALUE_WITNESS_RANGE(RequiredTypeLayoutWitness,
228228
/// The ValueWitnessIsNonPOD bit is set if the type is not POD.
229229
///
230230
/// The ValueWitnessIsNonInline bit is set if the type cannot be
231-
/// represented in a fixed-size buffer.
231+
/// represented in a fixed-size buffer or if it is not bitwise takable.
232232
///
233233
/// The Enum_HasExtraInhabitants bit is set if the type's binary
234234
/// representation has "extra inhabitants" that do not form valid values of

‎include/swift/Runtime/Metadata.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -278,13 +278,15 @@ struct TargetValueBuffer {
278278
using ValueBuffer = TargetValueBuffer<InProcess>;
279279

280280
/// Can a value with the given size and alignment be allocated inline?
281-
constexpr inline bool canBeInline(size_t size, size_t alignment) {
282-
return size <= sizeof(ValueBuffer) && alignment <= alignof(ValueBuffer);
281+
constexpr inline bool canBeInline(bool isBitwiseTakable, size_t size,
282+
size_t alignment) {
283+
return isBitwiseTakable && size <= sizeof(ValueBuffer) &&
284+
alignment <= alignof(ValueBuffer);
283285
}
284286

285287
template <class T>
286-
constexpr inline bool canBeInline() {
287-
return canBeInline(sizeof(T), alignof(T));
288+
constexpr inline bool canBeInline(bool isBitwiseTakable) {
289+
return canBeInline(isBitwiseTakable, sizeof(T), alignof(T));
288290
}
289291

290292
struct ValueWitnessTable;
@@ -345,8 +347,8 @@ struct ValueWitnessTable {
345347

346348
/// Would values of a type with the given layout requirements be
347349
/// allocated inline?
348-
static bool isValueInline(size_t size, size_t alignment) {
349-
return (size <= sizeof(ValueBuffer) &&
350+
static bool isValueInline(bool isBitwiseTakable, size_t size, size_t alignment) {
351+
return (isBitwiseTakable && size <= sizeof(ValueBuffer) &&
350352
alignment <= alignof(ValueBuffer));
351353
}
352354

‎lib/IRGen/GenValueWitness.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -927,13 +927,16 @@ static void addValueWitness(IRGenModule &IGM,
927927
if (auto *fixedTI = dyn_cast<FixedTypeInfo>(&concreteTI)) {
928928
assert(packing == FixedPacking::OffsetZero ||
929929
packing == FixedPacking::Allocate);
930+
bool isInline = packing == FixedPacking::OffsetZero;
931+
bool isBitwiseTakable =
932+
fixedTI->isBitwiseTakable(ResilienceExpansion::Maximal);
933+
assert(isBitwiseTakable || !isInline);
930934
flags = flags.withAlignment(fixedTI->getFixedAlignment().getValue())
931-
.withPOD(fixedTI->isPOD(ResilienceExpansion::Maximal))
932-
.withInlineStorage(packing == FixedPacking::OffsetZero)
933-
.withExtraInhabitants(
935+
.withPOD(fixedTI->isPOD(ResilienceExpansion::Maximal))
936+
.withInlineStorage(isInline)
937+
.withExtraInhabitants(
934938
fixedTI->getFixedExtraInhabitantCount(IGM) > 0)
935-
.withBitwiseTakable(
936-
fixedTI->isBitwiseTakable(ResilienceExpansion::Maximal));
939+
.withBitwiseTakable(isBitwiseTakable);
937940
} else {
938941
flags = flags.withIncomplete(true);
939942
}
@@ -1288,6 +1291,10 @@ FixedPacking TypeInfo::getFixedPacking(IRGenModule &IGM) const {
12881291
if (!fixedTI)
12891292
return FixedPacking::Dynamic;
12901293

1294+
// By convention we only store bitwise takable values inline.
1295+
if (!fixedTI->isBitwiseTakable(ResilienceExpansion::Maximal))
1296+
return FixedPacking::Allocate;
1297+
12911298
Size bufferSize = getFixedBufferSize(IGM);
12921299
Size requiredSize = fixedTI->getFixedSize();
12931300

‎lib/IRGen/TypeInfo.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,9 @@ class TypeInfo {
452452
SILType T) const = 0;
453453

454454
/// Compute the packing of values of this type into a fixed-size buffer.
455+
/// A value might not be stored in the fixed-size buffer because it does not
456+
/// fit or because it is not bit-wise takable. Non bit-wise takable values are
457+
/// not stored inline by convention.
455458
FixedPacking getFixedPacking(IRGenModule &IGM) const;
456459

457460
/// Index into an array of objects of this type.

‎stdlib/public/runtime/Enum.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,14 @@ swift::swift_initEnumMetadataSinglePayload(EnumMetadata *self,
8989
auto vwtable = getMutableVWTableForInit(self, layoutFlags);
9090

9191
size_t align = payloadLayout->flags.getAlignment();
92+
bool isBT = payloadLayout->flags.isBitwiseTakable();
9293
TypeLayout layout;
9394
layout.size = size;
94-
layout.flags = payloadLayout->flags
95-
.withExtraInhabitants(unusedExtraInhabitants > 0)
96-
.withEnumWitnesses(true)
97-
.withInlineStorage(ValueWitnessTable::isValueInline(size, align));
95+
layout.flags =
96+
payloadLayout->flags.withExtraInhabitants(unusedExtraInhabitants > 0)
97+
.withEnumWitnesses(true)
98+
.withInlineStorage(
99+
ValueWitnessTable::isValueInline(isBT, size, align));
98100
auto rawStride = llvm::alignTo(size, align);
99101
layout.stride = rawStride == 0 ? 1 : rawStride;
100102

@@ -200,14 +202,14 @@ swift::swift_initEnumMetadataMultiPayload(EnumMetadata *enumType,
200202
TypeLayout layout;
201203
layout.size = totalSize;
202204
layout.flags = ValueWitnessFlags()
203-
.withAlignmentMask(alignMask)
204-
.withPOD(isPOD)
205-
.withBitwiseTakable(isBT)
206-
// TODO: Extra inhabitants
207-
.withExtraInhabitants(false)
208-
.withEnumWitnesses(true)
209-
.withInlineStorage(ValueWitnessTable::isValueInline(totalSize, alignMask+1))
210-
;
205+
.withAlignmentMask(alignMask)
206+
.withPOD(isPOD)
207+
.withBitwiseTakable(isBT)
208+
// TODO: Extra inhabitants
209+
.withExtraInhabitants(false)
210+
.withEnumWitnesses(true)
211+
.withInlineStorage(ValueWitnessTable::isValueInline(
212+
isBT, totalSize, alignMask + 1));
211213
auto rawStride = (totalSize + alignMask) & ~alignMask;
212214
layout.stride = rawStride == 0 ? 1 : rawStride;
213215

‎stdlib/public/runtime/Metadata.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,13 +1215,15 @@ static void performBasicLayout(TypeLayout &layout,
12151215
if (!eltLayout->flags.isPOD()) isPOD = false;
12161216
if (!eltLayout->flags.isBitwiseTakable()) isBitwiseTakable = false;
12171217
}
1218-
bool isInline = ValueWitnessTable::isValueInline(size, alignMask + 1);
1218+
bool isInline =
1219+
ValueWitnessTable::isValueInline(isBitwiseTakable, size, alignMask + 1);
12191220

12201221
layout.size = size;
1221-
layout.flags = ValueWitnessFlags().withAlignmentMask(alignMask)
1222-
.withPOD(isPOD)
1223-
.withBitwiseTakable(isBitwiseTakable)
1224-
.withInlineStorage(isInline);
1222+
layout.flags = ValueWitnessFlags()
1223+
.withAlignmentMask(alignMask)
1224+
.withPOD(isPOD)
1225+
.withBitwiseTakable(isBitwiseTakable)
1226+
.withInlineStorage(isInline);
12251227
layout.stride = std::max(size_t(1), roundUpToAlignMask(size, alignMask));
12261228
}
12271229

‎stdlib/public/runtime/MetadataImpl.h

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -620,21 +620,25 @@ enum class FixedPacking {
620620
Allocate,
621621
OffsetZero
622622
};
623-
constexpr FixedPacking getFixedPacking(size_t size, size_t alignment) {
624-
return (canBeInline(size, alignment) ? FixedPacking::OffsetZero
625-
: FixedPacking::Allocate);
623+
constexpr FixedPacking getFixedPacking(bool isBitwiseTakable, size_t size,
624+
size_t alignment) {
625+
return (canBeInline(isBitwiseTakable, size, alignment)
626+
? FixedPacking::OffsetZero
627+
: FixedPacking::Allocate);
626628
}
627629

628630
/// A CRTP base class which provides default implementations of a
629631
/// number of value witnesses.
630-
template <class Impl, size_t Size, size_t Alignment,
631-
FixedPacking Packing = getFixedPacking(Size, Alignment)>
632+
template <class Impl, bool isBitwiseTakable, size_t Size, size_t Alignment,
633+
FixedPacking Packing =
634+
getFixedPacking(isBitwiseTakable, Size, Alignment)>
632635
struct BufferValueWitnesses;
633636

634637
/// An implementation of ValueBase suitable for classes that can be
635638
/// allocated inline.
636-
template <class Impl, size_t Size, size_t Alignment>
637-
struct BufferValueWitnesses<Impl, Size, Alignment, FixedPacking::OffsetZero>
639+
template <class Impl, bool isBitwiseTakable, size_t Size, size_t Alignment>
640+
struct BufferValueWitnesses<Impl, isBitwiseTakable, Size, Alignment,
641+
FixedPacking::OffsetZero>
638642
: BufferValueWitnessesBase<Impl> {
639643
static constexpr bool isInline = true;
640644

@@ -655,8 +659,9 @@ struct BufferValueWitnesses<Impl, Size, Alignment, FixedPacking::OffsetZero>
655659

656660
/// An implementation of BufferValueWitnesses suitable for types that
657661
/// cannot be allocated inline.
658-
template <class Impl, size_t Size, size_t Alignment>
659-
struct BufferValueWitnesses<Impl, Size, Alignment, FixedPacking::Allocate>
662+
template <class Impl, bool isBitwiseTakable, size_t Size, size_t Alignment>
663+
struct BufferValueWitnesses<Impl, isBitwiseTakable, Size, Alignment,
664+
FixedPacking::Allocate>
660665
: BufferValueWitnessesBase<Impl> {
661666
static constexpr bool isInline = false;
662667

@@ -741,15 +746,16 @@ struct NonFixedBufferValueWitnesses : BufferValueWitnessesBase<Impl> {
741746

742747
/// Provides implementations for
743748
/// getEnumTagSinglePayload/storeEnumTagSinglePayload.
744-
template<class Impl, size_t Size, size_t Alignment, bool hasExtraInhabitants>
749+
template <class Impl, bool isBitwiseTakable, size_t Size, size_t Alignment,
750+
bool hasExtraInhabitants>
745751
struct FixedSizeBufferValueWitnesses;
746752

747753
/// A fixed size buffer value witness that can rely on the presents of the extra
748754
/// inhabitant functions.
749-
template <class Impl, size_t Size, size_t Alignment>
750-
struct FixedSizeBufferValueWitnesses<Impl, Size, Alignment,
755+
template <class Impl, bool isBitwiseTakable, size_t Size, size_t Alignment>
756+
struct FixedSizeBufferValueWitnesses<Impl, isBitwiseTakable, Size, Alignment,
751757
true /*hasExtraInhabitants*/>
752-
: BufferValueWitnesses<Impl, Size, Alignment> {
758+
: BufferValueWitnesses<Impl, isBitwiseTakable, Size, Alignment> {
753759

754760
static unsigned getEnumTagSinglePayload(const OpaqueValue *enumAddr,
755761
unsigned numEmptyCases,
@@ -771,10 +777,10 @@ struct FixedSizeBufferValueWitnesses<Impl, Size, Alignment,
771777

772778
/// A fixed size buffer value witness that cannot rely on the presents of the
773779
/// extra inhabitant functions.
774-
template <class Impl, size_t Size, size_t Alignment>
775-
struct FixedSizeBufferValueWitnesses<Impl, Size, Alignment,
780+
template <class Impl, bool isBitwiseTakable, size_t Size, size_t Alignment>
781+
struct FixedSizeBufferValueWitnesses<Impl, isBitwiseTakable, Size, Alignment,
776782
false /*hasExtraInhabitants*/>
777-
: BufferValueWitnesses<Impl, Size, Alignment> {
783+
: BufferValueWitnesses<Impl, isBitwiseTakable, Size, Alignment> {
778784

779785
static unsigned getEnumTagSinglePayload(const OpaqueValue *enumAddr,
780786
unsigned numEmptyCases,
@@ -801,11 +807,12 @@ static constexpr bool hasExtraInhabitants(unsigned numExtraInhabitants) {
801807
/// The box type has to provide a numExtraInhabitants member, but as
802808
/// long as it's zero, the rest is fine.
803809
template <class Box>
804-
struct ValueWitnesses : FixedSizeBufferValueWitnesses<
805-
ValueWitnesses<Box>, Box::size, Box::alignment,
806-
hasExtraInhabitants(Box::numExtraInhabitants)> {
810+
struct ValueWitnesses
811+
: FixedSizeBufferValueWitnesses<
812+
ValueWitnesses<Box>, Box::isBitwiseTakable, Box::size, Box::alignment,
813+
hasExtraInhabitants(Box::numExtraInhabitants)> {
807814
using Base = FixedSizeBufferValueWitnesses<
808-
ValueWitnesses<Box>, Box::size, Box::alignment,
815+
ValueWitnesses<Box>, Box::isBitwiseTakable, Box::size, Box::alignment,
809816
hasExtraInhabitants(Box::numExtraInhabitants)>;
810817

811818
static constexpr size_t size = Box::size;
@@ -817,7 +824,7 @@ struct ValueWitnesses : FixedSizeBufferValueWitnesses<
817824
static constexpr bool hasExtraInhabitants = (numExtraInhabitants != 0);
818825
static constexpr ValueWitnessFlags flags =
819826
ValueWitnessFlags().withAlignmentMask(alignment - 1)
820-
.withInlineStorage(Base::isInline)
827+
.withInlineStorage(Base::isInline && isBitwiseTakable)
821828
.withPOD(isPOD)
822829
.withBitwiseTakable(isBitwiseTakable)
823830
.withExtraInhabitants(hasExtraInhabitants);

‎test/IRGen/struct_resilience.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,24 @@ public func partialApplyOfResilientMethod(s: Size) {
170170
_ = s.method
171171
}
172172

173+
public func wantsAny(_ any: Any) {}
174+
175+
public func resilientAny(s : ResilientWeakRef) {
176+
wantsAny(s)
177+
}
178+
179+
// CHECK-LABEL: define{{.*}} swiftcc void @"$S17struct_resilience12resilientAny1sy0c1_A016ResilientWeakRefV_tF"(%swift.opaque* noalias nocapture)
180+
// CHECK: entry:
181+
// CHECK: [[ANY:%.*]] = alloca %Any
182+
// CHECK: [[META:%.*]] = call swiftcc %swift.metadata_response @"$S16resilient_struct16ResilientWeakRefVMa"([[INT]] 0)
183+
// CHECK: [[META2:%.*]] = extractvalue %swift.metadata_response %3, 0
184+
// CHECK: [[TYADDR:%.*]] = getelementptr inbounds %Any, %Any* %1, i32 0, i32 1
185+
// CHECK: store %swift.type* [[META2]], %swift.type** [[TYADDR]]
186+
// CHECK: call %swift.opaque* @__swift_allocate_boxed_opaque_existential_0(%Any* [[ANY]])
187+
// CHECK: call swiftcc void @"$S17struct_resilience8wantsAnyyyypF"(%Any* noalias nocapture dereferenceable({{(32|16)}}) [[ANY]])
188+
// CHECK: call void @__swift_destroy_boxed_opaque_existential_0(%Any* [[ANY]])
189+
// CHECK: ret void
190+
173191
// Public metadata accessor for our resilient struct
174192

175193
// CHECK-LABEL: define{{( dllexport)?}}{{( protected)?}} swiftcc %swift.metadata_response @"$S17struct_resilience6MySizeVMa"

‎test/IRGen/weak.sil

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ public struct A {
2828
}
2929

3030
// size 8
31-
// flags 0x1100007 == 1114119 (non-POD, non-inline, non-bitwise-takable)
31+
// flags 0x130007 == 1245191 (non-POD, non-inline, non-bitwise-takable)
3232
// stride 8
33-
// CHECK: @"$S4weak1AVWV" = {{.*}} i8* inttoptr (i64 8 to i8*), i8* inttoptr (i64 1114119 to i8*), i8* inttoptr (i64 8 to i8*)]
33+
// CHECK: @"$S4weak1AVWV" = {{.*}} i8* inttoptr (i64 8 to i8*), i8* inttoptr (i64 1245191 to i8*), i8* inttoptr (i64 8 to i8*)]
3434

3535
sil @test_weak_load_store : $@convention(thin) (@inout A, Optional<C>) -> () {
3636
bb0(%0 : $*A, %1 : $Optional<C>):
@@ -104,16 +104,19 @@ bb0(%0 : $Optional<P>):
104104

105105
// initializeBufferWithCopyOfBuffer
106106
// CHECK: define linkonce_odr hidden [[OPAQUE]]* @"$S4weak1AVwCP"([[BUFFER:\[24 x i8\]]]* noalias [[DESTBUF:%.*]], [[BUFFER]]* noalias [[SRCBUF:%.*]], [[TYPE]]*
107-
// CHECK: [[DEST:%.*]] = bitcast [[BUFFER]]* [[DESTBUF]] to [[A]]*
108-
// CHECK-NEXT: [[SRC:%.*]] = bitcast [[BUFFER]]* [[SRCBUF]] to [[A]]*
109-
// CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[A]], [[A]]* [[DEST]], i32 0, i32 0
110-
// CHECK-NEXT: [[T1:%.*]] = getelementptr inbounds [[A]], [[A]]* [[SRC]], i32 0, i32 0
111-
// CHECK-NEXT: call [[WEAK]]* @swift_weakCopyInit([[WEAK]]* returned [[T0]], [[WEAK]]* [[T1]])
112-
// CHECK-NEXT: [[T0:%.*]] = bitcast [[A]]* [[DEST]] to [[OPAQUE]]*
113-
// CHECK-NEXT: ret [[OPAQUE]]* [[T0]]
114-
115-
// TAILCALL: {{_?}}$S4weak1AVwCP:
116-
// TAILCALL: jmp {{_?}}swift_weakCopyInit
107+
// CHECK: [[DEST:%.*]] = bitcast [[BUFFER]]* [[DESTBUF]] to %swift.refcounted**
108+
// CHECK: [[SRC:%.*]] = bitcast [[BUFFER]]* [[SRCBUF]] to %swift.refcounted**
109+
// CHECK: [[REF:%.*]] = load %swift.refcounted*, %swift.refcounted** [[SRC]]
110+
// CHECK: call %swift.refcounted* @swift_retain(%swift.refcounted* returned [[REF]])
111+
// CHECK: store %swift.refcounted* [[REF]], %swift.refcounted** [[DEST]]
112+
// CHECK: [[DEST:%.*]] = bitcast [[BUFFER]]* [[DESTBUF]] to %swift.refcounted**
113+
// CHECK: [[REF:%.*]] = load %swift.refcounted*, %swift.refcounted** [[DEST]]
114+
// CHECK: [[PTR:%.*]] = bitcast %swift.refcounted* [[REF]] to i8*
115+
// CHECK: [[PTR2:%.*]] = getelementptr inbounds i8, i8* [[PTR]], {{(i64|i32)}} {{(16|8)}}
116+
// CHECK: [[CAST:%.*]] = bitcast i8* [[PTR2]] to %swift.opaque*
117+
// CHECK: [[CAST2:%.*]] = bitcast %swift.opaque* [[CAST]] to %T4weak1AV*
118+
// CHECK: [[CAST3:%.*]] = bitcast %T4weak1AV* [[CAST2]] to %swift.opaque*
119+
// CHECK: ret %swift.opaque* [[CAST3]]
117120

118121
// destroy
119122
// CHECK: define linkonce_odr hidden void @"$S4weak1AVwxx"([[OPAQUE]]* noalias [[ARG:%.*]], [[TYPE]]*
@@ -166,6 +169,3 @@ bb0(%0 : $Optional<P>):
166169

167170
// TAILCALL: {{_?}}$S4weak1AVwta:
168171
// TAILCALL: jmp {{_?}}swift_weakTakeAssign
169-
170-
// TAILCALL: {{_?}}$S4weak1AVwTK:
171-
// TAILCALL: jmp {{_?}}swift_weakTakeInit

0 commit comments

Comments
 (0)