Skip to content

Commit 799cd91

Browse files
jckarterjckarter
committed
KeyPaths: Pointer-align pointer fields within key path patterns.
To get the full benefit of dyld3 on Darwin platforms, pointer relocations need to be pointer-aligned, which unfortunately requires growing some key path data structures a little bit. This does tidy up some code that had to hack around our lack of unaligned load/store operations on UnsafeRawPointer, at least. While we're here, we can also simplify the identification strategy for reabstracted stored properties; we only need the property index to identify, not the absolute offset. rdar://problem/32318829
1 parent c7fc620 commit 799cd91

File tree

9 files changed

+258
-294
lines changed

9 files changed

+258
-294
lines changed

‎include/swift/ABI/KeyPath.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,14 +173,13 @@ class KeyPathComponentHeader {
173173

174174
enum ComputedPropertyIDKind {
175175
Pointer,
176-
StoredPropertyOffset,
176+
StoredPropertyIndex,
177177
VTableOffset,
178178
};
179179

180180
constexpr static uint32_t
181181
getResolutionStrategy(ComputedPropertyIDKind idKind) {
182182
return idKind == Pointer ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
183-
: idKind == StoredPropertyOffset ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFieldOffset
184183
: (assert("no resolution strategy implemented" && false), 0);
185184
}
186185

@@ -196,7 +195,7 @@ class KeyPathComponentHeader {
196195
? _SwiftKeyPathComponentHeader_ComputedSettableFlag : 0)
197196
| (kind == SettableMutating
198197
? _SwiftKeyPathComponentHeader_ComputedMutatingFlag : 0)
199-
| (idKind == StoredPropertyOffset
198+
| (idKind == StoredPropertyIndex
200199
? _SwiftKeyPathComponentHeader_ComputedIDByStoredPropertyFlag : 0)
201200
| (idKind == VTableOffset
202201
? _SwiftKeyPathComponentHeader_ComputedIDByVTableOffsetFlag : 0)

‎lib/IRGen/ConstantBuilder.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,14 @@ class ConstantAggregateBuilderBase
107107
Size getNextOffsetFromGlobal() const {
108108
return Size(super::getNextOffsetFromGlobal().getQuantity());
109109
}
110+
111+
void addAlignmentPadding(Alignment align) {
112+
auto misalignment = getNextOffsetFromGlobal() % IGM().getPointerAlignment();
113+
if (misalignment != Size(0))
114+
add(llvm::ConstantAggregateZero::get(
115+
llvm::ArrayType::get(IGM().Int8Ty,
116+
align.getValue() - misalignment.getValue())));
117+
}
110118
};
111119

112120
class ConstantArrayBuilder

‎lib/IRGen/GenClass.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,13 @@ irgen::tryEmitConstantClassFragilePhysicalMemberOffset(IRGenModule &IGM,
506506
}
507507
}
508508

509+
unsigned
510+
irgen::getClassFieldIndex(IRGenModule &IGM, SILType baseType, VarDecl *field) {
511+
auto &baseClassTI = IGM.getTypeInfo(baseType).as<ClassTypeInfo>();
512+
auto &classLayout = baseClassTI.getClassLayout(IGM, baseType);
513+
return classLayout.getFieldIndex(field);
514+
}
515+
509516
FieldAccess
510517
irgen::getClassFieldAccess(IRGenModule &IGM, SILType baseType, VarDecl *field) {
511518
auto &baseClassTI = IGM.getTypeInfo(baseType).as<ClassTypeInfo>();

‎lib/IRGen/GenClass.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@ namespace irgen {
140140
SILType baseType,
141141
VarDecl *field);
142142

143+
unsigned getClassFieldIndex(IRGenModule &IGM,
144+
SILType baseType,
145+
VarDecl *field);
146+
143147
FieldAccess getClassFieldAccess(IRGenModule &IGM,
144148
SILType baseType,
145149
VarDecl *field);

‎lib/IRGen/GenKeyPath.cpp

Lines changed: 43 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ IRGenModule::getAddrOfKeyPathPattern(KeyPathPattern *pattern,
123123
fields.add(emitMetadataGenerator(rootTy));
124124
fields.add(emitMetadataGenerator(valueTy));
125125

126-
// TODO: 32-bit still has a padding word
126+
// TODO: 32-bit heap object header still has an extra word
127127
if (SizeTy == Int32Ty) {
128128
fields.addInt32(0);
129129
}
@@ -155,65 +155,31 @@ IRGenModule::getAddrOfKeyPathPattern(KeyPathPattern *pattern,
155155
// Leave a placeholder for the buffer header, since we need to know the full
156156
// buffer size to fill it in.
157157
auto headerPlaceholder = fields.addPlaceholderWithSize(Int32Ty);
158+
fields.addAlignmentPadding(getPointerAlignment());
158159

159160
auto startOfKeyPathBuffer = fields.getNextOffsetFromGlobal();
160161

161162
// Build out the components.
162163
auto baseTy = rootTy;
163164

164-
auto getPropertyOffsetOrIndirectOffset
165-
= [&](SILType loweredBaseTy, VarDecl *property)
166-
-> std::pair<llvm::Constant*, bool> {
167-
llvm::Constant *offset;
168-
bool isResolved;
169-
bool isStruct;
170-
if (loweredBaseTy.getStructOrBoundGenericStruct()) {
171-
offset = emitPhysicalStructMemberFixedOffset(*this,
172-
loweredBaseTy,
173-
property);
174-
isStruct = true;
175-
} else if (loweredBaseTy.getClassOrBoundGenericClass()) {
176-
offset = tryEmitConstantClassFragilePhysicalMemberOffset(*this,
177-
loweredBaseTy,
178-
property);
179-
isStruct = false;
180-
} else {
181-
llvm_unreachable("property of non-struct, non-class?!");
182-
}
183-
184-
// If the offset isn't fixed, try instead to get the field offset vector
185-
// offset for the field to look it up dynamically.
186-
isResolved = offset != nullptr;
187-
if (!isResolved) {
188-
if (isStruct) {
189-
offset = emitPhysicalStructMemberOffsetOfFieldOffset(
190-
*this, loweredBaseTy, property);
191-
assert(offset && "field is neither fixed-offset nor in offset vector");
192-
} else {
193-
auto offsetValue = getClassFieldOffset(*this,
194-
loweredBaseTy.getClassOrBoundGenericClass(),
195-
property);
196-
offset = llvm::ConstantInt::get(Int32Ty, offsetValue.getValue());
197-
}
198-
}
199-
200-
return {offset, isResolved};
201-
};
165+
auto assertPointerAlignment = [&]{
166+
assert(fields.getNextOffsetFromGlobal() % getPointerAlignment() == Size(0)
167+
&& "must be pointer-aligned here");
168+
};
202169

203170
for (unsigned i : indices(pattern->getComponents())) {
171+
assertPointerAlignment();
204172
SILType loweredBaseTy;
205173
Lowering::GenericContextScope scope(getSILTypes(),
206174
pattern->getGenericSignature());
207175
loweredBaseTy = getLoweredType(AbstractionPattern::getOpaque(),
208176
baseTy->getLValueOrInOutObjectType());
209-
210177
auto &component = pattern->getComponents()[i];
211178
switch (auto kind = component.getKind()) {
212179
case KeyPathPatternComponent::Kind::StoredProperty: {
213180
auto property = cast<VarDecl>(component.getStoredPropertyDecl());
214181

215182
auto addFixedOffset = [&](bool isStruct, llvm::Constant *offset) {
216-
offset = llvm::ConstantExpr::getTruncOrBitCast(offset, Int32Ty);
217183
if (auto offsetInt = dyn_cast_or_null<llvm::ConstantInt>(offset)) {
218184
auto offsetValue = offsetInt->getValue().getZExtValue();
219185
if (KeyPathComponentHeader::offsetCanBeInline(offsetValue)) {
@@ -228,7 +194,7 @@ IRGenModule::getAddrOfKeyPathPattern(KeyPathPattern *pattern,
228194
? KeyPathComponentHeader::forStructComponentWithOutOfLineOffset()
229195
: KeyPathComponentHeader::forClassComponentWithOutOfLineOffset();
230196
fields.addInt32(header.getData());
231-
fields.add(offset);
197+
fields.add(llvm::ConstantExpr::getTruncOrBitCast(offset, Int32Ty));
232198
};
233199

234200
// For a struct stored property, we may know the fixed offset of the field,
@@ -247,11 +213,10 @@ IRGenModule::getAddrOfKeyPathPattern(KeyPathPattern *pattern,
247213
// of the type metadata at instantiation time.
248214
auto fieldOffset = emitPhysicalStructMemberOffsetOfFieldOffset(
249215
*this, loweredBaseTy, property);
250-
fieldOffset = llvm::ConstantExpr::getTruncOrBitCast(fieldOffset,
251-
Int32Ty);
252216
auto header = KeyPathComponentHeader::forStructComponentWithUnresolvedFieldOffset();
253217
fields.addInt32(header.getData());
254-
fields.add(fieldOffset);
218+
fields.add(llvm::ConstantExpr::getTruncOrBitCast(fieldOffset,
219+
Int32Ty));
255220
break;
256221
}
257222

@@ -276,6 +241,7 @@ IRGenModule::getAddrOfKeyPathPattern(KeyPathPattern *pattern,
276241
auto header =
277242
KeyPathComponentHeader::forClassComponentWithUnresolvedIndirectOffset();
278243
fields.addInt32(header.getData());
244+
fields.addAlignmentPadding(getPointerAlignment());
279245
auto offsetVar = getAddrOfFieldOffset(property, /*indirect*/ false,
280246
NotForDefinition);
281247
fields.add(cast<llvm::Constant>(offsetVar.getAddress()));
@@ -358,17 +324,44 @@ IRGenModule::getAddrOfKeyPathPattern(KeyPathPattern *pattern,
358324
break;
359325
}
360326
case KeyPathPatternComponent::ComputedPropertyId::Property:
361-
idKind = KeyPathComponentHeader::StoredPropertyOffset;
362-
std::tie(idValue, idResolved) =
363-
getPropertyOffsetOrIndirectOffset(loweredBaseTy, id.getProperty());
364-
idValue = llvm::ConstantExpr::getZExtOrBitCast(idValue, SizeTy);
327+
// Use the index of the stored property within the aggregate to key
328+
// the property.
329+
auto property = id.getProperty();
330+
idKind = KeyPathComponentHeader::StoredPropertyIndex;
331+
if (baseTy->getStructOrBoundGenericStruct()) {
332+
idResolved = true;
333+
idValue = llvm::ConstantInt::get(SizeTy,
334+
getPhysicalStructFieldIndex(*this,
335+
SILType::getPrimitiveAddressType(baseTy), property));
336+
} else if (baseTy->getClassOrBoundGenericClass()) {
337+
// TODO: This field index would require runtime resolution with Swift
338+
// native class resilience. We never directly access ObjC-imported
339+
// ivars so we can disregard ObjC ivar resilience for this computation
340+
// and start counting at the Swift native root.
341+
switch (getClassFieldAccess(*this, loweredBaseTy, property)) {
342+
case FieldAccess::ConstantDirect:
343+
case FieldAccess::ConstantIndirect:
344+
case FieldAccess::NonConstantDirect:
345+
idResolved = true;
346+
idValue = llvm::ConstantInt::get(SizeTy,
347+
getClassFieldIndex(*this,
348+
SILType::getPrimitiveAddressType(baseTy), property));
349+
break;
350+
case FieldAccess::NonConstantIndirect:
351+
llvm_unreachable("not implemented");
352+
}
353+
354+
} else {
355+
llvm_unreachable("neither struct nor class");
356+
}
365357
break;
366358
}
367359

368360
auto header = KeyPathComponentHeader::forComputedProperty(componentKind,
369361
idKind, !isInstantiableInPlace, idResolved);
370362

371363
fields.addInt32(header.getData());
364+
fields.addAlignmentPadding(getPointerAlignment());
372365
fields.add(idValue);
373366

374367
if (isInstantiableInPlace) {
@@ -392,6 +385,7 @@ IRGenModule::getAddrOfKeyPathPattern(KeyPathPattern *pattern,
392385

393386
// For all but the last component, we pack in the type of the component.
394387
if (i + 1 != pattern->getComponents().size()) {
388+
fields.addAlignmentPadding(getPointerAlignment());
395389
fields.add(emitMetadataGenerator(component.getComponentType()));
396390
}
397391
baseTy = component.getComponentType();

‎stdlib/public/SwiftShims/KeyPath.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,6 @@ static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDResolutionM
9090
= 0x0000000FU;
9191
static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDResolved
9292
= 0x00000000U;
93-
static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFieldOffset
94-
= 0x00000001U;
9593
static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
9694
= 0x00000002U;
9795

0 commit comments

Comments
 (0)