Runtime: Compact the uniquing header for foreign metadata records.

include/swift/Remote/MetadataReader.h

@@ -894,9 +894,9 @@ class MetadataReader {
     case MetadataKind::ForeignClass: {
       auto namePtrAddress =
         Meta.getAddress() + TargetForeignClassMetadata<Runtime>::OffsetToName;
-      StoredPointer namePtr;
-      if (!Reader->readInteger(RemoteAddress(namePtrAddress), &namePtr) ||
-          namePtr == 0)
+
+      StoredPointer namePtr = resolveRelativeOffset<int32_t>(namePtrAddress);
+      if (namePtr == 0)
         return BuiltType();
       std::string name;
       if (!Reader->readString(RemoteAddress(namePtr), name))

include/swift/Runtime/Metadata.h

@@ -1567,86 +1567,152 @@ struct TargetForeignTypeMetadata : public TargetMetadata<Runtime> {
   using StoredPointer = typename Runtime::StoredPointer;
   using StoredSize = typename Runtime::StoredSize;
   using InitializationFunction_t =
-    void (*)(TargetForeignTypeMetadata<Runtime> *selectedMetadata);
+    void (TargetForeignTypeMetadata<Runtime> *selectedMetadata);
   using RuntimeMetadataPointer =
       ConstTargetMetadataPointer<Runtime, swift::TargetForeignTypeMetadata>;
 
+  /// An invasive cache for the runtime-uniqued lookup structure that is stored
+  /// in the header prefix of foreign metadata records.
+  ///
+  /// Prior to initialization, as emitted by the compiler, this contains the
+  /// initialization flags.
+  /// After initialization, it holds a pointer to the actual, runtime-uniqued
+  /// metadata for this type.
+  struct CacheValue {
+    StoredSize Value;
+
+    /// Work around a bug in libstdc++'s std::atomic that requires the type to
+    /// be default-constructible.
+    CacheValue() = default;
+
+    explicit CacheValue(RuntimeMetadataPointer p)
+      : Value(reinterpret_cast<StoredSize>(p))
+    {}
+
+    /// Various flags. The largest flag bit should be less than 4096 so that
+    /// a flag set is distinguishable from a valid pointer.
+    enum : StoredSize {
+      /// This metadata has an initialization callback function.  If
+      /// this flag is not set, the metadata object needn't actually
+      /// have a InitializationFunction field, and that field will be
+      /// undefined.
+      HasInitializationFunction = 0x1,
+
+      LargestFlagMask = 0xFFF,
+    };    
+
+    /// True if the metadata record associated with this cache has not been
+    /// initialized, so contains a flag set describing parameters to the
+    /// initialization operation. isFlags() == !isInitialized()
+    bool isFlags() const {
+      return Value <= LargestFlagMask;
+    }
+    /// True if the metadata record associated with this cache has an
+    /// initialization function which must be run if it is picked as the
+    /// canonical metadata record for its key.
+    ///
+    /// Undefined if !isFlags().
+    bool hasInitializationFunction() const {
+      assert(isFlags());
+      return Value & HasInitializationFunction;
+    }
+
+    /// True if the metadata record associated with this cache has been
+    /// initialized, so the cache contains an absolute pointer to the
+    /// canonical metadata record for its key. isInitialized() == !isFlags()
+    bool isInitialized() const {
+      return !isFlags();
+    }
+
+    /// Gets the cached pointer to the unique canonical metadata record for
+    /// this metadata record's key.
+    ///
+    /// Undefined if !isInitialized().
+    RuntimeMetadataPointer getCachedUniqueMetadata() const {
+      assert(isInitialized());
+      return RuntimeMetadataPointer(Value);
+    }
+  };
+
+
   /// Foreign type metadata may have extra header fields depending on
   /// the flags.
   struct HeaderPrefix {
     /// An optional callback performed when a particular metadata object
     /// is chosen as the unique structure.
+    ///
     /// If there is no initialization function, this metadata record can be
     /// assumed to be immutable (except for the \c Unique invasive cache
-    /// field).
-    InitializationFunction_t InitializationFunction;
+    /// field). The field is not present unless the HasInitializationFunction
+    /// flag is set.
+    RelativeDirectPointer<InitializationFunction_t> InitializationFunction;
 
-    /// The Swift-mangled name of the type. This is the uniquing key for the
-    /// type.
-    TargetPointer<Runtime, const char> Name;
-
-    /// A pointer to the actual, runtime-uniqued metadata for this
-    /// type.  This is essentially an invasive cache for the lookup
-    /// structure.
-    mutable std::atomic<RuntimeMetadataPointer> Unique;
+    /// The uniquing key for the metadata record. Metadata records with the
+    /// same Name string are considered equivalent by the runtime, and the
+    /// runtime will pick one to be canonical.
+    RelativeDirectPointer<const char> Name;
 
-    /// Various flags.
-    enum : StoredSize {
-      /// This metadata has an initialization callback function.  If
-      /// this flag is not set, the metadata object needn't actually
-      /// have a InitializationFunction field.
-      HasInitializationFunction = 0x1,
-    } Flags;
+    mutable std::atomic<CacheValue> Cache;
   };
 
   struct HeaderType : HeaderPrefix, TypeMetadataHeader {};
 
-  static constexpr int OffsetToName =
-    (int) offsetof(HeaderType, Name) - (int) sizeof(HeaderType);
+  static constexpr int32_t OffsetToName =
+    (int32_t) offsetof(HeaderType, Name) - (int32_t) sizeof(HeaderType);
 
   TargetPointer<Runtime, const char> getName() const {
     return reinterpret_cast<TargetPointer<Runtime, const char>>(
-      asFullMetadata(this)->Name);
+      asFullMetadata(this)->Name.get());
   }
 
-  RuntimeMetadataPointer getCachedUniqueMetadata() const {
-#if __alpha__
-    // TODO: This can be a relaxed-order load if there is no initialization
-    // function. On platforms we care about, consume is no more expensive than
-    // relaxed, so there's no reason to branch here (and LLVM isn't smart
-    // enough to eliminate it when it's not needed).
-    if (!hasInitializationFunction())
-      return asFullMetadata(this)->Unique.load(std::memory_order_relaxed);
-#endif
-    return asFullMetadata(this)->Unique.load(SWIFT_MEMORY_ORDER_CONSUME);
+  CacheValue getCacheValue() const {
+    /// NB: This can be a relaxed-order load if there is no initialization
+    /// function. On platforms Swift currently targets, consume is no more
+    /// expensive than relaxed, so there's no reason to branch here (and LLVM
+    /// isn't smart enough to eliminate it when it's not needed).
+    ///
+    /// A port to a platform where relaxed is significantly less expensive than
+    /// consume (historically, Alpha) would probably want to preserve the
+    /// 'hasInitializationFunction' bit in its own word to be able to avoid
+    /// the consuming load when not needed.
+    return asFullMetadata(this)->Cache
+      .load(SWIFT_MEMORY_ORDER_CONSUME);
   }
 
   void setCachedUniqueMetadata(RuntimeMetadataPointer unique) const {
-    assert((static_cast<RuntimeMetadataPointer>(asFullMetadata(this)->Unique) ==
-                nullptr ||
-            asFullMetadata(this)->Unique == unique) &&
-           "already set unique metadata");
+    auto cache = getCacheValue();
+
+    // If the cache was already set to a pointer, we're done. We ought to
+    // converge on a single unique pointer.
+    if (cache.isInitialized()) {
+      assert(cache.getCachedUniqueMetadata() == unique
+             && "already set unique metadata to something else");
+      return;
+    }
+
+    auto newCache = CacheValue(unique);
 
     // If there is no initialization function, this can be a relaxed store.
-    if (!hasInitializationFunction())
-      asFullMetadata(this)->Unique.store(unique, std::memory_order_relaxed);
+    if (cache.hasInitializationFunction())
+      asFullMetadata(this)->Cache.store(newCache, std::memory_order_relaxed);
 
     // Otherwise, we need a release store to publish the result of
-    // initialization
+    // initialization.
     else
-      asFullMetadata(this)->Unique.store(unique, std::memory_order_release);
+      asFullMetadata(this)->Cache.store(newCache, std::memory_order_release);
   }
 
-  StoredSize getFlags() const {
-    return asFullMetadata(this)->Flags;
-  }
-
-  bool hasInitializationFunction() const {
-    return getFlags() & HeaderPrefix::HasInitializationFunction;
-  }
+  /// Return the initialization function for this metadata record.
+  ///
+  /// As a prerequisite, the metadata record must not have been initialized yet,
+  /// and must have an initialization function to begin with, otherwise the
+  /// result is undefined.
+  InitializationFunction_t *getInitializationFunction() const {
+#ifndef NDEBUG
+    auto cache = getCacheValue();
+    assert(cache.hasInitializationFunction());
+#endif
 
-  InitializationFunction_t getInitializationFunction() const {
-    assert(hasInitializationFunction());
     return asFullMetadata(this)->InitializationFunction;
   }
 };

lib/IRGen/GenMeta.cpp

@@ -4923,8 +4923,9 @@ namespace {
     void layout() {
       if (asImpl().requiresInitializationFunction())
         asImpl().addInitializationFunction();
+      else
+        asImpl().addPaddingForInitializationFunction();
       asImpl().addForeignName();
-      asImpl().addUniquePointer();
       asImpl().addForeignFlags();
       super::layout();
     }
@@ -4937,11 +4938,8 @@ namespace {
 
     void addForeignName() {
       CanType targetType = asImpl().getTargetType();
-      B.add(getMangledTypeName(IGM, targetType));
-    }
-
-    void addUniquePointer() {
-      B.addNullPointer(IGM.TypeMetadataPtrTy);
+      B.addRelativeAddress(getMangledTypeName(IGM, targetType,
+                                              /*relative addressed?*/ true));
     }
 
     void addInitializationFunction() {
@@ -4967,7 +4965,23 @@ namespace {
 
       IGF.Builder.CreateRetVoid();
 
-      B.add(fn);
+      B.addRelativeAddress(fn);
+    }
+
+    void addPaddingForInitializationFunction() {
+      // The initialization function field is placed at the least offset of the
+      // record so it can be omitted when not needed. However, the metadata
+      // record is still pointer-aligned, so on 64 bit platforms we need to
+      // occupy the space to keep the rest of the record with the right layout.
+      switch (IGM.getPointerSize().getValue()) {
+      case 4:
+        break;
+      case 8:
+        B.addInt32(0);
+        break;
+      default:
+        llvm_unreachable("unsupported word size");
+      }
     }
 
     void noteAddressPoint() {

stdlib/public/runtime/Metadata.cpp

@@ -2464,14 +2464,15 @@ static Lazy<ForeignTypeState> ForeignTypes;
 const ForeignTypeMetadata *
 swift::swift_getForeignTypeMetadata(ForeignTypeMetadata *nonUnique) {
   // Fast path: check the invasive cache.
-  if (auto unique = nonUnique->getCachedUniqueMetadata()) {
-    return unique;
+  auto cache = nonUnique->getCacheValue();
+  if (cache.isInitialized()) {
+    return cache.getCachedUniqueMetadata();
   }
 
   // Okay, check the global map.
   auto &foreignTypes = ForeignTypes.get();
   GlobalString key(nonUnique->getName());
-  bool hasInit = nonUnique->hasInitializationFunction();
+  bool hasInit = cache.hasInitializationFunction();
 
   const ForeignTypeMetadata *uniqueMetadata;
   bool inserted;

test/IRGen/cf.sil

@@ -7,11 +7,18 @@
 // CHECK: [[REFRIGERATOR:%TSo14CCRefrigeratorC]] = type
 // CHECK: [[OBJC:%objc_object]] = type
 
-// CHECK: [[REFRIGERATOR_NAME:@.*]] = private unnamed_addr constant [20 x i8] c"So14CCRefrigeratorC\00"
+// CHECK: [[REFRIGERATOR_NAME:@.*]] = private constant [20 x i8] c"So14CCRefrigeratorC\00"
 
-// CHECK-32: @_T0So14CCRefrigeratorCN = linkonce_odr hidden global <{ {{.*}} }> <{ i8* getelementptr inbounds ([20 x i8], [20 x i8]* [[REFRIGERATOR_NAME]], i32 0, i32 0), [[TYPE]]* null, i32 0, i8** @_T0BOWV, i32 16, [[TYPE]]* null, i8* null, i8* null, i8* null }>
+// CHECK-32: @_T0So14CCRefrigeratorCN = linkonce_odr hidden global <{ {{.*}} }> <{
+// CHECK-32-SAME: [[REFRIGERATOR_NAME]] to i32
+// CHECK-32-SAME: i32 0,
+// CHECK-32-SAME: i8** @_T0BOWV, i32 16, [[TYPE]]* null, i8* null, i8* null, i8* null }>
 
-// CHECK-64: @_T0So14CCRefrigeratorCN = linkonce_odr hidden global <{ {{.*}} }> <{ i8* getelementptr inbounds ([20 x i8], [20 x i8]* [[REFRIGERATOR_NAME]], i64 0, i64 0), [[TYPE]]* null, i64 0, i8** @_T0BOWV, i64 16, [[TYPE]]* null, i8* null, i8* null, i8* null }>
+// CHECK-64: @_T0So14CCRefrigeratorCN = linkonce_odr hidden global <{ {{.*}} }> <{
+// CHECK-64-SAME: i32 0,
+// CHECK-64-SAME: i32 trunc {{.*}} [[REFRIGERATOR_NAME]] to i64
+// CHECK-64-SAME: i64 0,
+// CHECK-64-SAME: i8** @_T0BOWV, i64 16, [[TYPE]]* null, i8* null, i8* null, i8* null }>
 
 sil_stage canonical
 
@@ -35,5 +42,5 @@ bb0(%0 : $CCRefrigerator):
 // CHECK-NEXT: ret void
 
 // CHECK:    define linkonce_odr hidden [[TYPE]]* @_T0So14CCRefrigeratorCMa()
-// CHECK-32:    call [[TYPE]]* @swift_getForeignTypeMetadata([[TYPE]]* bitcast (i8* getelementptr inbounds (i8, i8* bitcast ({{.*}}* @_T0So14CCRefrigeratorCN to i8*), i32 16) to [[TYPE]]*))
-// CHECK-64:    call [[TYPE]]* @swift_getForeignTypeMetadata([[TYPE]]* bitcast (i8* getelementptr inbounds (i8, i8* bitcast ({{.*}}* @_T0So14CCRefrigeratorCN to i8*), i64 32) to [[TYPE]]*))
+// CHECK-32:    call [[TYPE]]* @swift_getForeignTypeMetadata([[TYPE]]* bitcast (i8* getelementptr inbounds (i8, i8* bitcast ({{.*}}* @_T0So14CCRefrigeratorCN to i8*), i32 12) to [[TYPE]]*))
+// CHECK-64:    call [[TYPE]]* @swift_getForeignTypeMetadata([[TYPE]]* bitcast (i8* getelementptr inbounds (i8, i8* bitcast ({{.*}}* @_T0So14CCRefrigeratorCN to i8*), i64 24) to [[TYPE]]*))

test/IRGen/foreign_types.sil

@@ -4,9 +4,8 @@ sil_stage canonical
 import c_layout
 
 // CHECK-LABEL: @_T0SC14HasNestedUnionV18__Unnamed_struct_sVN = linkonce_odr hidden global
-// CHECK-SAME:  i8* getelementptr inbounds
-// CHECK-SAME:  %swift.type* null,
-// CHECK-SAME:  [[INT:i[0-9]+]] 0,
+// CHECK-SAME:  [[INT:i[0-9]+]] sub ([[INT]] ptrtoint
+// CHECK-SAME:  [[INT]] 0,
 // CHECK-SAME:  @_T0SC14HasNestedUnionV18__Unnamed_struct_sVWV
 // CHECK-SAME:  [[INT]] 1,
 // CHECK-SAME:  @_T0SC14HasNestedUnionV18__Unnamed_struct_sVMn