llvm / llvm/llvm-project

[MLIR][Python] Tracking: Finalize design and breaking changes before EuroLLVM for Python-defined dialects

Open
#189,318 2 comments 1 reaction 2 assignees Claimed by @PragmaTwice View on GitHub
mlir:python
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

Since breaking changes may become difficult to implement following the Python-defined dialect presentation at EuroLLVM, we need to review the design and implementation of the Python-defined dialect beforehand to avoid introducing unnecessary breaking changes in the future due to design flaws.

Below are several aspects that may need to be considered (items may be added incrementally):
- [ ] region fields in operations:
- [ ] currently it doesn't affect `__init__` parameters. is it reasonable?
- [ ] no field specifiers for region fields
- [ ] generic type parameters for `Region`?
- [ ] allow specifying number of blocks as an argument on the field specifier
- [ ] is current field specifiers fully compatible to `dataclass_transform`?
- [ ] variadic (operand/result/attribute) fields:
- [ ] could we use `= None` to initialize an empty list for them?
- [ ] operations with generic types: is the current design sound?
- [ ] is `irdl.all_of` useful? should we add it to the current constraint system?
- [ ] how to handle the implicit `TypeAttr` problem from IRDL?
- [ ] is field specifiers useful in type/attribute definitions?
- [ ] Should we use/allow class-based Pass implementations in Python?
- [ ] Should we use/allow class-based RewritePattern implementations in Python?
- [ ] Allow all op names that MLIR allows -- IRDL issue: https://github.com/llvm/llvm-project/pull/187911 (Are all dialect names supported?)
- [ ] Fix needing to pass `[()]` to ext.Result, e.g. `ext.Result[transform.AnyParamType[()]]`.

Further off changes:
- [ ] Enable multiple inheritance so interfaces (and traits) can be implemented on the op def class directly
- [ ] As nanobind prohibits multiple inheritance, we would need a hack, like proxy classes (which also have a performance impact)
- [ ] Support custom `__init__` on `ext.Operation` subclasses

Discussion points:
- [ ] Is `class MyOp(MyDialect.Operation, name='...'): pass` the correct way to add `MyOp` to the `MyDialect` dialect?
- [ ] E.g., with `@MyDialect.register_operation` as a decorator on `class MyOp(ext.Operation, name='...'): pass` it is clear that the `MyDialect` object is likely to track that an op has been added.
- [ ] Also allows for an op to be added to multiple dialects, in so far as that is useful

cc @rolfmorel

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.