[Mlir-commits] [mlir] [mlir][python] Make the Context/Operation capsule creation methods work as documented. (PR #76010)

Maksim Levental llvmlistbot at llvm.org
Thu Dec 21 02:39:01 PST 2023


makslevental wrote:

> so that it can interpret `MlirOperation` as reference when the corresponding operation itself (and maybe one of its ancestors) is already owned by python or (b) somehow providing a mechanism for downstream binding implementers to indicate whether they intend to transfer ownership.

I see 

```cpp
static handle cast(MlirOperation v, return_value_policy, handle) {
  return py::module::import(MAKE_MLIR_PYTHON_QUALNAME("ir"))
        .attr("Operation")
        .attr(MLIR_PYTHON_CAPI_FACTORY_ATTR)(capsule)
        .release();
}
```

which could be changed to

```cpp
static handle cast(MlirOperation v, return_value_policy p, handle h) {
  // call MLIR_PYTHON_CAPI_FACTORY_ATTR somehow differently depending on p/h
}
```

(`h` is the parent object). 

This would enable downstream users to 

```
.def("_get_first_in_block", [](MlirOperation op) -> MlirOperation {
  MlirBlock block = mlirOperationGetBlock(op);
  MlirOperation first = mlirBlockGetFirstOperation(block);
  return py::cast(first, return_value_policy::reference);
})
```
in order to communicate intent.

But I don't actually know what `// call MLIR_PYTHON_CAPI_FACTORY_ATTR somehow differently depending on p/h` should be - the call to `_CAPICreate` is the important piece and it's a call through the python API (which we don't want to expand to support something like a `return_value_policy` arg?). 

> Upstream doesn't have a problem because it can, and does, return a `PyOpreationRef`.

I'll take this opportunity to beat my favorite dead horse: we could just start exposing the utilities in `lib/Bindings`, such as `PyOpreationRef`, in order to avoid the inherent difficulties of relying on only `PybindAdaptors.h` to always do the right things.

https://github.com/llvm/llvm-project/pull/76010


More information about the Mlir-commits mailing list