[mlir][ODS]: Add per-op cppNamespace.

[silvas]

Summary

This is useful for dialects that have logical subparts.

[silvas]

I have a dialect in npcomp that would greatly benefit from this. The basic problem is that we are importing a notion of "op" that is present in PyTorch, and has a "namespace" and "unqualified portion" of the op names. The "namespace" isn't really a dialect, but is useful to map to C++ namespaces in the code. We still model all these ops as part of a single dialect of "registered torch ops".

[mehdi_amini]

I don't quire get why can't just use the dialect namespace?
Anyway, not really opposed either, but can you add an op in the test dialect that would use this?

[silvas]

In D102200#2749139, @mehdi_amini wrote:

I don't quire get why can't just use the dialect namespace?
Anyway, not really opposed either, but can you add an op in the test dialect that would use this?

What I want is for different ops in the dialect to be in different namespaces. See dialect.td in the patch for an example.

[silvas]

factor helper

[mehdi_amini]

In D102200#2749288, @silvas wrote:

In D102200#2749139, @mehdi_amini wrote:

I don't quire get why can't just use the dialect namespace?
Anyway, not really opposed either, but can you add an op in the test dialect that would use this?

What I want is for different ops in the dialect to be in different namespaces. See dialect.td in the patch for an example.

I understand what the patch is doing and what you want, I don't really understand why...

[silvas]

In D102200#2749475, @mehdi_amini wrote:

In D102200#2749288, @silvas wrote:

In D102200#2749139, @mehdi_amini wrote:

I don't quire get why can't just use the dialect namespace?
Anyway, not really opposed either, but can you add an op in the test dialect that would use this?

What I want is for different ops in the dialect to be in different namespaces. See dialect.td in the patch for an example.

I understand what the patch is doing and what you want, I don't really understand why...

See my above comment:

I have a dialect in npcomp that would greatly benefit from this. The basic problem is that we are importing a notion of "op" that is present in PyTorch, and has a "namespace" and "unqualified portion" of the op names. The "namespace" isn't really a dialect, but is useful to map to C++ namespaces in the code. We still model all these ops as part of a single dialect of "registered torch ops".

The basic problem is that PyTorch has notions of "ops" like prim::ListUnpack, aten::add, etc. The logical thing (because these "namespaces" are not "dialects" in the MLIR sense; they are just random prefixes that don't really have a strong structure to them) is to import them into the same dialect as torch.prim.ListUnpack and torch.aten.add. However, at the C++ level, I still want to retain ...::prim::ListUnpackOp and ...::aten::AddOp. Does that make sense? I could instead do PrimListUnpackOp and AtenAddOp but it just seems ugly.

[jpienaar]

In D102200#2750867, @silvas wrote:

In D102200#2749475, @mehdi_amini wrote:

In D102200#2749288, @silvas wrote:

In D102200#2749139, @mehdi_amini wrote:

I don't quire get why can't just use the dialect namespace?
Anyway, not really opposed either, but can you add an op in the test dialect that would use this?

What I want is for different ops in the dialect to be in different namespaces. See dialect.td in the patch for an example.

I understand what the patch is doing and what you want, I don't really understand why...

See my above comment:

I have a dialect in npcomp that would greatly benefit from this. The basic problem is that we are importing a notion of "op" that is present in PyTorch, and has a "namespace" and "unqualified portion" of the op names. The "namespace" isn't really a dialect, but is useful to map to C++ namespaces in the code. We still model all these ops as part of a single dialect of "registered torch ops".

The basic problem is that PyTorch has notions of "ops" like prim::ListUnpack, aten::add, etc. The logical thing (because these "namespaces" are not "dialects" in the MLIR sense; they are just random prefixes that don't really have a strong structure to them) is to import them into the same dialect as torch.prim.ListUnpack and torch.aten.add. However, at the C++ level, I still want to retain ...::prim::ListUnpackOp and ...::aten::AddOp. Does that make sense? I could instead do PrimListUnpackOp and AtenAddOp but it just seems ugly.

I'd have thought it most natural to import each into its own dialect (e.g., torch_prim.ListUnpack) or having "subdialect" concept (which has been tossed around but unclear what it means - although with dialect fallbacks these become interesting because a subdialect could fallback to main dialect ...). These seem to be in same dialect just because from same frontend?

[silvas]

In D102200#2750906, @jpienaar wrote:

In D102200#2750867, @silvas wrote:

In D102200#2749475, @mehdi_amini wrote:

In D102200#2749288, @silvas wrote:

In D102200#2749139, @mehdi_amini wrote:

I don't quire get why can't just use the dialect namespace?
Anyway, not really opposed either, but can you add an op in the test dialect that would use this?

What I want is for different ops in the dialect to be in different namespaces. See dialect.td in the patch for an example.

I understand what the patch is doing and what you want, I don't really understand why...

See my above comment:

I have a dialect in npcomp that would greatly benefit from this. The basic problem is that we are importing a notion of "op" that is present in PyTorch, and has a "namespace" and "unqualified portion" of the op names. The "namespace" isn't really a dialect, but is useful to map to C++ namespaces in the code. We still model all these ops as part of a single dialect of "registered torch ops".

The basic problem is that PyTorch has notions of "ops" like prim::ListUnpack, aten::add, etc. The logical thing (because these "namespaces" are not "dialects" in the MLIR sense; they are just random prefixes that don't really have a strong structure to them) is to import them into the same dialect as torch.prim.ListUnpack and torch.aten.add. However, at the C++ level, I still want to retain ...::prim::ListUnpackOp and ...::aten::AddOp. Does that make sense? I could instead do PrimListUnpackOp and AtenAddOp but it just seems ugly.

I'd have thought it most natural to import each into its own dialect (e.g., torch_prim.ListUnpack) or having "subdialect" concept (which has been tossed around but unclear what it means - although with dialect fallbacks these become interesting because a subdialect could fallback to main dialect ...). These seem to be in same dialect just because from same frontend?

One-dialect-per-torch-namespace-prefix is not a good design point for us, because there are many namespaces, and they don't follow the types of structural rationales that characterize MLIR dialects.

[mehdi_amini]

In D102200#2750867, @silvas wrote:

See my above comment:

I have a dialect in npcomp that would greatly benefit from this. The basic problem is that we are importing a notion of "op" that is present in PyTorch, and has a "namespace" and "unqualified portion" of the op names. The "namespace" isn't really a dialect, but is useful to map to C++ namespaces in the code. We still model all these ops as part of a single dialect of "registered torch ops".

I read it, but beyond saying "it is useful", it didn't bring clarity to me.

However, at the C++ level, I still want to retain ...::prim::ListUnpackOp and ...::aten::AddOp. Does that make sense?

Not really: I mean it seems arbitrary and disconnected from the way Ops in dialect usually work. Again I'm not objecting to adding the feature here to MLIR, but I would find it confusing that I can't access all the ops in a dialect from the dialect namespace. As long as it makes sense in the context of your use case that's fine...

[silvas]

It seems there is agreement that this is a useful feature to support. Can someone LGTM?

[mehdi_amini]

LGTM

mlir/include/mlir/IR/OpBase.td

 // Base class for all ops.
 class Op<Dialect dialect, string mnemonic, list<OpTrait> props = []> {
   // The dialect of the op.
   Dialect opDialect = dialect;
 
   // The mnemonic of the op.
   string opName = mnemonic;
 
+  // The C++ namespace to use for this op.
+  string cppNamespace = dialect.cppNamespace;
+
   // One-line human-readable description of what the op does.
   string summary = "";
 
   // Additional, longer human-readable description of what the op does.
   string description = "";
 
   // Dag containing the arguments of the op. Default to 0 arguments.
   dag arguments = (ins);

mlir/include/mlir/TableGen/CodeGenHelpers.h

 };
 
 // A helper RAII class to emit nested namespaces for this op.
 class NamespaceEmitter {
 public:
   NamespaceEmitter(raw_ostream &os, const Dialect &dialect) : os(os) {
     if (!dialect)
       return;
-    llvm::SplitString(dialect.getCppNamespace(), namespaces, "::");
-    for (StringRef ns : namespaces)
-      os << "namespace " << ns << " {\n";
+    emitNamespaceStarts(os, dialect.getCppNamespace());
+  }
+  NamespaceEmitter(raw_ostream &os, StringRef cppNamespace) : os(os) {
+    emitNamespaceStarts(os, cppNamespace);
   }
 
   ~NamespaceEmitter() {
     for (StringRef ns : llvm::reverse(namespaces))
       os << "} // namespace " << ns << "\n";
   }
 
 private:
+  void emitNamespaceStarts(raw_ostream &os, StringRef cppNamespace) {
+    llvm::SplitString(cppNamespace, namespaces, "::");
+    for (StringRef ns : namespaces)
+      os << "namespace " << ns << " {\n";
+  }
   raw_ostream &os;
   SmallVector<StringRef, 2> namespaces;
 };
 
 } // namespace tblgen
 } // namespace mlir
 
 #endif // MLIR_TABLEGEN_CODEGENHELPERS_H

mlir/include/mlir/TableGen/Operator.h

public:
   std::string getOperationName() const;
 
   // Returns this op's C++ class name.
   StringRef getCppClassName() const;
 
   // Returns this op's C++ class name prefixed with namespaces.
   std::string getQualCppClassName() const;
 
+  // Returns this op's C++ namespace.
+  StringRef getCppNamespace() const;
+
   // Returns the name of op's adaptor C++ class.
   std::string getAdaptorName() const;
 
   /// A class used to represent the decorators of an operator variable, i.e.
   /// argument or result.
   struct VariableDecorator {
   public:
     explicit VariableDecorator(const llvm::Record *def) : def(def) {}
void populateTypeInferenceInfo(
       const llvm::StringMap<int> &argumentsAndResultsIndex);
 
   // The dialect of this op.
   Dialect dialect;
 
   // The unqualified C++ class name of the op.
   StringRef cppClassName;
 
+  // The C++ namespace for this op.
+  StringRef cppNamespace;
+
   // The operands of the op.
   SmallVector<NamedTypeConstraint, 4> operands;
 
   // The attributes of the op.  Contains native attributes (corresponding to the
   // actual stored attributed of the operation) followed by derived attributes
   // (corresponding to dynamic properties of the operation that are computed
   // upon request).
   SmallVector<NamedAttribute, 4> attributes;

mlir/lib/TableGen/Operator.cpp

Operator::Operator(const llvm::Record &def)
   if (prefix.empty()) {
     // Class name with a leading underscore and without dialect prefix
     cppClassName = def.getName();
   } else if (cppClassName.empty()) {
     // Class name without dialect prefix
     cppClassName = prefix;
   }
 
+  cppNamespace = def.getValueAsString("cppNamespace");
+
   populateOpStructure();
 }
 
 std::string Operator::getOperationName() const {
   auto prefix = dialect.getName();
   auto opName = def.getValueAsString("opName");
   if (prefix.empty())
     return std::string(opName);
   return std::string(llvm::formatv("{0}.{1}", prefix, opName));
 }
 
 std::string Operator::getAdaptorName() const {
   return std::string(llvm::formatv("{0}Adaptor", getCppClassName()));
 }
 
 StringRef Operator::getDialectName() const { return dialect.getName(); }
 
 StringRef Operator::getCppClassName() const { return cppClassName; }
 
 std::string Operator::getQualCppClassName() const {
-  auto prefix = dialect.getCppNamespace();
-  if (prefix.empty())
+  if (cppNamespace.empty())
     return std::string(cppClassName);
-  return std::string(llvm::formatv("{0}::{1}", prefix, cppClassName));
+  return std::string(llvm::formatv("{0}::{1}", cppNamespace, cppClassName));
 }
 
+StringRef Operator::getCppNamespace() const { return cppNamespace; }
+
 int Operator::getNumResults() const {
   DagInit *results = def.getValueAsDag("results");
   return results->getNumArgs();
 }
 
 StringRef Operator::getExtraClassDeclaration() const {
   constexpr auto attr = "extraClassDeclaration";
   if (def.isValueUnset(attr))

mlir/test/mlir-tblgen/dialect.td

 // Check no namespaces
 def D_Dialect : Dialect {
   let name = "d";
   let cppNamespace = "";
 }
 
 def D_DSomeOp : Op<D_Dialect, "some_op", []>;
 
+// Check op with namespace override.
+def E_Dialect : Dialect {
+  let name = "e";
+  let cppNamespace = "ENS";
+}
+
+def E_SomeOp : Op<E_Dialect, "some_op", []>;
+def E_SpecialNSOp : Op<E_Dialect, "special_ns_op", []> {
+  let cppNamespace = "::E::SPECIAL_NS";
+}
+
 // DEF-LABEL: GET_OP_LIST
 // DEF:      a::SomeOp
 // DEF-NEXT: BNS::SomeOp
 // DEF-NEXT: ::C::CC::SomeOp
 // DEF-NEXT: DSomeOp
+// DEF-NEXT: ENS::SomeOp
+// DEF-NEXT: ::E::SPECIAL_NS::SpecialNSOp
 
 // DEF-LABEL: GET_OP_CLASSES
 // DEF: a::SomeOp definitions
 // DEF: BNS::SomeOp definitions
 // DEF: ::C::CC::SomeOp definitions
 // DEF: DSomeOp definitions
+// DEF: ENS::SomeOp definitions
+// DEF: ::E::SPECIAL_NS::SpecialNSOp definitions
 
 // DECL-LABEL: GET_OP_CLASSES
 // DECL: a::SomeOp declarations
 // DECL: BNS::SomeOp declarations
 // DECL: ::C::CC::SomeOp declarations
 // DECL: DSomeOp declarations
+// DECL: ENS::SomeOp declarations
+// DECL: ::E::SPECIAL_NS::SpecialNSOp declarations

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

 
 StaticVerifierFunctionEmitter::StaticVerifierFunctionEmitter(
     const llvm::RecordKeeper &records, ArrayRef<llvm::Record *> opDefs,
     raw_ostream &os, bool emitDecl)
     : uniqueOutputLabel(getUniqueName(records)) {
   llvm::Optional<NamespaceEmitter> namespaceEmitter;
   if (!emitDecl) {
     os << formatv(opCommentHeader, "Local Utility Method", "Definitions");
-    namespaceEmitter.emplace(os, Operator(*opDefs[0]).getDialect());
+    namespaceEmitter.emplace(os, Operator(*opDefs[0]).getCppNamespace());
   }
 
   emitTypeConstraintMethods(opDefs, os, emitDecl);
 }
 
 std::string StaticVerifierFunctionEmitter::getUniqueName(
     const llvm::RecordKeeper &records) {
   // Use the input file name when generating a unique name.
static void emitOpClasses(const RecordKeeper &recordKeeper,
                           bool emitDecl) {
   // First emit forward declaration for each class, this allows them to refer
   // to each others in traits for example.
   if (emitDecl) {
     os << "#if defined(GET_OP_CLASSES) || defined(GET_OP_FWD_DEFINES)\n";
     os << "#undef GET_OP_FWD_DEFINES\n";
     for (auto *def : defs) {
       Operator op(*def);
-      NamespaceEmitter emitter(os, op.getDialect());
+      NamespaceEmitter emitter(os, op.getCppNamespace());
       os << "class " << op.getCppClassName() << ";\n";
     }
     os << "#endif\n\n";
   }
 
   IfDefScope scope("GET_OP_CLASSES", os);
   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.getDialect());
+    NamespaceEmitter emitter(os, op.getCppNamespace());
     if (emitDecl) {
       os << formatv(opCommentHeader, op.getQualCppClassName(), "declarations");
       OpOperandAdaptorEmitter::emitDecl(op, os);
       OpEmitter::emitDecl(op, os, staticVerifierEmitter);
     } else {
       os << formatv(opCommentHeader, op.getQualCppClassName(), "definitions");
       OpOperandAdaptorEmitter::emitDef(op, os);
       OpEmitter::emitDef(op, os, staticVerifierEmitter);