Emit strong definition for TypeID storage in Op definition (WIP)

[lattner]

mlir/include/mlir/Dialect/Affine/IR/AffineOps.td

24 ↗

(On Diff #358301)

FWIW, these namespace cleanups to Affine dialect are "obvious" and can be landed separately

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

2602

I'm thrilled to see this.

Given this is emitted into a C++ file, you could emit these as true global variables, instead of as lazily initialized things within the function. Does clang optimize the lazily initialized one away when it has no initializer? If clang handles this correctly then your approach is better, but if not, then hand holding it seems best given this is autogenerated code.

This would look something like this:

os << "    static TypeID::Storage <name_mangled>_typeid_instance;\n";
os << "template<>\n";
os << "mlir::TypeID mlir::detail::TypeIDExported::get<"
   << op.getCppNamespace() << "::" << op.getCppClassName() << ">() {\n";
os << "    return TypeID(&<name_mangled>_typeid_instance);\n";
os << "}\n";

[mehdi_amini]

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

2602

Does it make a difference if the static is "file scoped" or "function scoped" when the type is trivial? (we're only interested in the address, not the value).

I was expecting that this only matters if a constructor has to be invoked.

Here is the diff between the two cases as I understand it: https://godbolt.org/z/Msf1PT43j

[stellaraccident]

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

2602

I don't think it makes a big difference. File-level static is marginally nicer should some poor downstream find themselves in the position of needing to filter symbol lists and such.

It should probably also be emitted as visibility default.

[lattner]

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

2602

Looks identical to me! Please keep it as function scoped then. Thanks again!

[rriddle]

mlir/include/mlir/Dialect/Affine/IR/AffineOps.h

375 ↗

(On Diff #358301)

Unrelated change?

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

2587

Can we have a macro or something in TypeID.h that does this? That would make it easier to generate, and would also make it easier if we transition to making this mandatory.

[mehdi_amini]

mlir/include/mlir/Dialect/Affine/IR/AffineOps.h

375 ↗

(On Diff #358301)

Not really: we can't do the explicit specialization emitted by TableGen otherwise: we have to rely on the "generated files are included at the top level namespace" I think.

That said, yes I'll land the affine dialect change in a separate commit, I left them here to show that this change in ODS generation can break client who haven't switched to the new include model (i.e. not fully specified the namespace for their dialect).

[mehdi_amini]

Address comments and add support for Types and Attributes

[mehdi_amini]

Add DialectGen support as well

mlir/include/mlir/Support/TypeID.h

 }
 template <template <typename> class Trait>
 TypeID TypeID::get() {
   return detail::TypeIDExported::get<Trait>();
 }
 
 } // end namespace mlir
 
+// Declare/define an explicit specialization for TypeID: this forces the
+// compiler to emit a strong definition for a class and controls which
+// translation unit and shared object will actually have it.
+// This can be useful to turn to a link-time failure what would be in other
+// circumstances a hard-to-catch runtime bug when a TypeID is hidden in two
+// different shared libraries and instances of the same class only gets the same
+// TypeID inside a given DSO.
+#define DECLARE_EXPLICIT_TYPE_ID(CLASS_NAME)                                   \
+  template <>                                                                  \
+  LLVM_EXTERNAL_VISIBILITY mlir::TypeID                                        \
+  mlir::detail::TypeIDExported::get<CLASS_NAME>();
+#define DEFINE_EXPLICIT_TYPE_ID(CLASS_NAME)                                    \
+  template <>                                                                  \
+  LLVM_EXTERNAL_VISIBILITY mlir::TypeID                                        \
+  mlir::detail::TypeIDExported::get<CLASS_NAME>() {                            \
+    static mlir::TypeID::Storage instance;                                     \
+    return mlir::TypeID(&instance);                                            \
+  }
+
 namespace llvm {
 template <> struct DenseMapInfo<mlir::TypeID> {
   static mlir::TypeID getEmptyKey() {
     void *pointer = llvm::DenseMapInfo<void *>::getEmptyKey();
     return mlir::TypeID::getFromOpaquePointer(pointer);
   }
   static mlir::TypeID getTombstoneKey() {
     void *pointer = llvm::DenseMapInfo<void *>::getTombstoneKey();

mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp

bool DefGenerator::emitDecls(StringRef selectedDialect) {
 
   // Output the common "header".
   os << typeDefDeclHeader;
 
   SmallVector<AttrOrTypeDef, 16> defs;
   collectAllDefs(selectedDialect, defRecords, defs);
   if (defs.empty())
     return false;
-
+  {
     NamespaceEmitter nsEmitter(os, defs.front().getDialect());
 
     // Declare all the def classes first (in case they reference each other).
     for (const AttrOrTypeDef &def : defs)
       os << "  class " << def.getCppClassName() << ";\n";
 
     // Emit the declarations.
     for (const AttrOrTypeDef &def : defs)
       emitDefDecl(def);
+  }
+  // Emit the TypeID explicit specializations to have a single definition for
+  // each of these.
+  for (const AttrOrTypeDef &def : defs)
+    if (!def.getDialect().getCppNamespace().empty())
+      os << "DECLARE_EXPLICIT_TYPE_ID(" << def.getDialect().getCppNamespace()
+         << "::" << def.getCppClassName() << ")\n";
+
   return false;
 }
 
 //===----------------------------------------------------------------------===//
 // GEN: Def List
 //===----------------------------------------------------------------------===//
 
 void DefGenerator::emitTypeDefList(ArrayRef<AttrOrTypeDef> defs) {
bool DefGenerator::emitDefs(StringRef selectedDialect) {
   SmallVector<AttrOrTypeDef, 16> defs;
   collectAllDefs(selectedDialect, defRecords, defs);
   if (defs.empty())
     return false;
   emitTypeDefList(defs);
 
   IfDefScope scope("GET_" + defTypePrefix.upper() + "DEF_CLASSES", os);
   emitParsePrintDispatch(defs);
-  for (const AttrOrTypeDef &def : defs)
+  for (const AttrOrTypeDef &def : defs) {
     emitDefDef(def);
+    // Emit the TypeID explicit specializations to have a single symbol def.
+    if (!def.getDialect().getCppNamespace().empty())
+      os << "DEFINE_EXPLICIT_TYPE_ID(" << def.getDialect().getCppNamespace()
+         << "::" << def.getCppClassName() << ")\n";
+  }
 
   return false;
 }
 
 //===----------------------------------------------------------------------===//
 // GEN: Registration hooks
 //===----------------------------------------------------------------------===//
 

mlir/tools/mlir-tblgen/DialectGen.cpp

static void emitDialectDecl(Dialect &dialect,
   {
     llvm::raw_string_ostream dialectsOs(dependentDialectRegistrations);
     for (StringRef dependentDialect : dialect.getDependentDialects())
       dialectsOs << llvm::formatv(dialectRegistrationTemplate,
                                   dependentDialect);
   }
 
   // Emit all nested namespaces.
+  {
     NamespaceEmitter nsEmitter(os, dialect);
 
     // Emit the start of the decl.
     std::string cppName = dialect.getCppClassName();
     os << llvm::formatv(dialectDeclBeginStr, cppName, dialect.getName(),
                         dependentDialectRegistrations);
 
     // Check for any attributes/types registered to this dialect.  If there are,
     // add the hooks for parsing/printing.
     if (!dialectAttrs.empty())
       os << attrParserDecl;
     if (!dialectTypes.empty())
       os << typeParserDecl;
 
     // Add the decls for the various features of the dialect.
     if (dialect.hasCanonicalizer())
       os << canonicalizerDecl;
     if (dialect.hasConstantMaterializer())
       os << constantMaterializerDecl;
     if (dialect.hasOperationAttrVerify())
       os << opAttrVerifierDecl;
     if (dialect.hasRegionArgAttrVerify())
       os << regionArgAttrVerifierDecl;
     if (dialect.hasRegionResultAttrVerify())
       os << regionResultAttrVerifierDecl;
     if (dialect.hasOperationInterfaceFallback())
       os << operationInterfaceFallbackDecl;
-  if (llvm::Optional<StringRef> extraDecl = dialect.getExtraClassDeclaration())
+    if (llvm::Optional<StringRef> extraDecl =
+            dialect.getExtraClassDeclaration())
       os << *extraDecl;
 
     // End the dialect decl.
     os << "};\n";
   }
+  if (!dialect.getCppNamespace().empty())
+    os << "DECLARE_EXPLICIT_TYPE_ID(" << dialect.getCppNamespace()
+       << "::" << dialect.getCppClassName() << ")\n";
+}
 
 static bool emitDialectDecls(const llvm::RecordKeeper &recordKeeper,
                              raw_ostream &os) {
   emitSourceFileHeader("Dialect Declarations", os);
 
   auto dialectDefs = recordKeeper.getAllDerivedDefinitions("Dialect");
   if (dialectDefs.empty())
     return false;

 ///
 /// {0}: The name of the dialect class.
 static const char *const dialectDestructorStr = R"(
 {0}::~{0}() = default;
 
 )";
 
 static void emitDialectDef(Dialect &dialect, raw_ostream &os) {
+  // Emit the TypeID explicit specializations to have a single symbol def.
+  if (!dialect.getCppNamespace().empty())
+    os << "DEFINE_EXPLICIT_TYPE_ID(" << dialect.getCppNamespace()
+       << "::" << dialect.getCppClassName() << ")\n";
+
   // Emit all nested namespaces.
   NamespaceEmitter nsEmitter(os, dialect);
 
   if (!dialect.hasNonDefaultDestructor())
     os << llvm::formatv(dialectDestructorStr, dialect.getCppClassName());
 }
 
 static bool emitDialectDefs(const llvm::RecordKeeper &recordKeeper,

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

OpEmitter::OpEmitter(const Operator &op,
   genVerifier();
   genCanonicalizerDecls();
   genFolderDecls();
   genTypeInterfaceMethods();
   genOpInterfaceMethods();
   generateOpFormat(op, opClass);
   genSideEffectInterfaceMethods();
 }
-
 void OpEmitter::emitDecl(
     const Operator &op, raw_ostream &os,
     const StaticVerifierFunctionEmitter &staticVerifierEmitter) {
   OpEmitter(op, staticVerifierEmitter).emitDecl(os);
 }
 
 void OpEmitter::emitDef(
     const Operator &op, raw_ostream &os,
static void emitOpClasses(const RecordKeeper &recordKeeper,
   if (defs.empty())
     return;
 
   // Generate all of the locally instantiated methods first.
   StaticVerifierFunctionEmitter staticVerifierEmitter(recordKeeper, defs, os,
                                                       emitDecl);
   for (auto *def : defs) {
     Operator op(*def);
-    NamespaceEmitter emitter(os, op.getCppNamespace());
     if (emitDecl) {
-      os << formatv(opCommentHeader, op.getQualCppClassName(), "declarations");
+      {
+        NamespaceEmitter emitter(os, op.getCppNamespace());
+        os << formatv(opCommentHeader, op.getQualCppClassName(),
+                      "declarations");
         OpOperandAdaptorEmitter::emitDecl(op, os);
         OpEmitter::emitDecl(op, os, staticVerifierEmitter);
+      }
+      // Emit the TypeID explicit specialization to have a single definition.
+      if (!op.getCppNamespace().empty())
+        os << "DECLARE_EXPLICIT_TYPE_ID(" << op.getCppNamespace()
+           << "::" << op.getCppClassName() << ")\n\n";
     } else {
+      {
+        NamespaceEmitter emitter(os, op.getCppNamespace());
         os << formatv(opCommentHeader, op.getQualCppClassName(), "definitions");
         OpOperandAdaptorEmitter::emitDef(op, os);
         OpEmitter::emitDef(op, os, staticVerifierEmitter);
       }
+      // Emit the TypeID explicit specialization to have a single definition.
+      if (!op.getCppNamespace().empty())
+        os << "DEFINE_EXPLICIT_TYPE_ID(" << op.getCppNamespace()
+           << "::" << op.getCppClassName() << ")\n\n";
+    }
   }
 }
 
 // Emits a comma-separated list of the ops.
 static void emitOpList(const std::vector<Record *> &defs, raw_ostream &os) {
   IfDefScope scope("GET_OP_LIST", os);
 
   interleave(
       // TODO: We are constructing the Operator wrapper instance just for