Merge pull request swiftlang#20206 from slavapestov/fix-field-offset-global-update

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

Author: slavapestov

lib/IRGen/GenClass.cpp

@@ -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);
 

lib/IRGen/GenMeta.cpp

@@ -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;
 
@@ -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;
     }
@@ -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.
@@ -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();
 

lib/IRGen/GenMeta.h

@@ -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.

test/IRGen/completely_fragile_class_layout.sil

@@ -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:
@@ -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
@@ -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