This is an archive of the discontinued LLVM Phabricator instance.

[mlir][python] support taking ops instead of values in op constructors
ClosedPublic

Authored by ftynse on Oct 7 2021, 5:13 AM.

Details

Summary

Introduce support for accepting ops instead of values when constructing ops. A
single-result op can be used instead of a value, including in lists of values,
and any op can be used instead of a list of values. This is similar to, but
more powerful, than the C++ API that allows for implicitly casting an OpType to
Value if it is statically known to have a single result - the cast in Python is
based on the op dynamically having a single result, and also handles the
multi-result case. This allows to build IR in a more concise way:

op = dialect.produce_multiple_results()
other = dialect.produce_single_result()
dialect.consume_multiple_results(other, op)

instead of having to access the results manually

op = dialect.produce.multiple_results()
other = dialect.produce_single_result()
dialect.consume_multiple_results(other.result, op.operation.results)

The dispatch is implemented directly in Python and is triggered automatically
for autogenerated OpView subclasses. Extension OpView classes should use the
functions provided in ods_common.py if they want to implement this behavior.
An alternative could be to implement the dispatch in the C++ bindings code, but
it would require to forward opaque types through all Python functions down to a
binding call, which makes it hard to inspect them in Python, e.g., to obtain
the types of values.

Diff Detail

Event Timeline

ftynse created this revision.Oct 7 2021, 5:13 AM
ftynse requested review of this revision.Oct 7 2021, 5:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2021, 5:13 AM
gysit added inline comments.Oct 7 2021, 7:22 AM
mlir/python/mlir/dialects/_ods_common.py
133

typo: reuslt -> result

mlir/python/mlir/dialects/linalg/opdsl/lang/emitter.py
36

Should we support inputs of type Union[Value, Operation, OpView] as well?

Additionally I wonder if it would be easier to do the StructuredOpOuts to ValueList conversion right in dsl.py (https://github.com/llvm/llvm-project/blob/94e2b0258a176c7451dd8291cdf060ea048fee44/mlir/python/mlir/dialects/linalg/opdsl/lang/dsl.py#L47) where the arguments are handed over to the operation?

mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp
686–687

nit: singleElementAppendTemplate -> multiOperandAppendPackTemplate

722

nit: singleElementAppendTemplate -> singleResultAppendTemplate

ftynse updated this revision to Diff 377889.Oct 7 2021, 9:29 AM
ftynse marked 4 inline comments as done.

Address review.

For ops producing multiple results, would this effectively flatten all results into args?

gysit accepted this revision.Oct 7 2021, 9:38 AM

Thanks for these usability improvements!

This revision is now accepted and ready to land.Oct 7 2021, 9:38 AM
ftynse added a comment.Oct 7 2021, 9:44 AM

For ops producing multiple results, would this effectively flatten all results into args?

It would flatten them to one list. If the consumer of the values takes a single list of operands, it will be that. There are many cases where we have several variadic operand lists that are passed as independent arguments of the constructor. Each of those arguments may accept an op instead, but there is no intermixing between different lists. For example,

lbs = std.CallOp([IndexType.get()] * 3, "compute_lower_bounds", [])
ubs = std.CallOp([IndexType.get()] * 3, "compute_lower_bounds", [])
one = std.ConstantOp(IndexType.get(), 1)
scf.ParallelOp(lbs, ubs, [one] * 3)

each of lbs and ubs is an op that produces three values that are passed into individual groups of operands (or ODS-level arguments).