[all-commits] [llvm/llvm-project] a2288a: [mlir][python] remove mixins (#68853)

Maksim Levental via All-commits all-commits at lists.llvm.org
Thu Oct 19 14:20:27 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: a2288a8944c310fcad1196302f16513797e1fcbc
      https://github.com/llvm/llvm-project/commit/a2288a8944c310fcad1196302f16513797e1fcbc
  Author: Maksim Levental <maksim.levental at gmail.com>
  Date:   2023-10-19 (Thu, 19 Oct 2023)

  Changed paths:
    M mlir/docs/Bindings/Python.md
    M mlir/lib/Bindings/Python/Globals.h
    M mlir/lib/Bindings/Python/IRModule.cpp
    M mlir/lib/Bindings/Python/MainModule.cpp
    M mlir/python/CMakeLists.txt
    R mlir/python/mlir/dialects/_affine_ops_ext.py
    R mlir/python/mlir/dialects/_arith_ops_ext.py
    R mlir/python/mlir/dialects/_bufferization_ops_ext.py
    R mlir/python/mlir/dialects/_bufferization_transform_ops_ext.py
    R mlir/python/mlir/dialects/_builtin_ops_ext.py
    R mlir/python/mlir/dialects/_func_ops_ext.py
    R mlir/python/mlir/dialects/_gpu_transform_ops_ext.py
    R mlir/python/mlir/dialects/_linalg_ops_ext.py
    R mlir/python/mlir/dialects/_loop_transform_ops_ext.py
    R mlir/python/mlir/dialects/_memref_ops_ext.py
    R mlir/python/mlir/dialects/_memref_transform_ops_ext.py
    R mlir/python/mlir/dialects/_ml_program_ops_ext.py
    M mlir/python/mlir/dialects/_ods_common.py
    R mlir/python/mlir/dialects/_pdl_ops_ext.py
    R mlir/python/mlir/dialects/_scf_ops_ext.py
    R mlir/python/mlir/dialects/_structured_transform_ops_ext.py
    R mlir/python/mlir/dialects/_tensor_ops_ext.py
    R mlir/python/mlir/dialects/_tensor_transform_ops_ext.py
    R mlir/python/mlir/dialects/_transform_ops_ext.py
    R mlir/python/mlir/dialects/_transform_pdl_extension_ops_ext.py
    M mlir/python/mlir/dialects/affine.py
    M mlir/python/mlir/dialects/arith.py
    M mlir/python/mlir/dialects/bufferization.py
    M mlir/python/mlir/dialects/builtin.py
    M mlir/python/mlir/dialects/func.py
    M mlir/python/mlir/dialects/linalg/opdsl/lang/emitter.py
    M mlir/python/mlir/dialects/linalg/opdsl/ops/core_named_ops.py
    M mlir/python/mlir/dialects/memref.py
    M mlir/python/mlir/dialects/ml_program.py
    M mlir/python/mlir/dialects/pdl.py
    M mlir/python/mlir/dialects/python_test.py
    M mlir/python/mlir/dialects/scf.py
    M mlir/python/mlir/dialects/tensor.py
    M mlir/python/mlir/dialects/transform/__init__.py
    M mlir/python/mlir/dialects/transform/bufferization.py
    M mlir/python/mlir/dialects/transform/gpu.py
    M mlir/python/mlir/dialects/transform/loop.py
    M mlir/python/mlir/dialects/transform/memref.py
    M mlir/python/mlir/dialects/transform/pdl.py
    M mlir/python/mlir/dialects/transform/structured.py
    M mlir/python/mlir/dialects/transform/tensor.py
    M mlir/python/mlir/runtime/np_to_memref.py
    M mlir/test/python/dialects/arith_dialect.py
    M mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp

  Log Message:
  -----------
  [mlir][python] remove mixins (#68853)

This PR replaces the mixin `OpView` extension mechanism with the
standard inheritance mechanism.

Why? Firstly, mixins are not very pythonic (inheritance is usually used
for this), a little convoluted, and too "tight" (can only be used in the
immediately adjacent `_ext.py`). Secondly, it (mixins) are now blocking
are correct implementation of "value builders" (see
[here](https://github.com/llvm/llvm-project/pull/68764)) where the
problem becomes how to choose the correct base class that the value
builder should call.

This PR looks big/complicated but appearances are deceiving; 4 things
were needed to make this work:

1. Drop `skipDefaultBuilders` in
`OpPythonBindingGen::emitDefaultOpBuilders`
2. Former mixin extension classes are converted to inherit from the
generated `OpView` instead of being "mixins"
a. extension classes that simply were calling into an already generated
`super().__init__` continue to do so
b. (almost all) extension classes that were calling `self.build_generic`
because of a lack of default builder being generated can now also just
call `super().__init__`
3. To handle the [lone single
use-case](https://sourcegraph.com/search?q=context%3Aglobal+select_opview_mixin&patternType=standard&sm=1&groupBy=repo)
of `select_opview_mixin`, namely
[linalg](https://github.com/llvm/llvm-project/blob/main/mlir/python/mlir/dialects/_linalg_ops_ext.py#L38),
only a small change was necessary in `opdsl/lang/emitter.py` (thanks to
the emission/generation of default builders/`__init__`s)
4. since the `extend_opview_class` decorator is removed, we need a way
to register extension classes as the desired `OpView` that `op.opview`
conjures into existence; so we do the standard thing and just enable
replacing the existing registered `OpView` i.e.,
`register_operation(_Dialect, replace=True)`.

Note, the upgrade path for the common case is to change an extension to
inherit from the generated builder and decorate it with
`register_operation(_Dialect, replace=True)`. In the slightly more
complicated case where `super().__init(self.build_generic(...))` is
called in the extension's `__init__`, this needs to be updated to call
`__init__` in `OpView`, i.e., the grandparent (see updated docs). 
Note, also `<DIALECT>_ext.py` files/modules will no longer be automatically loaded.

Note, the PR has 3 base commits that look funny but this was done for
the purpose of tracking the line history of moving the
`<DIALECT>_ops_ext.py` class into `<DIALECT>.py` and updating (commit
labeled "fix").




More information about the All-commits mailing list