[Mlir-commits] [mlir] 0e9b572 - [mlir] Fix TypeID for shared libraries built with -fvisibility=hidden.

[Stella Laurenzo]

Author: Stella Laurenzo
Date: 2020-10-09T12:12:34-07:00
New Revision: 0e9b572949ce00e5ca01bf7555abdda12052a213

URL: https://github.com/llvm/llvm-project/commit/0e9b572949ce00e5ca01bf7555abdda12052a213
DIFF: https://github.com/llvm/llvm-project/commit/0e9b572949ce00e5ca01bf7555abdda12052a213.diff

LOG: [mlir] Fix TypeID for shared libraries built with -fvisibility=hidden.

* Isolates the visibility controlled parts of its implementation to a detail namespace.
* Applies a struct level visibility attribute which applies to the static local within the get() functions.
* The prior version was not emitting a symbol for the static local "instance" fields when the user TU was compiled with -fvisibility=hidden.

Differential Revision: https://reviews.llvm.org/D89153

Added: 
    

Modified: 
    mlir/include/mlir/Support/TypeID.h

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Support/TypeID.h b/mlir/include/mlir/Support/TypeID.h
index 518ff39e8669..ef19182882c6 100644
--- a/mlir/include/mlir/Support/TypeID.h
+++ b/mlir/include/mlir/Support/TypeID.h
@@ -20,6 +20,10 @@
 
 namespace mlir {
 
+namespace detail {
+struct TypeIDExported;
+} // namespace detail
+
 /// This class provides an efficient unique identifier for a specific C++ type.
 /// This allows for a C++ type to be compared, hashed, and stored in an opaque
 /// context. This class is similar in some ways to std::type_index, but can be
@@ -62,19 +66,10 @@ class TypeID {
   bool operator!=(const TypeID &other) const { return !(*this == other); }
 
   /// Construct a type info object for the given type T.
-  /// TODO: This currently won't work when using DLLs as it requires properly
-  /// attaching dllimport and dllexport. Fix this when that information is
-  /// available within LLVM.
   template <typename T>
-  LLVM_EXTERNAL_VISIBILITY static TypeID get() {
-    static Storage instance;
-    return TypeID(&instance);
-  }
+  static TypeID get();
   template <template <typename> class Trait>
-  LLVM_EXTERNAL_VISIBILITY static TypeID get() {
-    static Storage instance;
-    return TypeID(&instance);
-  }
+  static TypeID get();
 
   /// Methods for supporting PointerLikeTypeTraits.
   const void *getAsOpaquePointer() const {
@@ -92,6 +87,8 @@ class TypeID {
 
   /// The storage of this type info object.
   const Storage *storage;
+
+  friend struct detail::TypeIDExported;
 };
 
 /// Enable hashing TypeID.
@@ -99,6 +96,43 @@ inline ::llvm::hash_code hash_value(TypeID id) {
   return llvm::hash_value(id.storage);
 }
 
+namespace detail {
+/// The static local instance of each get method must be emitted with
+/// "default" (public) visibility across all shared libraries, regardless of
+/// whether they are compiled with hidden visibility or not. The only reliable
+/// way to make this happen is to set the visibility attribute at the
+/// containing namespace/struct scope. We don't do this on the TypeID (internal
+/// API) class in order to reduce the scope of what gets exported with
+/// public visibility. Instead, the get() methods on TypeID trampoline
+/// through those on this detail class with specific visibility controls
+/// applied, making visibility declarations on the internal TypeID class not
+/// required (all visibility relevant pieces are here).
+/// TODO: This currently won't work when using DLLs as it requires properly
+/// attaching dllimport and dllexport. Fix this when that information is
+/// available within LLVM.
+struct LLVM_EXTERNAL_VISIBILITY TypeIDExported {
+  template <typename T>
+  static TypeID get() {
+    static TypeID::Storage instance;
+    return TypeID(&instance);
+  }
+  template <template <typename> class Trait>
+  static TypeID get() {
+    static TypeID::Storage instance;
+    return TypeID(&instance);
+  }
+};
+} // namespace detail
+
+template <typename T>
+TypeID TypeID::get() {
+  return detail::TypeIDExported::get<T>();
+}
+template <template <typename> class Trait>
+TypeID TypeID::get() {
+  return detail::TypeIDExported::get<Trait>();
+}
+
 } // end namespace mlir
 
 namespace llvm {