This is an archive of the discontinued LLVM Phabricator instance.

[MLIR][TableGen] Add default value for named attributes for 2 more build methods
ClosedPublic

Authored by jurahul on Jul 14 2020, 6:28 PM.

Details

Summary
  • Added more default values for attributes parameter for 2 more build methods
  • Extend the op-decls.td unit test to test these build methods.

Diff Detail

Event Timeline

jurahul created this revision.Jul 14 2020, 6:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 14 2020, 6:28 PM
jurahul marked an inline comment as done.
jurahul added inline comments.
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
1064

Note, eliminating use of formatv() as it does not correctly handle escaped closing braces. I have put up a fix for that (https://reviews.llvm.org/D83837) but here we don't need to use formatv.

jpienaar accepted this revision.Jul 16 2020, 9:19 AM
jpienaar marked 2 inline comments as done.
jpienaar added inline comments.
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
1033

Part of me wonders if we should where ops has required attributes. E.g., if we know op X requires an attribute (not default and not optional), then any invocation of build that doesn't pass an attribute will fail verification. Unless added later which we would have no knowledge of here, but this would force them to think about it.

I'm not sure the value of complicating this for that use though, so good as is.

1064

Interesting, I thought there was some trick wrt escaping that we had used elsewhere.

This revision is now accepted and ready to land.Jul 16 2020, 9:19 AM
jurahul marked an inline comment as done.Jul 16 2020, 9:24 AM
jurahul added inline comments.
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
1033

FYI, I have seen cases of this where we call build() with empty attributes and then tack on a bunch of attributes on the generated operation.

This revision was automatically updated to reflect the committed changes.