Give attributes C++ namespaces.

[mehdi_amini]

mlir/include/mlir/IR/OpBase.td

795

s/types/attributes?

[fedelebron]

mlir/include/mlir/IR/OpBase.td

795

Maybe? I mean attributes don't live in namespaces, since attributes are an MLIR concept, and (this notion of) namespaces are a C++ thing. What I wanted to communicate is the codegenned types will be generated inside this namesapce. WDYT? Does MLIR endorse saying attributes/things "live in" namespaces, or their dialect's namespaces?

[fedelebron]

btw, no idea what the "failed" things above in "Diff Detail" are, and clicking around didn't really teach me. Hoping I can ignore them...

[mehdi_amini]

In D89014#2318277, @fedelebron wrote:

btw, no idea what the "failed" things above in "Diff Detail" are, and clicking around didn't really teach me. Hoping I can ignore them...

Pre-merge testing. It is failing because of an infrastructure issue I believe (@goncharov @kuhnel ):

stderr: 'kex_exchange_identification: Connection closed by remote host

mlir/include/mlir/IR/OpBase.td

795

Then I'd rather read "The fully-qualified C++ namespace where the generated class lives."

[kuhnel]

Teh build error looks like a connection issue to github, the script can't git fetch. We already have an issue to retry in such cases.

The easiest solution is probably to just restart the buildable in Phabricator (what I just did).

[fedelebron]

Fixing review comment.

[fedelebron]

Could this be committed? (I don't have commit access)

mlir/include/mlir/IR/OpBase.td

class Attr<Pred condition, string descr = ""> :
   //
   // This field is used by attribute wrapper classes (DefaultValuedAttr,
   // OptionalAttr, etc.) to retrieve the base-level attribute definition.
   // This can be used for getting its name; otherwise, we will see
   // "anonymous_<number>" as the attribute def name because of template
   // instantiation.
   // TOOD(b/132458159): deduplicate the fields in attribute wrapper classes.
   Attr baseAttr = ?;
+
+  // The fully-qualified C++ namespace where the generated class lives.
+  string cppNamespace = "";
 }
 
 // An attribute of a specific dialect.
 class DialectAttr<Dialect d, Pred condition, string descr = ""> :
     Attr<condition, descr> {
   Dialect dialect = d;
+  let cppNamespace = d.cppNamespace;
 }
 
 //===----------------------------------------------------------------------===//
 // Attribute modifier definition
 
 // Decorates an attribute to have an (unvalidated) default value if not present.
 class DefaultValuedAttr<Attr attr, string val> :
     Attr<attr.predicate, attr.description> {
class EnumAttrInfo<string name, list<EnumAttrCaseInfo> cases> {
 
   // The following fields are only used by the EnumsGen backend to generate
   // an enum class definition and conversion utility functions.
 
   // The underlying type for the C++ enum class. An empty string mean the
   // underlying type is not explicitly specified.
   string underlyingType = "";
 
-  // The C++ namespaces that the enum class definition and utility functions
-  // should be placed into.
-  //
-  // Normally you want to place the full namespace path here. If it is nested,
-  // use "::" as the delimiter, e.g., given "A::B", generated code will be
-  // placed in `namespace A { namespace B { ... } }`. To avoid placing in any
-  // namespace, use "".
-  // TODO: use dialect to provide the namespace.
-  string cppNamespace = "";
-
   // The name of the utility function that converts a value of the underlying
   // type to the corresponding symbol. It will have the following signature:
   //
   // ```c++
   // llvm::Optional<<qualified-enum-class-name>> <fn-name>(<underlying-type>);
   // ```
   string underlyingToSymbolFnName = "symbolize" # name;
 
class StructFieldAttr<string thisName, Attr thisType> {
   Attr type = thisType;
 }
 
 // Structured attribute that wraps a DictionaryAttr and provides both a
 // validation method and set of accessors for a fixed set of fields. This is
 // useful when representing data that would normally be in a structure.
 class StructAttr<string name, Dialect d,
                  list<StructFieldAttr> attributes> :
-    DictionaryAttrBase<CPred<"$_self.isa<" # name # ">()">,
+    DictionaryAttrBase<CPred<"$_self.isa<" # d.cppNamespace
+                                           # "::" # name # ">()">,
         "DictionaryAttr with field(s): " #
         StrJoin<!foreach(a, attributes, "'" # a.name # "'"), ", ">.result #
         " (each field having its own constraints)"> {
   // Name for this StructAttr.
   string className = name;
 
   // Return type should match the name of the structure.
-  let returnType = name;
+  let returnType = d.cppNamespace # "::" # name;
 
   // Storage type should match the name of the structure.
-  let storageType = name;
+  let storageType = d.cppNamespace # "::" # name;
 
   // The dialect this StructAttr belongs to.
   Dialect dialect = d;
 
+  let cppNamespace = d.cppNamespace;
+
   // List of fields that the StructAttr contains.
   list<StructFieldAttr> fields = attributes;
 }
 
 // Attributes containing symbol references.
 def SymbolRefAttr : Attr<CPred<"$_self.isa<::mlir::SymbolRefAttr>()">,
                         "symbol reference attribute"> {
   let storageType = [{ ::mlir::SymbolRefAttr }];

mlir/lib/TableGen/Pattern.cpp

std::string SymbolInfoMap::SymbolInfo::getVarDecl(StringRef name) const {
   case Kind::Attr: {
     auto type =
         op->getArg(*argIndex).get<NamedAttribute *>()->attr.getStorageType();
     return std::string(formatv("{0} {1};\n", type, name));
   }
   case Kind::Operand: {
     // Use operand range for captured operands (to support potential variadic
     // operands).
-    return std::string(
-        formatv("Operation::operand_range {0}(op0->getOperands());\n", name));
+    return std::string(formatv(
+        "::mlir::Operation::operand_range {0}(op0->getOperands());\n", name));
   }
   case Kind::Value: {
-    return std::string(formatv("ArrayRef<Value> {0};\n", name));
+    return std::string(formatv("::llvm::ArrayRef<::mlir::Value> {0};\n", name));
   }
   case Kind::Result: {
     // Use the op itself for captured results.
     return std::string(formatv("{0} {1};\n", op->getQualCppClassName(), name));
   }
   }
   llvm_unreachable("unknown kind");
 }

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

auto emitDerivedAttr = [&](StringRef name, Attribute attr) {
     if (!method)
       return;
     auto &body = method->body();
     body << "  " << attr.getDerivedCodeBody() << "\n";
   };
 
   // Emit with return type specified.
   auto emitAttrWithReturnType = [&](StringRef name, Attribute attr) {
-    Dialect attrDialect = attr.getDialect();
-    // Does the current operation have a different namespace than the attribute?
-    bool differentNamespace =
-        attrDialect && opDialect && attrDialect != opDialect;
-    std::string returnType = differentNamespace
-                                 ? (llvm::Twine(attrDialect.getCppNamespace()) +
-                                    "::" + attr.getReturnType())
-                                       .str()
-                                 : attr.getReturnType().str();
-    auto *method = opClass.addMethodAndPrune(returnType, name);
+    auto *method = opClass.addMethodAndPrune(attr.getReturnType(), name);
     auto &body = method->body();
     body << "  auto attr = " << name << "Attr();\n";
     if (attr.hasDefaultValue()) {
       // Returns the default value if not set.
       // TODO: this is inefficient, we are recreating the attribute for every
       // call. This should be set instead.
       std::string defaultValue = std::string(
           tgfmt(attr.getConstBuilderTemplate(), &fctx, attr.getDefaultValue()));
for (int i = 0; i != numResults; ++i)
     resultNames[i] = op.getResultName(i);
 
   // Don't add the trait if none of the results have a valid name.
   if (llvm::all_of(resultNames, [](StringRef name) { return name.empty(); }))
     return;
   opClass.addTrait("::mlir::OpAsmOpInterface::Trait");
 
   // Generate the right accessor for the number of results.
-  auto *method = opClass.addMethodAndPrune("void", "getAsmResultNames",
-                                           "OpAsmSetValueNameFn", "setNameFn");
+  auto *method = opClass.addMethodAndPrune(
+      "void", "getAsmResultNames", "::mlir::OpAsmSetValueNameFn", "setNameFn");
   auto &body = method->body();
   for (int i = 0; i != numResults; ++i) {
     body << "  auto resultGroup" << i << " = getODSResults(" << i << ");\n"
          << "  if (!llvm::empty(resultGroup" << i << "))\n"
          << "    setNameFn(*resultGroup" << i << ".begin(), \""
          << resultNames[i] << "\");\n";
   }
 }

mlir/tools/mlir-tblgen/RewriterGen.cpp

 void PatternEmitter::emitOpMatch(DagNode tree, int depth) {
   Operator &op = tree.getDialectOp(opMap);
   LLVM_DEBUG(llvm::dbgs() << "start emitting match for op '"
                           << op.getOperationName() << "' at depth " << depth
                           << '\n');
 
   int indent = 4 + 2 * depth;
   os.indent(indent) << formatv(
-      "auto castedOp{0} = dyn_cast_or_null<{1}>(op{0}); (void)castedOp{0};\n",
+      "auto castedOp{0} = ::llvm::dyn_cast_or_null<{1}>(op{0}); "
+      "(void)castedOp{0};\n",
       depth, op.getQualCppClassName());
   // Skip the operand matching at depth 0 as the pattern rewriter already does.
   if (depth != 0) {
     // Skip if there is no defining operation (e.g., arguments to function).
     os << formatv("if (!castedOp{0})\n  return failure();\n", depth);
   }
   if (tree.getNumArgs() != op.getNumArgs()) {
     PrintFatalError(loc, formatv("op '{0}' argument number mismatch: {1} in "
::mlir::LogicalResult matchAndRewrite(::mlir::Operation *op0,
 
       os << "// Match\n";
       os << "tblgen_ops[0] = op0;\n";
       emitMatchLogic(sourceTree);
 
       os << "\n// Rewrite\n";
       emitRewriteLogic();
 
-      os << "return success();\n";
+      os << "return ::mlir::success();\n";
     }
     os << "};\n";
   }
   os << "};\n\n";
 }
 
 void PatternEmitter::emitRewriteLogic() {
   LLVM_DEBUG(llvm::dbgs() << "--- start emitting rewrite logic ---\n");
LLVM_DEBUG(llvm::dbgs()
                << "=== start generating pattern '" << name << "' ===\n");
     PatternEmitter(p, &recordOpMap, os).emit(name);
     LLVM_DEBUG(llvm::dbgs()
                << "=== done generating pattern '" << name << "' ===\n");
     rewriterNames.push_back(std::move(name));
   }
 
   // Emit function to add the generated matchers to the pattern list.
-  os << "void LLVM_ATTRIBUTE_UNUSED populateWithGenerated(MLIRContext "
-        "*context, OwningRewritePatternList *patterns) {\n";
+  os << "void LLVM_ATTRIBUTE_UNUSED populateWithGenerated(::mlir::MLIRContext "
+        "*context, ::mlir::OwningRewritePatternList *patterns) {\n";
   for (const auto &name : rewriterNames) {
     os << "  patterns->insert<" << name << ">(context);\n";
   }
   os << "}\n";
 }
 
 static mlir::GenRegistration
     genRewriters("gen-rewriters", "Generate pattern rewriters",
                  [](const RecordKeeper &records, raw_ostream &os) {
                    emitRewriters(records, os);
                    return false;
                  });