This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Fully qualify default generated type/attribute printer and parser
ClosedPublic

Authored by zero9178 on Nov 18 2021, 8:57 AM.

Details

Summary

This patch makes it possible to use the newly added useDefaultAttributePrinterParser and useDefaultTypePrinterParser dialect options without any using namespace declarations. Two things had to be done to make this possible:

  • Fully qualify any type usages or function from the mlir namespace in the generated C++ code
  • Makes sure to emit the printers and parsers inside the same namespace as the Dialect

Diff Detail

Event Timeline

zero9178 created this revision.Nov 18 2021, 8:57 AM
zero9178 requested review of this revision.Nov 18 2021, 8:57 AM
rriddle added inline comments.Nov 18 2021, 10:19 AM
mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
1043

valueType == "Attribute" || valueType == "Type"

When is the not true?

1044–1046

Instead of duplicating checks, why not just sink this into both of the if statements below?

zero9178 updated this revision to Diff 388262.Nov 18 2021, 10:34 AM

Addressed review comments

zero9178 marked 2 inline comments as done.Nov 18 2021, 10:35 AM
zero9178 added inline comments.
mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
1043

I think my original thought was in case valueType does ever get another possible value. Unlikely now that I think about it so happy to remove that check. Sinking the NamespaceEmitter as you suggested makes this redundant either way.

rriddle accepted this revision.Nov 18 2021, 10:48 AM

Thanks for the fix!

This revision is now accepted and ready to land.Nov 18 2021, 10:48 AM
mehdi_amini accepted this revision.Nov 18 2021, 11:10 AM
This revision was landed with ongoing or failed builds.Nov 18 2021, 11:24 AM
This revision was automatically updated to reflect the committed changes.
zero9178 marked an inline comment as done.