This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Add basic support for attributes in ODS-generated Python bindings
ClosedPublic

Authored by ftynse on Nov 16 2020, 7:34 AM.

Details

Summary

In ODS, attributes of an operation can be provided as a part of the "arguments"
field, together with operands. Such attributes are accepted by the op builder
and have accessors generated.

Implement similar functionality for ODS-generated op-specific Python bindings:
the __init__ method now accepts arguments together with operands, in the same
order as in the ODS arguments field; the instance properties are introduced
to OpView classes to access the attributes.

This initial implementation accepts and returns instances of the corresponding
attribute class, and not the underlying values since the mapping scheme of the
value types between C++, C and Python is not yet clear. Default-valued
attributes are not supported as that would require Python to be able to parse
C++ literals.

Since attributes in ODS are tightely related to the actual C++ type system,
provide a separate Tablegen file with the mapping between ODS storage type for
attributes (typically, the underlying C++ attribute class), and the
corresponding class name. So far, this might look unnecessary since all names
match exactly, but this is not necessarily the cases for non-standard,
out-of-tree attributes, which may also be placed in non-default namespaces or
Python modules. This also allows out-of-tree users to generate Python bindings
without having to modify the bindings generator itself. Storage type was
preferred over the Tablegen "def" of the attribute class because ODS
essentially encodes attribute _constraints_ rather than classes, e.g. there may
be many Tablegen "def"s in the ODS that correspond to the same attribute type
with additional constraints

The presence of the explicit mapping requires the change in the .td file
structure: instead of just calling the bindings generator directly on the main
ODS file of the dialect, it becomes necessary to create a new file that
includes the main ODS file of the dialect and provides the mapping for
attribute types. Arguably, this approach offers better separability of the
Python bindings in the build system as the main dialect no longer needs to know
that it is being processed by the bindings generator.

Diff Detail

Event Timeline

ftynse created this revision.Nov 16 2020, 7:34 AM
ftynse requested review of this revision.Nov 16 2020, 7:34 AM
ftynse updated this revision to Diff 305539.Nov 16 2020, 9:17 AM

Simplify cmake further

ftynse updated this revision to Diff 305548.Nov 16 2020, 10:10 AM

Customize handling of unit attributes

stellaraccident accepted this revision.Nov 16 2020, 9:59 PM

This is quite slick. A couple of nits.

mlir/lib/Bindings/Python/Attributes.td
26

Are these relative to mlir.ir if unqualified?

mlir/test/mlir-tblgen/op-python-bindings.td
121

Valid? You qualify other things with _ir. but not the Location class.

I can't see the whole file yet so leaving a comment now. Also some other instances of this (IntegerAttr, FloatAttr, below).

159

I think we can add a helper for this. Something like mlir.ir._defaultLocContext that can take a None Location and do the right thing. (for a followup).

This revision is now accepted and ready to land.Nov 16 2020, 9:59 PM
ftynse updated this revision to Diff 305710.Nov 17 2020, 2:41 AM
ftynse marked 2 inline comments as done.

Address review.

This revision was landed with ongoing or failed builds.Nov 17 2020, 2:47 AM
This revision was automatically updated to reflect the committed changes.
ftynse added inline comments.Nov 17 2020, 3:27 AM
mlir/lib/Bindings/Python/Attributes.td
26

Good point. I added an explicit qualification with _ir. for now. We will have to figure out the story for dialect-specific attributes, presumably import definitions into the generated file somehow.

mlir/test/mlir-tblgen/op-python-bindings.td
159

Ack.