Page MenuHomePhabricator

[mlir] Expand operand adapter to take attributes
ClosedPublic

Authored by jpienaar on May 21 2020, 6:42 PM.

Details

Summary
  • Enables using with more variadic sized operands;
  • Generate convenience accessors for attributes;
    • The accessor are named the same as their name in ODS and returns attribute type (not convenience type) and no derived attributes.

This would also allow verifying argument constraints before the op is even
created (and dedupe some checking between shapes and created ops). This does
not change the name of adaptor nor does it require it except for ops with
variadic operands to keep this change smaller. The named attributes created
such could potentially also replace the xAttr variants being generated.

I considered creating separate adapter for attributes but decided against that
given operands queries require attributes in general (and definitely for
verification of operands and attributes).

Diff Detail

Event Timeline

jpienaar created this revision.May 21 2020, 6:42 PM
antiagainst accepted this revision.May 22 2020, 6:16 AM

Nice, thanks Jacques!

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
73–77

This is not really code for calculation. What about naming it as "adapterSegmentSizeAttrInitCode" or something?

74

Nit: "missing segment size attribute for op" ?

77

Similarly here. What about "opSegmentSizeAttrInitCode"?

1860

Nit: we can do std::string sizeAttrInit = formatv() to avoid the .str() at the end and be more explicit about the type.

1872
This revision is now accepted and ready to land.May 22 2020, 6:16 AM
jpienaar updated this revision to Diff 265958.May 24 2020, 8:43 PM
jpienaar marked 6 inline comments as done.

Handling default attributes in adapter as in op

Updated, thanks

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
1872

Good point, this might also provide a different path to fixing that todo :)

This revision was automatically updated to reflect the committed changes.