[PATCH] D78434: [mlir] resolve types from attributes in assemblyFormat

[River Riddle via Phabricator via llvm-commits]

rriddle added a comment.

Sorry for the delay. Thanks for tackling this! Can you add a few tests?
https://github.com/llvm/llvm-project/blob/master/mlir/test/mlir-tblgen/op-format-spec.td
https://github.com/llvm/llvm-project/blob/master/mlir/test/mlir-tblgen/op-format.mlir
https://github.com/llvm/llvm-project/blob/6b3168f8cdb46656330929877b0b4daab35d30de/mlir/test/lib/Dialect/Test/TestOps.td#L1165



================
Comment at: mlir/tools/mlir-tblgen/OpFormatGen.cpp:314
+    /// the attribute that this type is resolved to.
+    const NamedAttribute *attribute;
   };
----------------
Can you use an llvm::PointerUnion for the variable and attribute to avoid another field?


================
Comment at: mlir/tools/mlir-tblgen/OpFormatGen.cpp:1339
     const NamedTypeConstraint *type;
+    const NamedAttribute *attr;
     Optional<StringRef> transformer;
----------------
Same here, could you use PointerUnion instead?


================
Comment at: mlir/tools/mlir-tblgen/OpFormatGen.cpp:1742
+    variableTyResolver[rhsName] = {
+        lhsArg, nullptr, def.getValueAsString("transformer")};
+  else if (const auto *attr = findSeenAttr(lhsName))
----------------
Is there a reason why we couldn't use the transformer with the attribute type?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78434/new/

https://reviews.llvm.org/D78434