Skip to content

Commit 85ae390

Browse files
committed
IRGen: Fix extra inhabitants of multi-payload enums with more spare bits than tag bits.
Because layout minimizes the number of tag bits used, and favors high spare bits, the spare bit representations end up overlapping the extra inhabitant representations, since we just counted down from -1. If there are fewer tag bits than total spare bits, rotate the extra inhabitant values so they correctly line up with the tag representations in this situation. rdar://problem/46468090
1 parent a061df5 commit 85ae390

File tree

2 files changed

+142
-1
lines changed

2 files changed

+142
-1
lines changed

lib/IRGen/GenEnum.cpp

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4844,6 +4844,31 @@ namespace {
48444844
}
48454845
return addr;
48464846
}
4847+
4848+
// If there are common spare bits we didn't use for tags, rotate the
4849+
// extra inhabitant values so that the used tag bits are at the bottom.
4850+
// This will cleanly separate the used tag values from the extra inhabitants
4851+
// so we can discriminate them with one comparison. The tag favors high
4852+
// bits, whereas extra inhabitants count down from -1 using all bits
4853+
// (capping out at up to 32 spare bits, in which case the lowest 32
4854+
// bits are used).
4855+
std::pair<unsigned, unsigned> getRotationAmountsForExtraInhabitants() const{
4856+
assert([&]{
4857+
auto maskedBits = PayloadTagBits;
4858+
maskedBits &= CommonSpareBits;
4859+
return maskedBits == PayloadTagBits;
4860+
}());
4861+
4862+
unsigned commonSpareBitsCount = CommonSpareBits.count();
4863+
unsigned payloadTagBitsCount = PayloadTagBits.count();
4864+
if (commonSpareBitsCount == payloadTagBitsCount
4865+
|| commonSpareBitsCount - payloadTagBitsCount >= 32) {
4866+
return std::make_pair(0, 0);
4867+
}
4868+
unsigned shlAmount = commonSpareBitsCount - payloadTagBitsCount;
4869+
unsigned shrAmount = std::min(commonSpareBitsCount, 32u) - shlAmount;
4870+
return {shlAmount, shrAmount};
4871+
}
48474872

48484873
llvm::Value *getExtraInhabitantIndex(IRGenFunction &IGF,
48494874
Address src,
@@ -4859,6 +4884,32 @@ namespace {
48594884
auto payload = EnumPayload::load(IGF, projectPayload(IGF, src),
48604885
PayloadSchema);
48614886
tag = payload.emitGatherSpareBits(IGF, CommonSpareBits, 0, 32);
4887+
4888+
// If there are common spare bits we didn't use for tags, rotate the
4889+
// tag value so that the used tag bits are at the bottom. This will
4890+
// cleanly separate the used tag values from the extra inhabitants so
4891+
// we can discriminate them with one comparison. The tag favors high
4892+
// bits, whereas extra inhabitants count down from -1 using all bits
4893+
// (capping out at up to 32 spare bits, in which case the lowest 32
4894+
// bits are used).
4895+
//
4896+
// Note that since this is the inverse operation--we're taking the bits
4897+
// out of a payload and mapping them back to an extra inhabitant index--
4898+
// the `shr` and `shl` amounts are intentionally swapped here.
4899+
unsigned shrAmount, shlAmount;
4900+
std::tie(shrAmount, shlAmount) = getRotationAmountsForExtraInhabitants();
4901+
if (shrAmount != 0) {
4902+
assert(getExtraTagBitCountForExtraInhabitants() == 0);
4903+
auto tagLo = IGF.Builder.CreateLShr(tag, shrAmount);
4904+
auto tagHi = IGF.Builder.CreateShl(tag, shlAmount);
4905+
tag = IGF.Builder.CreateOr(tagLo, tagHi);
4906+
if (CommonSpareBits.count() < 32) {
4907+
auto mask = llvm::ConstantInt::get(IGF.IGM.Int32Ty,
4908+
(1u << CommonSpareBits.count()) - 1u);
4909+
tag = IGF.Builder.CreateAnd(tag, mask);
4910+
}
4911+
}
4912+
48624913
if (getExtraTagBitCountForExtraInhabitants()) {
48634914
auto extraTagAddr = projectExtraTagBitsForExtraInhabitants(IGF, src);
48644915
auto extraTag = IGF.Builder.CreateLoad(extraTagAddr);
@@ -4899,6 +4950,29 @@ namespace {
48994950
}
49004951

49014952
auto indexValue = IGF.Builder.CreateNot(index);
4953+
4954+
// If there are common spare bits we didn't use for tags, rotate the
4955+
// tag value so that the used tag bits are at the bottom. This will
4956+
// cleanly separate the used tag values from the extra inhabitants so
4957+
// we can discriminate them with one comparison. The tag favors high
4958+
// bits, whereas extra inhabitants count down from -1 using all bits
4959+
// (capping out at up to 32 spare bits, in which case the lowest 32
4960+
// bits are used).
4961+
unsigned shlAmount, shrAmount;
4962+
std::tie(shlAmount, shrAmount) = getRotationAmountsForExtraInhabitants();
4963+
4964+
if (shlAmount != 0) {
4965+
assert(getExtraTagBitCountForExtraInhabitants() == 0);
4966+
if (CommonSpareBits.count() < 32) {
4967+
auto mask = llvm::ConstantInt::get(IGF.IGM.Int32Ty,
4968+
(1u << CommonSpareBits.count()) - 1u);
4969+
indexValue = IGF.Builder.CreateAnd(indexValue, mask);
4970+
}
4971+
auto indexValueHi = IGF.Builder.CreateShl(indexValue, shlAmount);
4972+
auto indexValueLo = IGF.Builder.CreateLShr(indexValue, shrAmount);
4973+
indexValue = IGF.Builder.CreateOr(indexValueHi, indexValueLo);
4974+
}
4975+
49024976
if (CommonSpareBits.count()) {
49034977
// Factor the index value into parts to scatter into the payload and
49044978
// to store in the extra tag bits, if any.
@@ -4952,6 +5026,25 @@ namespace {
49525026
// Count down from all-ones since a small negative number constant is
49535027
// likely to be easier to reify.
49545028
auto mask = ~index;
5029+
5030+
// If there are common spare bits we didn't use for tags, rotate the
5031+
// tag value so that the used tag bits are at the bottom. This will
5032+
// cleanly separate the used tag values from the extra inhabitants so
5033+
// we can discriminate them with one comparison. The tag favors high
5034+
// bits, whereas extra inhabitants count down from -1 using all bits
5035+
// (capping out at up to 32 spare bits, in which case the lowest 32
5036+
// bits are used).
5037+
unsigned shlAmount, shrAmount;
5038+
std::tie(shlAmount, shrAmount) = getRotationAmountsForExtraInhabitants();
5039+
5040+
if (shlAmount != 0) {
5041+
assert(getExtraTagBitCountForExtraInhabitants() == 0);
5042+
if (CommonSpareBits.count() < 32) {
5043+
mask &= (1u << CommonSpareBits.count()) - 1;
5044+
}
5045+
mask = (mask >> shrAmount) | (mask << shlAmount);
5046+
}
5047+
49555048
auto extraTagMask = getExtraTagBitCountForExtraInhabitants() >= 32
49565049
? ~0u : (1 << getExtraTagBitCountForExtraInhabitants()) - 1;
49575050

test/Interpreter/multi_payload_extra_inhabitant.swift

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %empty-directory(%t)
22

3-
// RUN: %target-build-swift -parse-stdlib -Xfrontend -verify-type-layout -Xfrontend SpareBitExtraInhabitants -Xfrontend -verify-type-layout -Xfrontend SpareBitSingleExtraInhabitant -Xfrontend -verify-type-layout -Xfrontend SpareBitNoExtraInhabitant -Xfrontend -verify-type-layout -Xfrontend SpareBitNoExtraInhabitant2 -Xfrontend -verify-type-layout -Xfrontend TwoTagExtraInhabitants -Xfrontend -verify-type-layout -Xfrontend ThreeTagExtraInhabitants -Xfrontend -verify-type-layout -Xfrontend NoTagExtraInhabitants -Xfrontend -verify-type-layout -Xfrontend DynamicExtraInhabitantsNever -Xfrontend -verify-type-layout -Xfrontend DynamicExtraInhabitantsZeroBytes -Xfrontend -verify-type-layout -Xfrontend DynamicExtraInhabitantsOneByte -Xfrontend -verify-type-layout -Xfrontend DynamicExtraInhabitantsTwoBytes -O -o %t/a.out %s
3+
// RUN: %target-build-swift -parse-stdlib -Xfrontend -verify-type-layout -Xfrontend SpareBitExtraInhabitants -Xfrontend -verify-type-layout -Xfrontend SpareBitSingleExtraInhabitant -Xfrontend -verify-type-layout -Xfrontend SpareBitNoExtraInhabitant -Xfrontend -verify-type-layout -Xfrontend SpareBitNoExtraInhabitant2 -Xfrontend -verify-type-layout -Xfrontend TwoTagExtraInhabitants -Xfrontend -verify-type-layout -Xfrontend ThreeTagExtraInhabitants -Xfrontend -verify-type-layout -Xfrontend NoTagExtraInhabitants -Xfrontend -verify-type-layout -Xfrontend DynamicExtraInhabitantsNever -Xfrontend -verify-type-layout -Xfrontend DynamicExtraInhabitantsZeroBytes -Xfrontend -verify-type-layout -Xfrontend DynamicExtraInhabitantsOneByte -Xfrontend -verify-type-layout -Xfrontend DynamicExtraInhabitantsTwoBytes -Xfrontend -verify-type-layout -Xfrontend MoreSpareBitsThanTagsExtraInhabitants -Xfrontend -verify-type-layout -Xfrontend OptOptMoreSpareBitsThanTagsExtraInhabitants -O -o %t/a.out %s
44
// RUN: %target-run %t/a.out 2>&1
55

66
// Type layout verifier is only compiled into the runtime in asserts builds.
@@ -50,6 +50,21 @@ enum ThreeTagExtraInhabitants {
5050
case c(Builtin.Int32)
5151
}
5252

53+
enum MoreSpareBitsThanTagsExtraInhabitants {
54+
case a(Builtin.Int29)
55+
case b(Builtin.Int29)
56+
case c(Builtin.Int29)
57+
case d(Builtin.Int29)
58+
}
59+
typealias OptOptMoreSpareBitsThanTagsExtraInhabitants =
60+
Optional<Optional<MoreSpareBitsThanTagsExtraInhabitants>>
61+
62+
enum MoreSpareBitsThanTagsExtraInhabitants2 {
63+
case a(Builtin.Int29)
64+
case b(Builtin.Int29)
65+
case c(Builtin.Int29)
66+
}
67+
5368
enum NoTagExtraInhabitants {
5469
case aaa(Builtin.Int32), aab(Builtin.Int32), aac(Builtin.Int32), aad(Builtin.Int32), aae(Builtin.Int32), aaf(Builtin.Int32), aag(Builtin.Int32), aah(Builtin.Int32)
5570
case aba(Builtin.Int32), abb(Builtin.Int32), abc(Builtin.Int32), abd(Builtin.Int32), abe(Builtin.Int32), abf(Builtin.Int32), abg(Builtin.Int32), abh(Builtin.Int32)
@@ -201,4 +216,37 @@ tests.test("types that have at least two extra inhabitants") {
201216
expectHasAtLeastTwoExtraInhabitants(DynamicExtraInhabitantsOneByte.self, nil: nil, someNil: .some(nil))
202217
expectHasAtLeastTwoExtraInhabitants(DynamicExtraInhabitantsTwoBytes.self, nil: nil, someNil: .some(nil))
203218
}
219+
tests.test("types with more spare bits than used by tags") {
220+
expectHasAtLeastTwoExtraInhabitants(MoreSpareBitsThanTagsExtraInhabitants.self,
221+
nil: nil, someNil: .some(nil))
222+
223+
for x in [MoreSpareBitsThanTagsExtraInhabitants.a(Builtin.zeroInitializer()),
224+
MoreSpareBitsThanTagsExtraInhabitants.b(Builtin.zeroInitializer()),
225+
MoreSpareBitsThanTagsExtraInhabitants.c(Builtin.zeroInitializer()),
226+
MoreSpareBitsThanTagsExtraInhabitants.d(Builtin.zeroInitializer())]{
227+
let opt = Optional(x)
228+
expectNotNil(opt)
229+
let opt2 = Optional(opt)
230+
expectNotNil(opt2)
231+
let opt3 = Optional(opt2)
232+
expectNotNil(opt3)
233+
let opt4 = Optional(opt3)
234+
expectNotNil(opt4)
235+
}
236+
237+
for x in [MoreSpareBitsThanTagsExtraInhabitants.a(Builtin.zeroInitializer()),
238+
MoreSpareBitsThanTagsExtraInhabitants.b(Builtin.zeroInitializer()),
239+
MoreSpareBitsThanTagsExtraInhabitants.c(Builtin.zeroInitializer())]{
240+
let opt = Optional(x)
241+
expectNotNil(opt)
242+
let opt2 = Optional(opt)
243+
expectNotNil(opt2)
244+
let opt3 = Optional(opt2)
245+
expectNotNil(opt3)
246+
let opt4 = Optional(opt3)
247+
expectNotNil(opt4)
248+
let opt5 = Optional(opt4)
249+
expectNotNil(opt5)
250+
}
251+
}
204252
runAllTests()

0 commit comments

Comments
 (0)