Skip to content

Commit

Permalink
IRGen: Clean up class archetype spare bits hack
Browse files Browse the repository at this point in the history
We were calling hasTypeParameter() on the interface type of the enum
element. Since enum elements are case constructors now, the interface
type was a GenericFunctionType, and since conceptually these cannot
contain free type variables, this would always return to false.

The right fix here is to pass down the unsubstituted type info and
look at the spare bits of that when doing multi-payload enum layout.

Now that this works, we can remove a FIXME that was added to patch
around this.
  • Loading branch information
slavapestov committed Nov 15, 2015
1 parent 2f04cb1 commit 7237098
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 34 deletions.
32 changes: 14 additions & 18 deletions lib/IRGen/GenEnum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4043,14 +4043,15 @@ EnumImplStrategy *EnumImplStrategy::get(TypeConverter &TC,
// optimizations we perform to things that are reproducible by the runtime.
Type origArgType = elt->getArgumentType();
if (origArgType.isNull()) {
elementsWithNoPayload.push_back({elt, nullptr});
elementsWithNoPayload.push_back({elt, nullptr, nullptr});
continue;
}

// If the payload is indirect, we can use the NativeObject type metadata
// without recurring. The box won't affect loadability or fixed-ness.
if (elt->isIndirect() || theEnum->isIndirect()) {
elementsWithPayload.push_back({elt, &TC.getNativeObjectTypeInfo()});
auto *nativeTI = &TC.getNativeObjectTypeInfo();
elementsWithPayload.push_back({elt, nativeTI, nativeTI});
continue;
}

Expand All @@ -4066,15 +4067,15 @@ EnumImplStrategy *EnumImplStrategy::get(TypeConverter &TC,

auto loadableOrigArgTI = dyn_cast<LoadableTypeInfo>(origArgTI);
if (loadableOrigArgTI && loadableOrigArgTI->isKnownEmpty()) {
elementsWithNoPayload.push_back({elt, nullptr});
elementsWithNoPayload.push_back({elt, nullptr, nullptr});
} else {
// *Now* apply the substitutions and get the type info for the instance's
// payload type, since we know this case carries an apparent payload in
// the generic case.
SILType fieldTy = type.getEnumElementType(elt, *TC.IGM.SILMod);
auto *substArgTI = &TC.IGM.getTypeInfo(fieldTy);

elementsWithPayload.push_back({elt, substArgTI});
elementsWithPayload.push_back({elt, substArgTI, origArgTI});
if (!substArgTI->isFixedSize())
tik = Opaque;
else if (!substArgTI->isLoadable() && tik > Fixed)
Expand Down Expand Up @@ -4571,7 +4572,7 @@ MultiPayloadEnumImplStrategy::completeFixedLayout(TypeConverter &TC,
IsPOD_t isPOD = IsPOD;
IsBitwiseTakable_t isBT = IsBitwiseTakable;
for (auto &elt : ElementsWithPayload) {
auto &fixedPayloadTI = cast<FixedTypeInfo>(*elt.ti); // FIXME
auto &fixedPayloadTI = cast<FixedTypeInfo>(*elt.ti);
if (fixedPayloadTI.getFixedAlignment() > worstAlignment)
worstAlignment = fixedPayloadTI.getFixedAlignment();
if (!fixedPayloadTI.isPOD(ResilienceScope::Component))
Expand All @@ -4592,20 +4593,15 @@ MultiPayloadEnumImplStrategy::completeFixedLayout(TypeConverter &TC,
continue;
}

// As a hack, if the payload type is generic, don't use any spare bits
// from it, even if our concrete instance has them. We don't want varying
// spare bits between ObjC and Swift class references to introduce dynamic
// layout; that's a lot of overhead in generic code for little gain.
// There's a corresponding hack in TypeConverter::convertArchetypeType to
// give class archetypes no spare bits.
if (elt.decl->getInterfaceType()->hasTypeParameter()) {
FixedTypeInfo::applyFixedSpareBitsMask(CommonSpareBits,
SpareBitVector::getConstant(payloadBits, false));
continue;
}
// Otherwise, all unsubstituted payload types are fixed-size and
// we have no constraints on what spare bits we can use.

// Otherwise, we have no constraints on what spare bits we can use.
fixedPayloadTI.applyFixedSpareBitsMask(CommonSpareBits);
// We might still have a dependently typed payload though, namely a
// class-bound archetype. These do not have any spare bits because
// they can contain Obj-C tagged pointers. To handle this case
// correctly, we get spare bits from the unsubstituted type.
auto &fixedOrigTI = cast<FixedTypeInfo>(*elt.origTI);
fixedOrigTI.applyFixedSpareBitsMask(CommonSpareBits);
}

unsigned commonSpareBitCount = CommonSpareBits.count();
Expand Down
1 change: 1 addition & 0 deletions lib/IRGen/GenEnum.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ class EnumImplStrategy {
struct Element {
EnumElementDecl *decl;
const TypeInfo *ti;
const TypeInfo *origTI;
};

enum TypeInfoKind {
Expand Down
17 changes: 1 addition & 16 deletions lib/IRGen/GenType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1702,22 +1702,7 @@ static bool isIRTypeDependent(IRGenModule &IGM, NominalTypeDecl *decl) {
return IsIRTypeDependent(IGM).visitStructDecl(sd);
} else {
auto ed = cast<EnumDecl>(decl);

// HACK: there's some sort of logic in multi-payload enums that
// tries to assume that there are no spare bits in class-bounded
// archetypes. This logic is quite broken; if you instantiate a
// non-shared implementation for the enum, you get different
// results. This check prevents this from being a problem in
// common practice by pretending that a shared implementation is
// acceptable as long as the generic instance is known to be fixed
// in size.
if (IGM.classifyTypeSize(SILType::getPrimitiveObjectType(
ed->getDeclaredTypeInContext()->getCanonicalType()),
ResilienceScope::Component)
== ObjectSize::Fixed)
return false;

return IsIRTypeDependent(IGM).visitEnumDecl(cast<EnumDecl>(decl));
return IsIRTypeDependent(IGM).visitEnumDecl(ed);
}
}

Expand Down

0 comments on commit 7237098

Please sign in to comment.