[llvm] [mlir] [mlir python] Add nanobind support for standalone dialects. (PR #117922)
Peter Hawkins via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 2 12:27:08 PST 2024
hawkinsp wrote:
> @hawkinsp basically looks good to me; two questions (one load bearing, one curiosity):
>
> 1. We currently (in the pybind impl) have `module_local` in lots of places. nanobind doesn't have `module_local` and instead uses `NB_DOMAIN` (which maybe works the [same](https://github.com/wjakob/nanobind/blob/master/include/nanobind/nb_defs.h#L190) [way](https://github.com/wjakob/nanobind/blob/master/src/nb_internals.cpp#L384-L385)???). Do we want to support that?
Well, this is a choice for whoever builds these extensions. In the most recent push I added an `NB_DOMAIN` of `mlir` to the CMake rules in this PR, which perhaps achieves what you want, i.e., a unique namespace for the upstream mlir bindings, when built in their default configuration?
It doesn't really matter at the moment because the only artifacts built with nanobind are the standalone example and the python test dialect, but it will matter in the future if we build, e.g., the ODS artifacts with nanobind.
> 2. Do the added bazel functions work in OSS? Possibly the question is ill-posed because scanning through the bazel I don't see a problematic rule that I thought I remembered seeing (`pybind_extension`) but I'm actually asking on behalf of users like @j2kun who have had trouble using python bindings bazel infra for OSS projects.
I verified:
```
bazel build --config=generic_clang @llvm-project//...
```
works with this PR included in OSS.
`pybind_extension` is frequently misnamed. To build a pybind11 extension, you need basically to build a shared library with: (a) to enable exceptions, if they aren't, (b) to depend on pybind11. Nanobind is exactly the same, just depend on nanobind instead of pybind11.
https://github.com/llvm/llvm-project/pull/117922
More information about the llvm-commits
mailing list