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

[lattner]

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

24

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

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

2601

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

2601

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

2601

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

2601

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

[rriddle]

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

375

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

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/Dialect/Affine/IR/AffineOps.h

 /// into `map`, maps of AffineApplyOps whose results are the values in
 /// `operands`, iteratively until no more of `operands` are the result of an
 /// AffineApplyOp. When this function returns, `map` becomes the composed affine
 /// map, and each Value in `operands` is guaranteed to be either a loop IV or a
 /// terminal symbol, i.e., a symbol defined at the top level or a block/function
 /// argument.
 void fullyComposeAffineMapAndOperands(AffineMap *map,
                                       SmallVectorImpl<Value> *operands);
-
+} // namespace mlir
 #include "mlir/Dialect/Affine/IR/AffineOpsDialect.h.inc"
 
 #define GET_OP_CLASSES
 #include "mlir/Dialect/Affine/IR/AffineOps.h.inc"
 
+namespace mlir {
 /// Returns true if the provided value is the induction variable of a
 /// AffineForOp.
 bool isForInductionVar(Value val);
 
 /// Returns the loop parent of an induction variable. If the provided value is
 /// not an induction variable, then return nullptr.
 AffineForOp getForInductionVarOwner(Value val);
 

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

 include "mlir/Dialect/StandardOps/IR/StandardOpsBase.td"
 include "mlir/Dialect/Affine/IR/AffineMemoryOpInterfaces.td"
 include "mlir/Interfaces/ControlFlowInterfaces.td"
 include "mlir/Interfaces/LoopLikeInterface.td"
 include "mlir/Interfaces/SideEffectInterfaces.td"
 
 def Affine_Dialect : Dialect {
   let name = "affine";
-  let cppNamespace = "";
+  let cppNamespace = "mlir";
   let hasConstantMaterializer = 1;
 }
 
 // Base class for Affine dialect ops.
 class Affine_Op<string mnemonic, list<OpTrait> traits = []> :
     Op<Affine_Dialect, mnemonic, traits> {
   // For every affine op, there needs to be a:
   //   * void print(OpAsmPrinter &p, ${C++ class of Op} op)

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);
+      }
+      os << "template<>\n";
+      os << "mlir::TypeID mlir::detail::TypeIDExported::get<"
+         << 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);
       }
+      os << "template<>\n";
+      os << "mlir::TypeID mlir::detail::TypeIDExported::get<"
+         << op.getCppNamespace() << "::" << op.getCppClassName() << ">() {\n";
+      os << "    static TypeID::Storage instance;\n";
+      os << "    return TypeID(&instance);\n";
+      os << "}\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(