In https://github.com/llvm/llvm-project/pull/155114 we removed
`liveOperations` but forgot this line which was being used to invalidate
operations under a transform root, which currently isn't being used for
anything. So remove.
FYI this led to a subtle double free bug after
https://github.com/llvm/llvm-project/pull/175405:
```python
@test_in_context
def check_builtin():
module = builtin_d.ModuleOp()
with module.context, ir.Location.unknown():
transform_module = builtin_d.Module.create()
transform_module.operation.attributes["transform.with_named_sequence"] = (
ir.UnitAttr.get()
)
with ir.InsertionPoint(transform_module.body):
named_sequence = NamedSequenceOp("__transform_main", [any_op_t()], [])
with ir.InsertionPoint(named_sequence.body):
YieldOp([])
interp.apply_named_sequence(
module,
transform_module.body.operations[0],
transform_module,
)
```
with error
```
python(7436,0x1f95a93c0) malloc: *** error for object 0x6000002b0000: pointer being freed was not allocated
python(7436,0x1f95a93c0) malloc: *** set a breakpoint in malloc_error_break to debug
```
This is because
```
nb::object obj = nb::cast(payloadRoot);
```
is actually equivalent to
```
nb::object obj = nb::cast(payloadRoot, nb::rv_policy::automatic);
```
which is actually equivalent to
```
nb::object obj = nb::cast(payloadRoot, nb::rv_policy::copy);
```
because I changed the API to `PyOperationBase &payloadRoot` i.e., an
lvalue reference and `nb::rv_policy::automatic` decays to
`nb::rv_policy::copy` for [lvalue
refs](https://nanobind.readthedocs.io/en/latest/api_core.html#_CPPv4N8nanobind9rv_policy9automaticE).
Fix some dialect bindings I missed in https://github.com/llvm/llvm-project/pull/174156 so they don't bind C structs (because that leads to multiple registration in the case when multiple packages are used simultaneously).
If we do not collect the diagnostics from the
CollectDiagnosticsToStringScope, even when the named_sequence applied
successfully, the Scope object's destructor will assert (with a
unhelpful message).
Historical context: `PyMlirContext::liveOperations` was an optimization
meant to cut down on the number of Python object allocations and
(partially) a mechanism for updating validity of ops after
transformation. E.g. during walking/transforming the AST. See original
patch [here](https://reviews.llvm.org/D87958).
Inspired by a
[renewed](https://github.com/llvm/llvm-project/pull/139721#issuecomment-3217131918)
interest in https://github.com/llvm/llvm-project/pull/139721 (which has
become a little stale...)
<p align="center">
<img width="504" height="375" alt="image"
src="https://github.com/user-attachments/assets/0daad562-d3d1-4876-8d01-5dba382ab186"
/>
</p>
In the previous go-around
(https://github.com/llvm/llvm-project/pull/92631) there were two issues
which have been resolved
1. ops that were "fetched" under a root op which has been transformed
are no longer reported as invalid. We simply "[formally
forbid](https://github.com/llvm/llvm-project/pull/92631#issuecomment-2119397018)"
this;
2. `Module._CAPICreate(module_capsule)` must now be followed by a
`module._clear_mlir_module()` to prevent double-freeing of the actual
`ModuleOp` object (i.e. calling the dtor on the
`OwningOpRef<ModuleOp>`):
```python
module = ...
module_dup = Module._CAPICreate(module._CAPIPtr)
module._clear_mlir_module()
```
- **the alternative choice** here is to remove the `Module._CAPICreate`
API altogether and replace it with something like `Module._move(module)`
which will do both `Module._CAPICreate` and `module._clear_mlir_module`.
Note, the other approach I explored last year was a [weakref
system](https://github.com/llvm/llvm-project/pull/97340) for
`mlir::Operation` which would effectively hoist this `liveOperations`
thing into MLIR core. Possibly doable but I now believe it's a bad idea.
The other potentially breaking change is `is`, which checks object
equality rather than value equality, will now report `False` because we
are always allocating `new` Python objects (ie that's the whole point of
this change). Users wanting to check equality for `Operation` and
`Module` should use `==`.
This is a companion to #118583, although it can be landed independently
because since #117922 dialects do not have to use the same Python
binding framework as the Python core code.
This PR ports all of the in-tree dialect and pass extensions to
nanobind, with the exception of those that remain for testing pybind11
support.
This PR also:
* removes CollectDiagnosticsToStringScope from NanobindAdaptors.h. This
was overlooked in a previous PR and it is duplicated in Diagnostics.h.
---------
Co-authored-by: Jacques Pienaar <jpienaar@google.com>
This PR allows out-of-tree dialects to write Python dialect modules
using nanobind instead of pybind11.
It may make sense to migrate in-tree dialects and some of the ODS Python
infrastructure to nanobind, but that is a topic for a future change.
This PR makes the following changes:
* adds nanobind to the CMake and Bazel build systems. We also add
robin_map to the Bazel build, which is a dependency of nanobind.
* adds a PYTHON_BINDING_LIBRARY option to various CMake functions, such
as declare_mlir_python_extension, allowing users to select a Python
binding library.
* creates a fork of mlir/include/mlir/Bindings/Python/PybindAdaptors.h
named NanobindAdaptors.h. This plays the same role, using nanobind
instead of pybind11.
* splits CollectDiagnosticsToStringScope out of PybindAdaptors.h and
into a new header mlir/include/mlir/Bindings/Python/Diagnostics.h, since
it is code that is no way related to pybind11 or for that matter,
Python.
* changed the standalone Python extension example to have both pybind11
and nanobind variants.
* changed mlir/python/mlir/dialects/python_test.py to have both pybind11
and nanobind variants.
Notes:
* A slightly unfortunate thing that I needed to do in the CMake
integration was to use FindPython in addition to FindPython3, since
nanobind's CMake integration expects the Python_ names for variables.
Perhaps there's a better way to do this.
