[Mlir-commits] [mlir] [mlir] load dialects for non-namespaced attrs (PR #94838)

[Jeremy Kun]

j2kun wrote:

Logging my attempts

---

Changing `FieldParser::parse` for a custom attribute:

```diff
diff --git a/mlir/include/mlir/IR/DialectImplementation.h b/mlir/include/mlir/IR/DialectImplementation.h
index 1e4f7f787a1..9d51994b012 100644
--- a/mlir/include/mlir/IR/DialectImplementation.h
+++ b/mlir/include/mlir/IR/DialectImplementation.h
@@ -64,6 +64,11 @@ struct FieldParser<
                                  AttributeT>> {
   static FailureOr<AttributeT> parse(AsmParser &parser) {
     AttributeT value;
+
+    SmallVector<StringRef, 2> vec;
+    StringRef(AttributeT::name).split(vec, '.');
+    parser.getContext()->getOrLoadDialect(vec[0]);
+
     if (parser.parseCustomAttributeWithFallback(value))
       return failure();
     return value;
```

Doesn't work because not all attributes have the `name` static field. Also somewhat messy with the string parsing. Similar obstacle to doing this lower in the call stack in `parseCustomAttributeWithFallback`

---

Changing `DefFormat::genParser` doesn't seem to work because:

1. Loading the context object's dialect doesn't work because the example in this PR is a custom parser so it doesn't get a `genParser`'ed parse method. In this case the `test` dialect is forced to load, but not `polynomial`.
2. Adding it when iterating over fields to parse doesn't work because, as far as I can tell, `AttrOrTypeParameter` does not have access to the dialect object or the dialect name, even though it logically comes from an `AttrOrTypeDef`, which does.

```diff
--- a/mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp
@@ -166,6 +166,7 @@ static const char *const parserErrorStr =
 /// {4}: C++ class of the parameter.
 static const char *const variableParser = R"(
 // Parse variable '{0}'
+odsParser.getContext()->getOrLoadDialect("{5}");
 _result_{0} = {1};
 if (::mlir::failed(_result_{0})) {{
   {2}"failed to parse {3} parameter '{0}' which is to be a `{4}`");
@@ -291,6 +292,10 @@ void DefFormat::genParser(MethodBody &os) {
   os.indent();
   os << "::mlir::Builder odsBuilder(odsParser.getContext());\n";

+  // Ensure the dialect is loaded
+  os << "odsParser.getContext()->getOrLoadDialect(\""
+     << def.getDialect().getName() << "\");\n"; // loads "test" dialect here
+
   // Store the initial location of the parser.
   ctx.addSubst("_loc", "odsLoc");
   os << tgfmt("::llvm::SMLoc $_loc = $_parser.getCurrentLocation();\n"
@@ -411,9 +416,13 @@ void DefFormat::genVariableParser(ParameterElement *el, FmtContext &ctx,
   auto customParser = param.getParser();
   auto parser =
       customParser ? *customParser : StringRef(defaultParameterParser);
+
+  StringRef dialectName = ???;
+
   os << formatv(variableParser, param.getName(),
                 tgfmt(parser, &ctx, param.getCppStorageType()),
-                tgfmt(parserErrorStr, &ctx), def.getName(), param.getCppType());
+                tgfmt(parserErrorStr, &ctx), def.getName(), param.getCppType(),
+                dialectName);
 }
```

https://github.com/llvm/llvm-project/pull/94838