Skip to content

IRGen: Only mark field offset globals as constant if they won't be updated at runtime #20206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/IRGen/GenClass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -993,17 +993,17 @@ void IRGenModule::emitClassDecl(ClassDecl *D) {
SILType selfType = getSelfType(D);
auto &classTI = getTypeInfo(selfType).as<ClassTypeInfo>();

// FIXME: For now, always use the fragile layout when emitting metadata.
// Use the fragile layout when emitting metadata.
auto &fragileLayout =
classTI.getClassLayout(*this, selfType, /*forBackwardDeployment=*/true);

// ... but still compute the resilient layout for better test coverage.
// The resilient layout tells us what parts of the metadata can be
// updated at runtime by the Objective-C metadata update callback.
auto &resilientLayout =
classTI.getClassLayout(*this, selfType, /*forBackwardDeployment=*/false);
(void) resilientLayout;

// Emit the class metadata.
emitClassMetadata(*this, D, fragileLayout);
emitClassMetadata(*this, D, fragileLayout, resilientLayout);

IRGen.addClassForEagerInitialization(D);

Expand Down
33 changes: 23 additions & 10 deletions lib/IRGen/GenMeta.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2293,9 +2293,10 @@ getAddrOfDestructorFunction(IRGenModule &IGM, ClassDecl *classDecl) {

static void emitFieldOffsetGlobals(IRGenModule &IGM,
ClassDecl *classDecl,
const ClassLayout &classLayout) {
const ClassLayout &fragileLayout,
const ClassLayout &resilientLayout) {
for (auto prop : classDecl->getStoredProperties()) {
auto fieldInfo = classLayout.getFieldAccessAndElement(prop);
auto fieldInfo = fragileLayout.getFieldAccessAndElement(prop);
auto access = fieldInfo.first;
auto element = fieldInfo.second;

Expand Down Expand Up @@ -2323,8 +2324,19 @@ static void emitFieldOffsetGlobals(IRGenModule &IGM,
auto offsetVar = cast<llvm::GlobalVariable>(offsetAddr.getAddress());
offsetVar->setInitializer(fieldOffsetOrZero);

// If we know the offset won't change, make it a constant.
offsetVar->setConstant(access == FieldAccess::ConstantDirect);
// If the offset is constant in the resilient layout, it will not change
// at runtime, and the global can be true const.
//
// If it is constant in the fragile layout only, newer Objective-C
// runtimes will still update them in place, so make sure to check the
// correct layout.
auto resilientInfo = resilientLayout.getFieldAccessAndElement(prop);
if (resilientInfo.first == FieldAccess::ConstantDirect) {
// If it is constant in the resilient layout, it should be constant in
// the fragile layout also.
assert(access == FieldAccess::ConstantDirect);
offsetVar->setConstant(true);
}

break;
}
Expand Down Expand Up @@ -3015,11 +3027,12 @@ static void emitObjCClassSymbol(IRGenModule &IGM,

/// Emit the type metadata or metadata template for a class.
void irgen::emitClassMetadata(IRGenModule &IGM, ClassDecl *classDecl,
const ClassLayout &fieldLayout) {
const ClassLayout &fragileLayout,
const ClassLayout &resilientLayout) {
assert(!classDecl->isForeign());
PrettyStackTraceDecl stackTraceRAII("emitting metadata for", classDecl);

emitFieldOffsetGlobals(IGM, classDecl, fieldLayout);
emitFieldOffsetGlobals(IGM, classDecl, fragileLayout, resilientLayout);

// Set up a dummy global to stand in for the metadata object while we produce
// relative references.
Expand All @@ -3034,28 +3047,28 @@ void irgen::emitClassMetadata(IRGenModule &IGM, ClassDecl *classDecl,
bool canBeConstant;
if (classDecl->isGenericContext()) {
GenericClassMetadataBuilder builder(IGM, classDecl, init,
fieldLayout);
fragileLayout);
builder.layout();
canBeConstant = true;

builder.createMetadataAccessFunction();
} else if (doesClassMetadataRequireRelocation(IGM, classDecl)) {
ResilientClassMetadataBuilder builder(IGM, classDecl, init,
fieldLayout);
fragileLayout);
builder.layout();
canBeConstant = true;

builder.createMetadataAccessFunction();
} else if (doesClassMetadataRequireInitialization(IGM, classDecl)) {
SingletonClassMetadataBuilder builder(IGM, classDecl, init,
fieldLayout);
fragileLayout);
builder.layout();
canBeConstant = builder.canBeConstant();

builder.createMetadataAccessFunction();
} else {
FixedClassMetadataBuilder builder(IGM, classDecl, init,
fieldLayout);
fragileLayout);
builder.layout();
canBeConstant = builder.canBeConstant();

Expand Down
3 changes: 2 additions & 1 deletion lib/IRGen/GenMeta.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ namespace irgen {

/// Emit the metadata associated with the given class declaration.
void emitClassMetadata(IRGenModule &IGM, ClassDecl *theClass,
const ClassLayout &fieldLayout);
const ClassLayout &fragileLayout,
const ClassLayout &resilientLayout);

/// Emit the constant initializer of the type metadata candidate for
/// the given foreign class declaration.
Expand Down
12 changes: 6 additions & 6 deletions test/IRGen/completely_fragile_class_layout.sil
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ sil_vtable SubclassOfClassWithResilientField {}

// Field offsets are statically known:
// CHECK-DAG: @"$s31completely_fragile_class_layout23ClassWithResilientFieldC5firstSivpWvd" = hidden constant i64 16, align 8
// CHECK-DAG: @"$s31completely_fragile_class_layout23ClassWithResilientFieldC6second16resilient_struct4SizeVvpWvd" = hidden constant i64 24, align 8
// CHECK-DAG: @"$s31completely_fragile_class_layout23ClassWithResilientFieldC5thirdSivpWvd" = hidden constant i64 40, align 8
// CHECK-DAG: @"$s31completely_fragile_class_layout23ClassWithResilientFieldC6second16resilient_struct4SizeVvpWvd" = hidden global i64 24, align 8
// CHECK-DAG: @"$s31completely_fragile_class_layout23ClassWithResilientFieldC5thirdSivpWvd" = hidden global i64 40, align 8


// RO-data for ClassWithResilientField:
Expand Down Expand Up @@ -91,8 +91,8 @@ public class ClassWithResilientEnum {
sil_vtable ClassWithResilientEnum {}

// Field offsets are statically known:
// CHECK-LABEL: @"$s31completely_fragile_class_layout22ClassWithResilientEnumC5firstAA0hfG7PayloadOvpWvd" = hidden constant i64 16, align 8
// CHECK-LABEL: @"$s31completely_fragile_class_layout22ClassWithResilientEnumC6seconds4Int8VvpWvd" = hidden constant i64 25, align 8
// CHECK-LABEL: @"$s31completely_fragile_class_layout22ClassWithResilientEnumC5firstAA0hfG7PayloadOvpWvd" = hidden global i64 16, align 8
// CHECK-LABEL: @"$s31completely_fragile_class_layout22ClassWithResilientEnumC6seconds4Int8VvpWvd" = hidden global i64 25, align 8


// Make sure extra inhabitants work when reading a legacy layout -- the
Expand All @@ -105,8 +105,8 @@ public class ClassWithResilientRef {
sil_vtable ClassWithResilientRef {}

// Field offsets are statically known:
// CHECK-LABEL: @"$s31completely_fragile_class_layout21ClassWithResilientRefC5first16resilient_struct0gH0VSgvpWvd" = hidden constant i64 16, align 8
// CHECK-LABEL: @"$s31completely_fragile_class_layout21ClassWithResilientRefC6secondSivpWvd" = hidden constant i64 24, align 8
// CHECK-LABEL: @"$s31completely_fragile_class_layout21ClassWithResilientRefC5first16resilient_struct0gH0VSgvpWvd" = hidden global i64 16, align 8
// CHECK-LABEL: @"$s31completely_fragile_class_layout21ClassWithResilientRefC6secondSivpWvd" = hidden global i64 24, align 8


// When allocating a class with resiliently-sized fields, we must still load
Expand Down