Skip to content

Commit f538dd7

Browse files
committed
Fix Enums with multiple payloads of < 32bit and empty payloads.
This used to crash because the code storing empty payload enum tag values would use the bit width of the tag (32 bit) as the minimum unit to store to the payload even if the actual bits required to store the biggest tag value in the payload was much smaller. With payload bit-widths < 32bit we would run out of space crashing looking for new payload to store the value to ... Instead pass the maximum size of the bits that need storing down. rdar://26926035
1 parent c2c1b84 commit f538dd7

File tree

4 files changed

+70
-8
lines changed

4 files changed

+70
-8
lines changed

lib/IRGen/EnumPayload.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,15 @@ template<typename Fn>
8484
static void withValueInPayload(IRGenFunction &IGF,
8585
const EnumPayload &payload,
8686
llvm::Type *valueType,
87+
int numBitsUsedInValue,
8788
unsigned payloadOffset,
8889
Fn &&f) {
8990
auto &DataLayout = IGF.IGM.DataLayout;
90-
int valueBitWidth = DataLayout.getTypeSizeInBits(valueType);
91-
91+
int valueTypeBitWidth = DataLayout.getTypeSizeInBits(valueType);
92+
int valueBitWidth =
93+
numBitsUsedInValue < 0 ? valueTypeBitWidth : numBitsUsedInValue;
94+
assert(numBitsUsedInValue <= valueTypeBitWidth);
95+
9296
// Find the elements we need to touch.
9397
// TODO: Linear search through the payload elements is lame.
9498
MutableArrayRef<EnumPayload::LazyValue> payloads = payload.PayloadValues;
@@ -107,7 +111,7 @@ static void withValueInPayload(IRGenFunction &IGF,
107111

108112
f(payloads.front(),
109113
payloadBitWidth, payloadValueOffset,
110-
valueBitWidth, valueOffset);
114+
valueTypeBitWidth, valueOffset);
111115

112116
// If we used the entire value, we're done.
113117
valueOffset += valueChunkWidth;
@@ -121,8 +125,9 @@ static void withValueInPayload(IRGenFunction &IGF,
121125
}
122126

123127
void EnumPayload::insertValue(IRGenFunction &IGF, llvm::Value *value,
124-
unsigned payloadOffset) {
125-
withValueInPayload(IGF, *this, value->getType(), payloadOffset,
128+
unsigned payloadOffset,
129+
int numBitsUsedInValue) {
130+
withValueInPayload(IGF, *this, value->getType(), numBitsUsedInValue, payloadOffset,
126131
[&](LazyValue &payloadValue,
127132
unsigned payloadBitWidth,
128133
unsigned payloadValueOffset,
@@ -182,7 +187,7 @@ void EnumPayload::insertValue(IRGenFunction &IGF, llvm::Value *value,
182187
llvm::Value *EnumPayload::extractValue(IRGenFunction &IGF, llvm::Type *type,
183188
unsigned payloadOffset) const {
184189
llvm::Value *result = nullptr;
185-
withValueInPayload(IGF, *this, type, payloadOffset,
190+
withValueInPayload(IGF, *this, type, -1, payloadOffset,
186191
[&](LazyValue &payloadValue,
187192
unsigned payloadBitWidth,
188193
unsigned payloadValueOffset,

lib/IRGen/EnumPayload.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,12 @@ class EnumPayload {
117117
/// Insert a value into the enum payload.
118118
///
119119
/// The current payload value at the given offset is assumed to be zero.
120+
/// If \p numBitsUsedInValue is non-negative denotes the actual number of bits
121+
/// that need storing in \p value otherwise the full bit-width of \p value
122+
/// will be stored.
120123
void insertValue(IRGenFunction &IGF,
121-
llvm::Value *value, unsigned bitOffset);
124+
llvm::Value *value, unsigned bitOffset,
125+
int numBitsUsedInValue = -1);
122126

123127
/// Extract a value from the enum payload.
124128
llvm::Value *extractValue(IRGenFunction &IGF,

lib/IRGen/GenEnum.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3570,7 +3570,10 @@ namespace {
35703570
} else if (CommonSpareBits.size() > 0) {
35713571
// Otherwise the payload is just the index.
35723572
payload = EnumPayload::zero(IGM, PayloadSchema);
3573-
payload.insertValue(IGF, tagIndex, 0);
3573+
// We know we won't use more than numCaseBits from tagIndex's value:
3574+
// We made sure of this in the logic above.
3575+
payload.insertValue(IGF, tagIndex, 0,
3576+
numCaseBits >= 32 ? -1 : numCaseBits);
35743577
}
35753578

35763579
// If the tag bits do not fit in the spare bits, the remaining tag bits

test/IRGen/enum.sil

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ import Swift
9797

9898
// CHECK: [[DYNAMIC_SINGLETON:%O4enum16DynamicSingleton.*]] = type <{}>
9999

100+
// CHECK: %O4enum40MultiPayloadLessThan32BitsWithEmptyCases = type <{ [1 x i8], [1 x i8] }>
101+
100102
// -- Dynamic metadata template carries a value witness table pattern
101103
// we fill in on instantiation.
102104
// The witness table pattern includes extra inhabitant witness
@@ -2628,3 +2630,51 @@ bb4:
26282630
// CHECK-64: %5 = or i128 %2, %4
26292631
// -- 0x1__0000_0000_0000_0000
26302632
// CHECK-64: %6 = or i128 %5, 18446744073709551616
2633+
2634+
// CHECK-LABEL: define linkonce_odr hidden i32 @_TwugO4enum40MultiPayloadLessThan32BitsWithEmptyCases(%swift.opaque* %value
2635+
// CHECK: [[VAL00:%.*]] = bitcast %swift.opaque* %value to %O4enum40MultiPayloadLessThan32BitsWithEmptyCases*
2636+
// CHECK: [[VAL01:%.*]] = bitcast %O4enum40MultiPayloadLessThan32BitsWithEmptyCases* [[VAL00]] to i8*
2637+
// CHECK: [[VAL02:%.*]] = load {{.*}} [[VAL01]]
2638+
// CHECK: [[VAL03:%.*]] = getelementptr inbounds {{.*}} [[VAL00]], i32 0, i32 1
2639+
// CHECK: [[VAL04:%.*]] = bitcast {{.*}} [[VAL03]] to i2*
2640+
// CHECK: [[VAL05:%.*]] = load {{.*}} [[VAL04]]
2641+
// CHECK: [[VAL06:%.*]] = zext i2 [[VAL05]] to i32
2642+
// CHECK: [[VAL07:%.*]] = sub i32 [[VAL06]], 2
2643+
// CHECK: [[VAL08:%.*]] = zext i8 [[VAL02]] to i32
2644+
// CHECK: [[VAL09:%.*]] = and i32 [[VAL08]], 255
2645+
// CHECK: [[VAL10:%.*]] = icmp sge i32 [[VAL07]], 0
2646+
// CHECK: [[VAL11:%.*]] = select i1 [[VAL10]], i32 [[VAL09]], i32 [[VAL07]]
2647+
// CHECK: ret i32 [[VAL11]]
2648+
2649+
// CHECK-LABEL: define linkonce_odr hidden void @_TwuiO4enum40MultiPayloadLessThan32BitsWithEmptyCases(%swift.opaque* %value, i32 %tag
2650+
// CHECK: entry:
2651+
// CHECK: [[VAL00:%.*]] = bitcast %swift.opaque* %value
2652+
// CHECK: [[VAL01:%.*]] = icmp sge i32 %tag, 0
2653+
// CHECK: br i1 [[VAL01]], label %[[TLABEL:.*]], label %[[FLABEL:.*]]
2654+
2655+
// CHECK: <label>:[[TLABEL]]
2656+
// CHECK: [[VAL03:%.*]] = trunc i32 %tag to i8
2657+
// CHECK: [[VAL04:%.*]] = bitcast %O4enum40MultiPayloadLessThan32BitsWithEmptyCases* [[VAL00]] to i8*
2658+
// CHECK: store i8 [[VAL03]], i8* [[VAL04]]
2659+
// CHECK: [[VAL05:%.*]] = getelementptr inbounds {{.*}} [[VAL00]], i32 0, i32 1
2660+
// CHECK: [[VAL06:%.*]] = bitcast [1 x i8]* [[VAL05]] to i2*
2661+
// CHECK: store i2 -2, i2* [[VAL06]]
2662+
// CHECK: br label %[[RLABEL:.*]]
2663+
2664+
// CHECK: <label>:[[FLABEL]]
2665+
// CHECK: [[VAL08:%.*]] = add i32 %tag, 2
2666+
// CHECK: [[VAL09:%.*]] = trunc i32 [[VAL08]] to i2
2667+
// CHECK: [[VAL10:%.*]] = getelementptr inbounds {{.*}} [[VAL00]], i32 0, i32 1
2668+
// CHECK: [[VAL11:%.*]] = bitcast [1 x i8]* [[VAL10]] to i2*
2669+
// CHECK: store i2 [[VAL09]], i2* [[VAL11]]
2670+
// CHECK: br label %[[RLABEL]]
2671+
2672+
// CHECK: <label>:[[RLABEL]]
2673+
// CHECK: ret void
2674+
2675+
enum MultiPayloadLessThan32BitsWithEmptyCases {
2676+
case A
2677+
case B
2678+
case C(Builtin.Int8)
2679+
case D(Builtin.Int8)
2680+
}

0 commit comments

Comments
 (0)