Skip to content

Commit fa86118

Browse files
authored
Merge pull request #4774 from eeckstein/stride-1
Let the stride of a type be at least one and remove the strideof_nonzero builtin
2 parents 1941975 + 7c37406 commit fa86118

File tree

19 files changed

+125
-72
lines changed

19 files changed

+125
-72
lines changed

include/swift/AST/Builtins.def

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -358,11 +358,6 @@ BUILTIN_MISC_OPERATION(IsPOD, "ispod", "n", Special)
358358
/// Alignof has type T.Type -> Int
359359
BUILTIN_MISC_OPERATION(Alignof, "alignof", "n", Special)
360360

361-
/// Strideof has type T.Type -> Int.
362-
/// Note that this version will never return 0. It instead always returns
363-
/// at least 1
364-
BUILTIN_MISC_OPERATION(StrideofNonZero, "strideof_nonzero", "n", Special)
365-
366361
/// AllocRaw has type (Int, Int) -> Builtin.RawPointer
367362
BUILTIN_MISC_OPERATION(AllocRaw, "allocRaw", "", Special)
368363

lib/AST/Builtins.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1559,7 +1559,6 @@ ValueDecl *swift::getBuiltinValueDecl(ASTContext &Context, Identifier Id) {
15591559
case BuiltinValueKind::Sizeof:
15601560
case BuiltinValueKind::Strideof:
15611561
case BuiltinValueKind::Alignof:
1562-
case BuiltinValueKind::StrideofNonZero:
15631562
return getSizeOrAlignOfOperation(Context, Id);
15641563

15651564
case BuiltinValueKind::IsPOD:

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/GenBuiltin.cpp

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -171,22 +171,6 @@ void irgen::emitBuiltinCall(IRGenFunction &IGF, Identifier FnId,
171171
return;
172172
}
173173

174-
if (Builtin.ID == BuiltinValueKind::StrideofNonZero) {
175-
// Note this case must never return 0.
176-
// It is implemented as max(strideof, 1)
177-
args.claimAll();
178-
auto valueTy = getLoweredTypeAndTypeInfo(IGF.IGM,
179-
substitutions[0].getReplacement());
180-
// Strideof should never return 0, so return 1 if the type has a 0 stride.
181-
llvm::Value *StrideOf = valueTy.second.getStride(IGF, valueTy.first);
182-
llvm::IntegerType *IntTy = cast<llvm::IntegerType>(StrideOf->getType());
183-
auto *Zero = llvm::ConstantInt::get(IntTy, 0);
184-
auto *One = llvm::ConstantInt::get(IntTy, 1);
185-
llvm::Value *Cmp = IGF.Builder.CreateICmpEQ(StrideOf, Zero);
186-
out.add(IGF.Builder.CreateSelect(Cmp, One, StrideOf));
187-
return;
188-
}
189-
190174
if (Builtin.ID == BuiltinValueKind::IsPOD) {
191175
args.claimAll();
192176
auto valueTy = getLoweredTypeAndTypeInfo(IGF.IGM,

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,

lib/SILOptimizer/Analysis/ValueTracking.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,6 @@ Optional<bool> swift::computeSignBit(SILValue V) {
141141
// Strideof always returns non-negative results.
142142
case BuiltinValueKind::Strideof:
143143
return false;
144-
// StrideofNonZero always returns positive results.
145-
case BuiltinValueKind::StrideofNonZero:
146-
return false;
147144
// Alignof always returns non-negative results.
148145
case BuiltinValueKind::Alignof:
149146
return false;

lib/SILOptimizer/SILCombiner/SILCombinerBuiltinVisitors.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ matchSizeOfMultiplication(SILValue I, MetatypeInst *RequiredType,
184184
m_ApplyInst(
185185
BuiltinValueKind::SMulOver, m_SILValue(Dist),
186186
m_ApplyInst(BuiltinValueKind::ZExtOrBitCast,
187-
m_ApplyInst(BuiltinValueKind::StrideofNonZero,
187+
m_ApplyInst(BuiltinValueKind::Strideof,
188188
m_MetatypeInst(StrideType)))),
189189
0))) ||
190190
match(
@@ -195,7 +195,7 @@ matchSizeOfMultiplication(SILValue I, MetatypeInst *RequiredType,
195195
m_ApplyInst(
196196
BuiltinValueKind::SMulOver,
197197
m_ApplyInst(BuiltinValueKind::ZExtOrBitCast,
198-
m_ApplyInst(BuiltinValueKind::StrideofNonZero,
198+
m_ApplyInst(BuiltinValueKind::Strideof,
199199
m_MetatypeInst(StrideType))),
200200
m_SILValue(Dist)),
201201
0)))) {
@@ -464,9 +464,6 @@ SILInstruction *SILCombiner::visitBuiltinInst(BuiltinInst *I) {
464464

465465
if (match(I, m_ApplyInst(BuiltinValueKind::SMulOver,
466466
m_ApplyInst(BuiltinValueKind::Strideof),
467-
m_ValueBase(), m_IntegerLiteralInst())) ||
468-
match(I, m_ApplyInst(BuiltinValueKind::SMulOver,
469-
m_ApplyInst(BuiltinValueKind::StrideofNonZero),
470467
m_ValueBase(), m_IntegerLiteralInst()))) {
471468
I->swapOperands(0, 1);
472469
return I;

lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ visitPointerToAddressInst(PointerToAddressInst *PTAI) {
9090
// Turn this also into an index_addr. We generate this pattern after switching
9191
// the Word type to an explicit Int32 or Int64 in the stdlib.
9292
//
93-
// %101 = builtin "strideof_nonzero"<Int>(%84 : $@thick Int.Type) :
93+
// %101 = builtin "strideof"<Int>(%84 : $@thick Int.Type) :
9494
// $Builtin.Word
9595
// %102 = builtin "zextOrBitCast_Word_Int64"(%101 : $Builtin.Word) :
9696
// $Builtin.Int64
@@ -119,13 +119,13 @@ visitPointerToAddressInst(PointerToAddressInst *PTAI) {
119119
m_ApplyInst(
120120
BuiltinValueKind::SMulOver, m_SILValue(Distance),
121121
m_ApplyInst(BuiltinValueKind::ZExtOrBitCast,
122-
m_ApplyInst(BuiltinValueKind::StrideofNonZero,
122+
m_ApplyInst(BuiltinValueKind::Strideof,
123123
m_MetatypeInst(Metatype))))) ||
124124
match(StrideMul,
125125
m_ApplyInst(
126126
BuiltinValueKind::SMulOver,
127127
m_ApplyInst(BuiltinValueKind::ZExtOrBitCast,
128-
m_ApplyInst(BuiltinValueKind::StrideofNonZero,
128+
m_ApplyInst(BuiltinValueKind::Strideof,
129129
m_MetatypeInst(Metatype))),
130130
m_SILValue(Distance)))) {
131131
SILType InstanceType =
@@ -167,10 +167,6 @@ visitPointerToAddressInst(PointerToAddressInst *PTAI) {
167167
if (match(Bytes, m_ApplyInst(BuiltinValueKind::SMulOver, m_ValueBase(),
168168
m_ApplyInst(BuiltinValueKind::Strideof,
169169
m_MetatypeInst(Metatype)),
170-
m_ValueBase())) ||
171-
match(Bytes, m_ApplyInst(BuiltinValueKind::SMulOver, m_ValueBase(),
172-
m_ApplyInst(BuiltinValueKind::StrideofNonZero,
173-
m_MetatypeInst(Metatype)),
174170
m_ValueBase()))) {
175171
SILType InstanceType =
176172
Metatype->getType().getMetatypeInstanceType(PTAI->getModule());

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/core/MemoryLayout.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public enum MemoryLayout<T> {
3030
/// performance for space efficiency. The result is always positive.
3131
@_transparent
3232
public static var stride: Int {
33-
return Int(Builtin.strideof_nonzero(T.self))
33+
return Int(Builtin.strideof(T.self))
3434
}
3535

3636
/// The default memory alignment of `T`.

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/builtins.swift

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -245,17 +245,6 @@ func generic_strideof_test<T>(_: T) {
245245
var s = Builtin.strideof(T.self)
246246
}
247247

248-
// CHECK: define hidden void @_TF8builtins29generic_strideof_nonzero_testurFxT_(
249-
func generic_strideof_nonzero_test<T>(_: T) {
250-
// CHECK: [[T0:%.*]] = getelementptr inbounds i8*, i8** [[T:%.*]], i32 19
251-
// CHECK-NEXT: [[T1:%.*]] = load i8*, i8** [[T0]]
252-
// CHECK-NEXT: [[STRIDE:%.*]] = ptrtoint i8* [[T1]] to i64
253-
// CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[STRIDE]], 0
254-
// CHECK-NEXT: [[SELECT:%.*]] = select i1 [[CMP]], i64 1, i64 [[STRIDE]]
255-
// CHECK-NEXT: store i64 [[SELECT]], i64* [[S:%.*]]
256-
var s = Builtin.strideof_nonzero(T.self)
257-
}
258-
259248
class X {}
260249

261250
class Y {}

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)