Skip to content

Commit c710b04

Browse files
committed
IRGen: Let the stride of a type be at least one, even for zero-sized types like the empty tuple.
This affects the computed stride for fixed-sized types in IRGen as well as the stored stride in value witness tables. The reason is to let comparisons and difference operations work for pointers to zero-sized types. (Currently this is achieved by using Builtin.strideof_nonzero in MemoryLayout.stride, but this requires a std::max(1, stride) operation after loading the stride)
1 parent 24e4456 commit c710b04

File tree

10 files changed

+114
-18
lines changed

10 files changed

+114
-18
lines changed

lib/IRGen/FixedTypeInfo.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,13 @@ class FixedTypeInfo : public TypeInfo {
120120
/// object. The stride is the storage size rounded up to the
121121
/// alignment; its practical use is that, in an array, it is the
122122
/// offset from the size of one element to the offset of the next.
123+
/// The stride is at least one, even for zero-sized types, like the empty
124+
/// tuple.
123125
Size getFixedStride() const {
124-
return StorageSize.roundUpToAlignment(getFixedAlignment());
126+
Size s = StorageSize.roundUpToAlignment(getFixedAlignment());
127+
if (s.isZero())
128+
s = Size(1);
129+
return s;
125130
}
126131

127132
/// Returns the fixed number of "extra inhabitants" (that is, bit

lib/IRGen/ValueWitness.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ enum class ValueWitness : unsigned {
228228

229229
/// size_t stride;
230230
///
231-
/// The required size per element of an array of this type.
231+
/// The required size per element of an array of this type. It is at least
232+
/// one, even for zero-sized types, like the empty tuple.
232233
Stride,
233234

234235
Last_RequiredValueWitness = Stride,

stdlib/public/Reflection/TypeLowering.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,8 @@ const RecordTypeInfo *RecordTypeInfoBuilder::build() {
421421

422422
// Calculate the stride
423423
unsigned Stride = ((Size + Alignment - 1) & ~(Alignment - 1));
424+
if (Stride == 0)
425+
Stride = 1;
424426

425427
return TC.makeTypeInfo<RecordTypeInfo>(
426428
Size, Alignment, Stride,
@@ -528,7 +530,7 @@ const TypeInfo *TypeConverter::getEmptyTypeInfo() {
528530
if (EmptyTI != nullptr)
529531
return EmptyTI;
530532

531-
EmptyTI = makeTypeInfo<TypeInfo>(TypeInfoKind::Builtin, 0, 1, 0, 0);
533+
EmptyTI = makeTypeInfo<TypeInfo>(TypeInfoKind::Builtin, 0, 1, 1, 0);
532534
return EmptyTI;
533535
}
534536

@@ -962,6 +964,8 @@ class EnumTypeInfoBuilder {
962964

963965
// Calculate the stride
964966
unsigned Stride = ((Size + Alignment - 1) & ~(Alignment - 1));
967+
if (Stride == 0)
968+
Stride = 1;
965969

966970
return TC.makeTypeInfo<RecordTypeInfo>(
967971
Size, Alignment, Stride,

stdlib/public/runtime/Enum.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ swift::swift_initEnumValueWitnessTableSinglePayload(ValueWitnessTable *vwtable,
102102
.withExtraInhabitants(unusedExtraInhabitants > 0)
103103
.withEnumWitnesses(true)
104104
.withInlineStorage(ValueWitnessTable::isValueInline(size, align));
105-
vwtable->stride = llvm::alignTo(size, align);
105+
auto rawStride = llvm::alignTo(size, align);
106+
vwtable->stride = rawStride == 0 ? 1 : rawStride;
106107

107108
// Substitute in better common value witnesses if we have them.
108109
// If the payload type is a single-refcounted pointer, and the enum has
@@ -310,7 +311,8 @@ swift::swift_initEnumMetadataMultiPayload(ValueWitnessTable *vwtable,
310311
.withEnumWitnesses(true)
311312
.withInlineStorage(ValueWitnessTable::isValueInline(totalSize, alignMask+1))
312313
;
313-
vwtable->stride = (totalSize + alignMask) & ~alignMask;
314+
auto rawStride = (totalSize + alignMask) & ~alignMask;
315+
vwtable->stride = rawStride == 0 ? 1 : rawStride;
314316

315317
installCommonValueWitnesses(vwtable);
316318
}

stdlib/public/runtime/ExistentialMetadataImpl.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,9 @@ struct LLVM_LIBRARY_VISIBILITY NonFixedOpaqueExistentialBox
214214
return std::max(alignof(void*), alignof(ValueBuffer));
215215
}
216216
static size_t getSize(unsigned numWitnessTables) {
217-
return sizeof(OpaqueExistentialContainer)
218-
+ numWitnessTables * sizeof(void*);
217+
constexpr size_t base = sizeof(OpaqueExistentialContainer);
218+
static_assert(base > 0, "stride needs base size > 0");
219+
return base + numWitnessTables * sizeof(void*);
219220
}
220221
static size_t getStride(unsigned numWitnessTables) {
221222
return getSize(numWitnessTables);
@@ -348,8 +349,9 @@ struct LLVM_LIBRARY_VISIBILITY NonFixedClassExistentialBox
348349
return alignof(void*);
349350
}
350351
static size_t getSize(unsigned numWitnessTables) {
351-
return sizeof(ClassExistentialContainer)
352-
+ numWitnessTables * sizeof(void*);
352+
constexpr size_t base = sizeof(ClassExistentialContainer);
353+
static_assert(base > 0, "stride needs base size > 0");
354+
return base + numWitnessTables * sizeof(void*);
353355
}
354356
static size_t getStride(unsigned numWitnessTables) {
355357
return getSize(numWitnessTables);
@@ -469,8 +471,9 @@ struct LLVM_LIBRARY_VISIBILITY NonFixedExistentialMetatypeBox
469471
return alignof(void*);
470472
}
471473
static size_t getSize(unsigned numWitnessTables) {
472-
return sizeof(ExistentialMetatypeContainer)
473-
+ numWitnessTables * sizeof(void*);
474+
constexpr size_t base = sizeof(ExistentialMetatypeContainer);
475+
static_assert(base > 0, "stride needs base size > 0");
476+
return base + numWitnessTables * sizeof(void*);
474477
}
475478
static size_t getStride(unsigned numWitnessTables) {
476479
return getSize(numWitnessTables);

stdlib/public/runtime/Metadata.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -982,7 +982,7 @@ void performBasicLayout(BasicLayout &layout,
982982
.withPOD(isPOD)
983983
.withBitwiseTakable(isBitwiseTakable)
984984
.withInlineStorage(isInline);
985-
layout.stride = roundUpToAlignMask(size, alignMask);
985+
layout.stride = std::max(size_t(1), roundUpToAlignMask(size, alignMask));
986986
}
987987
} // end anonymous namespace
988988

stdlib/public/runtime/MetadataImpl.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,9 @@ struct AggregateBox {
643643
using Helper = AggregateBoxHelper<0, EltBoxes...>;
644644
static constexpr size_t size = Helper::endOffset;
645645
static constexpr size_t alignment = Helper::alignment;
646-
static constexpr size_t stride = roundUpToAlignment(size, alignment);
646+
static constexpr size_t rawStride = roundUpToAlignment(size, alignment);
647+
static constexpr size_t stride = rawStride == 0 ? 1 : rawStride;
648+
647649
static constexpr bool isPOD = Helper::isPOD;
648650
static constexpr bool isBitwiseTakable = Helper::isBitwiseTakable;
649651

test/IRGen/indexing.sil

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@ entry(%p : $*DifferentSizeStride, %i: $Builtin.Word):
3131
return undef : $()
3232
}
3333

34+
// CHECK: define{{( protected)?}} void @zero_size(%swift.opaque* noalias nocapture, i64) {{.*}} {
35+
// CHECK: %2 = bitcast %swift.opaque* %0 to i8*
36+
// CHECK-NEXT: %3 = mul nsw i64 %1, 1
37+
// CHECK-NEXT: %4 = getelementptr inbounds i8, i8* %2, i64 %3
38+
sil @zero_size : $@convention(thin) (@in (), Builtin.Word) -> () {
39+
entry(%p : $*(), %i: $Builtin.Word):
40+
%q = index_addr %p : $*(), %i : $Builtin.Word
41+
return undef : $()
42+
}
43+
3444
// CHECK: define{{( protected)?}} void @dynamic_size(%swift.opaque* noalias nocapture, i64, %swift.type* %T) {{.*}} {
3545
// CHECK: %3 = bitcast %swift.type* %T to i8***
3646
// CHECK-NEXT: %4 = getelementptr inbounds i8**, i8*** %3, i64 -1

test/IRGen/zero_size_types.swift

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// RUN: rm -rf %t && mkdir -p %t
2+
// RUN: %target-build-swift -Xllvm -sil-disable-pass="Generic Specializer" %s -o %t/a.out
3+
// RUN: %target-run %t/a.out | %FileCheck %s
4+
// REQUIRES: executable_test
5+
6+
public struct EmptyStruct {
7+
}
8+
9+
public enum SingleCaseEnum {
10+
case A
11+
}
12+
13+
public struct GenStruct<T> {
14+
var t: T
15+
}
16+
17+
@inline(never)
18+
func fail() {
19+
fatalError("size/stride mismatch")
20+
}
21+
22+
@inline(never)
23+
public func testit<T>(_ t: T) {
24+
if MemoryLayout<T>.size != 0 {
25+
fail()
26+
}
27+
if MemoryLayout<T>.stride != 1 {
28+
fail()
29+
}
30+
if MemoryLayout<GenStruct<T>>.size != 0 {
31+
fail()
32+
}
33+
if MemoryLayout<GenStruct<T>>.stride != 1 {
34+
fail()
35+
}
36+
}
37+
38+
// Test size and stride which are loaded from the value witness tables.
39+
40+
testit(EmptyStruct())
41+
testit(())
42+
testit(SingleCaseEnum.A)
43+
44+
// Test size and stride which are computed as constants in IRGen.
45+
46+
if MemoryLayout<()>.size != 0 {
47+
fail()
48+
}
49+
if MemoryLayout<()>.stride != 1 {
50+
fail()
51+
}
52+
53+
if MemoryLayout<EmptyStruct>.size != 0 {
54+
fail()
55+
}
56+
if MemoryLayout<EmptyStruct>.stride != 1 {
57+
fail()
58+
}
59+
60+
if MemoryLayout<SingleCaseEnum>.size != 0 {
61+
fail()
62+
}
63+
if MemoryLayout<SingleCaseEnum>.stride != 1 {
64+
fail()
65+
}
66+
67+
// CHECK: success
68+
print("success")
69+

test/Reflection/typeref_lowering.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ V12TypeLowering14MetatypeStruct
594594
// CHECK-64-NEXT: (field name=wtable offset=16
595595
// CHECK-64-NEXT: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=1))))))
596596
// CHECK-64-NEXT: (field name=structMetatype offset=112
597-
// CHECK-64-NEXT: (builtin size=0 alignment=1 stride=0 num_extra_inhabitants=0))
597+
// CHECK-64-NEXT: (builtin size=0 alignment=1 stride=1 num_extra_inhabitants=0))
598598
// CHECK-64-NEXT: (field name=optionalStructMetatype offset=112
599599
// CHECK-64-NEXT: (single_payload_enum size=8 alignment=8 stride=8 num_extra_inhabitants=[[PTR_XI_SUB_1]]
600600
// CHECK-64-NEXT: (field name=some offset=0
@@ -667,7 +667,7 @@ V12TypeLowering14MetatypeStruct
667667
// CHECK-32-NEXT: (field name=wtable offset=8
668668
// CHECK-32-NEXT: (builtin size=4 alignment=4 stride=4 num_extra_inhabitants=1))))))
669669
// CHECK-32-NEXT: (field name=structMetatype offset=56
670-
// CHECK-32-NEXT: (builtin size=0 alignment=1 stride=0 num_extra_inhabitants=0))
670+
// CHECK-32-NEXT: (builtin size=0 alignment=1 stride=1 num_extra_inhabitants=0))
671671
// CHECK-32-NEXT: (field name=optionalStructMetatype offset=56
672672
// CHECK-32-NEXT: (single_payload_enum size=4 alignment=4 stride=4 num_extra_inhabitants=4095
673673
// CHECK-32-NEXT: (field name=some offset=0
@@ -689,11 +689,11 @@ V12TypeLowering10EnumStruct
689689
// CHECK-64: (struct TypeLowering.EnumStruct)
690690
// CHECK-64-NEXT: (struct size=81 alignment=8 stride=88 num_extra_inhabitants=0
691691
// CHECK-64-NEXT: (field name=empty offset=0
692-
// CHECK-64-NEXT: (no_payload_enum size=0 alignment=0 stride=0 num_extra_inhabitants=0))
692+
// CHECK-64-NEXT: (no_payload_enum size=0 alignment=0 stride=1 num_extra_inhabitants=0))
693693
// CHECK-64-NEXT: (field name=noPayload offset=0
694-
// CHECK-64-NEXT: (no_payload_enum size=1 alignment=0 stride=0 num_extra_inhabitants=0))
694+
// CHECK-64-NEXT: (no_payload_enum size=1 alignment=0 stride=1 num_extra_inhabitants=0))
695695
// CHECK-64-NEXT: (field name=sillyNoPayload offset=0
696-
// CHECK-64-NEXT: (no_payload_enum size=1 alignment=0 stride=0 num_extra_inhabitants=0))
696+
// CHECK-64-NEXT: (no_payload_enum size=1 alignment=0 stride=1 num_extra_inhabitants=0))
697697
// CHECK-64-NEXT: (field name=singleton offset=8
698698
// CHECK-64-NEXT: (reference kind=strong refcounting=native))
699699
// CHECK-64-NEXT: (field name=singlePayload offset=16

0 commit comments

Comments
 (0)