[mlir python] Add nanobind support for standalone dialects. #117922
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hawkinsp
requested review from
rupprecht,
keith,
ftynse,
makslevental and
stellaraccident
as code owners
llvmbot
added
mlir:python
|
@llvm/pr-subscribers-mlir Author: Peter Hawkins (hawkinsp) ChangesThis 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:
Notes:
Patch is 66.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117922.diff 25 Files Affected:
diff --git a/mlir/cmake/modules/AddMLIRPython.cmake b/mlir/cmake/modules/AddMLIRPython.cmake
index 7b91f43e2d57fd..afad430700bb1f 100644
--- a/mlir/cmake/modules/AddMLIRPython.cmake
+++ b/mlir/cmake/modules/AddMLIRPython.cmake
@@ -114,10 +114,11 @@ endfunction()
# EMBED_CAPI_LINK_LIBS: Dependent CAPI libraries that this extension depends
# on. These will be collected for all extensions and put into an
# aggregate dylib that is linked against.
+# PYTHON_BINDINGS_LIBRARY: Either pybind11 or nanobind.
function(declare_mlir_python_extension name)
cmake_parse_arguments(ARG
""
- "ROOT_DIR;MODULE_NAME;ADD_TO_PARENT"
+ "ROOT_DIR;MODULE_NAME;ADD_TO_PARENT;PYTHON_BINDINGS_LIBRARY"
"SOURCES;PRIVATE_LINK_LIBS;EMBED_CAPI_LINK_LIBS"
${ARGN})
@@ -126,15 +127,20 @@ function(declare_mlir_python_extension name)
endif()
set(_install_destination "src/python/${name}")
+ if(NOT ARG_PYTHON_BINDINGS_LIBRARY)
+ set(ARG_PYTHON_BINDINGS_LIBRARY "pybind11")
+ endif()
+
add_library(${name} INTERFACE)
set_target_properties(${name} PROPERTIES
# Yes: Leading-lowercase property names are load bearing and the recommended
# way to do this: https://gitlab.kitware.com/cmake/cmake/-/issues/19261
- EXPORT_PROPERTIES "mlir_python_SOURCES_TYPE;mlir_python_EXTENSION_MODULE_NAME;mlir_python_EMBED_CAPI_LINK_LIBS;mlir_python_DEPENDS"
+ EXPORT_PROPERTIES "mlir_python_SOURCES_TYPE;mlir_python_EXTENSION_MODULE_NAME;mlir_python_EMBED_CAPI_LINK_LIBS;mlir_python_DEPENDS;mlir_python_BINDINGS_LIBRARY"
mlir_python_SOURCES_TYPE extension
mlir_python_EXTENSION_MODULE_NAME "${ARG_MODULE_NAME}"
mlir_python_EMBED_CAPI_LINK_LIBS "${ARG_EMBED_CAPI_LINK_LIBS}"
mlir_python_DEPENDS ""
+ mlir_python_BINDINGS_LIBRARY "${ARG_PYTHON_BINDINGS_LIBRARY}"
)
# Set the interface source and link_libs properties of the target
@@ -223,12 +229,14 @@ function(add_mlir_python_modules name)
elseif(_source_type STREQUAL "extension")
# Native CPP extension.
get_target_property(_module_name ${sources_target} mlir_python_EXTENSION_MODULE_NAME)
+ get_target_property(_bindings_library ${sources_target} mlir_python_BINDINGS_LIBRARY)
# Transform relative source to based on root dir.
set(_extension_target "${modules_target}.extension.${_module_name}.dso")
add_mlir_python_extension(${_extension_target} "${_module_name}"
INSTALL_COMPONENT ${modules_target}
INSTALL_DIR "${ARG_INSTALL_PREFIX}/_mlir_libs"
OUTPUT_DIRECTORY "${ARG_ROOT_PREFIX}/_mlir_libs"
+ PYTHON_BINDINGS_LIBRARY ${_bindings_library}
LINK_LIBS PRIVATE
${sources_target}
${ARG_COMMON_CAPI_LINK_LIBS}
@@ -634,7 +642,7 @@ endfunction()
function(add_mlir_python_extension libname extname)
cmake_parse_arguments(ARG
""
- "INSTALL_COMPONENT;INSTALL_DIR;OUTPUT_DIRECTORY"
+ "INSTALL_COMPONENT;INSTALL_DIR;OUTPUT_DIRECTORY;PYTHON_BINDINGS_LIBRARY"
"SOURCES;LINK_LIBS"
${ARGN})
if(ARG_UNPARSED_ARGUMENTS)
@@ -644,9 +652,15 @@ function(add_mlir_python_extension libname extname)
# The actual extension library produces a shared-object or DLL and has
# sources that must be compiled in accordance with pybind11 needs (RTTI and
# exceptions).
- pybind11_add_module(${libname}
- ${ARG_SOURCES}
- )
+ if(NOT DEFINED ARG_PYTHON_BINDINGS_LIBRARY OR ARG_PYTHON_BINDINGS_LIBRARY STREQUAL "pybind11")
+ pybind11_add_module(${libname}
+ ${ARG_SOURCES}
+ )
+ elseif(ARG_PYTHON_BINDINGS_LIBRARY STREQUAL "nanobind")
+ nanobind_add_module(${libname}
+ ${ARG_SOURCES}
+ )
+ endif()
# The extension itself must be compiled with RTTI and exceptions enabled.
# Also, some warning classes triggered by pybind11 are disabled.
diff --git a/mlir/cmake/modules/MLIRDetectPythonEnv.cmake b/mlir/cmake/modules/MLIRDetectPythonEnv.cmake
index d3a98aaf6ffd17..6202c8745f0dba 100644
--- a/mlir/cmake/modules/MLIRDetectPythonEnv.cmake
+++ b/mlir/cmake/modules/MLIRDetectPythonEnv.cmake
@@ -20,6 +20,11 @@ macro(mlir_configure_python_dev_packages)
find_package(Python3 ${LLVM_MINIMUM_PYTHON_VERSION}
COMPONENTS Interpreter ${_python_development_component} REQUIRED)
+
+ # It's a little silly to detect Python a second time, but nanobind's cmake
+ # code looks for Python_ not Python3_.
+ find_package(Python ${LLVM_MINIMUM_PYTHON_VERSION}
+ COMPONENTS Interpreter ${_python_development_component} REQUIRED)
unset(_python_development_component)
message(STATUS "Found python include dirs: ${Python3_INCLUDE_DIRS}")
message(STATUS "Found python libraries: ${Python3_LIBRARIES}")
@@ -27,6 +32,9 @@ macro(mlir_configure_python_dev_packages)
mlir_detect_pybind11_install()
find_package(pybind11 2.10 CONFIG REQUIRED)
message(STATUS "Found pybind11 v${pybind11_VERSION}: ${pybind11_INCLUDE_DIR}")
+ mlir_detect_nanobind_install()
+ find_package(nanobind 2.2 CONFIG REQUIRED)
+ message(STATUS "Found nanobind v${nanobind_VERSION}: ${nanobind_INCLUDE_DIR}")
message(STATUS "Python prefix = '${PYTHON_MODULE_PREFIX}', "
"suffix = '${PYTHON_MODULE_SUFFIX}', "
"extension = '${PYTHON_MODULE_EXTENSION}")
@@ -56,3 +64,29 @@ function(mlir_detect_pybind11_install)
set(pybind11_DIR "${PACKAGE_DIR}" PARENT_SCOPE)
endif()
endfunction()
+
+
+# Detects a nanobind package installed in the current python environment
+# and sets variables to allow it to be found. This allows nanobind to be
+# installed via pip, which typically yields a much more recent version than
+# the OS install, which will be available otherwise.
+function(mlir_detect_nanobind_install)
+ if(nanobind_DIR)
+ message(STATUS "Using explicit nanobind cmake directory: ${nanobind_DIR} (-Dnanobind_DIR to change)")
+ else()
+ message(STATUS "Checking for nanobind in python path...")
+ execute_process(
+ COMMAND "${Python3_EXECUTABLE}"
+ -c "import nanobind;print(nanobind.cmake_dir(), end='')"
+ WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+ RESULT_VARIABLE STATUS
+ OUTPUT_VARIABLE PACKAGE_DIR
+ ERROR_QUIET)
+ if(NOT STATUS EQUAL "0")
+ message(STATUS "not found (install via 'pip install nanobind' or set nanobind_DIR)")
+ return()
+ endif()
+ message(STATUS "found (${PACKAGE_DIR})")
+ set(nanobind_DIR "${PACKAGE_DIR}" PARENT_SCOPE)
+ endif()
+endfunction()
diff --git a/mlir/docs/Bindings/Python.md b/mlir/docs/Bindings/Python.md
index 6e52c4deaad9aa..601876280eb4e2 100644
--- a/mlir/docs/Bindings/Python.md
+++ b/mlir/docs/Bindings/Python.md
@@ -1138,12 +1138,14 @@ attributes and types must connect to the relevant C APIs for building and
inspection, which must be provided first. Bindings for `Attribute` and `Type`
subclasses can be defined using
[`include/mlir/Bindings/Python/PybindAdaptors.h`](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Bindings/Python/PybindAdaptors.h)
-utilities that mimic pybind11 API for defining functions and properties. These
-bindings are to be included in a separate pybind11 module. The utilities also
-provide automatic casting between C API handles `MlirAttribute` and `MlirType`
-and their Python counterparts so that the C API handles can be used directly in
-binding implementations. The methods and properties provided by the bindings
-should follow the principles discussed above.
+or
+[`include/mlir/Bindings/Python/NanobindAdaptors.h`](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Bindings/Python/NanobindAdaptors.h)
+utilities that mimic pybind11/nanobind API for defining functions and
+properties. These bindings are to be included in a separate module. The
+utilities also provide automatic casting between C API handles `MlirAttribute`
+and `MlirType` and their Python counterparts so that the C API handles can be
+used directly in binding implementations. The methods and properties provided by
+the bindings should follow the principles discussed above.
The attribute and type bindings for a dialect can be located in
`lib/Bindings/Python/Dialect<Name>.cpp` and should be compiled into a separate
@@ -1179,7 +1181,9 @@ make the passes available along with the dialect.
Dialect functionality other than IR objects or passes, such as helper functions,
can be exposed to Python similarly to attributes and types. C API is expected to
exist for this functionality, which can then be wrapped using pybind11 and
-`[include/mlir/Bindings/Python/PybindAdaptors.h](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Bindings/Python/PybindAdaptors.h)`
+`[include/mlir/Bindings/Python/PybindAdaptors.h](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Bindings/Python/PybindAdaptors.h)`,
+or nanobind and
+`[include/mlir/Bindings/Python/NanobindAdaptors.h](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Bindings/Python/NanobindAdaptors.h)`
utilities to connect to the rest of Python API. The bindings can be located in a
-separate pybind11 module or in the same module as attributes and types, and
+separate module or in the same module as attributes and types, and
loaded along with the dialect.
diff --git a/mlir/examples/standalone/python/CMakeLists.txt b/mlir/examples/standalone/python/CMakeLists.txt
index a8c43827a5a375..21c8285a451795 100644
--- a/mlir/examples/standalone/python/CMakeLists.txt
+++ b/mlir/examples/standalone/python/CMakeLists.txt
@@ -17,18 +17,32 @@ declare_mlir_dialect_python_bindings(
ROOT_DIR "${CMAKE_CURRENT_SOURCE_DIR}/mlir_standalone"
TD_FILE dialects/StandaloneOps.td
SOURCES
- dialects/standalone.py
+ dialects/standalone_pybind11.py
+ dialects/standalone_nanobind.py
DIALECT_NAME standalone)
-declare_mlir_python_extension(StandalonePythonSources.Extension
- MODULE_NAME _standaloneDialects
+
+declare_mlir_python_extension(StandalonePythonSources.Pybind11Extension
+ MODULE_NAME _standaloneDialectsPybind11
+ ADD_TO_PARENT StandalonePythonSources
+ SOURCES
+ StandaloneExtensionPybind11.cpp
+ EMBED_CAPI_LINK_LIBS
+ StandaloneCAPI
+ PYTHON_BINDINGS_LIBRARY pybind11
+)
+
+declare_mlir_python_extension(StandalonePythonSources.NanobindExtension
+ MODULE_NAME _standaloneDialectsNanobind
ADD_TO_PARENT StandalonePythonSources
SOURCES
- StandaloneExtension.cpp
+ StandaloneExtensionNanobind.cpp
EMBED_CAPI_LINK_LIBS
StandaloneCAPI
+ PYTHON_BINDINGS_LIBRARY nanobind
)
+
################################################################################
# Common CAPI
################################################################################
@@ -62,3 +76,4 @@ add_mlir_python_modules(StandalonePythonModules
COMMON_CAPI_LINK_LIBS
StandalonePythonCAPI
)
+
\ No newline at end of file
diff --git a/mlir/examples/standalone/python/StandaloneExtensionNanobind.cpp b/mlir/examples/standalone/python/StandaloneExtensionNanobind.cpp
new file mode 100644
index 00000000000000..6d83dc585dcd1d
--- /dev/null
+++ b/mlir/examples/standalone/python/StandaloneExtensionNanobind.cpp
@@ -0,0 +1,35 @@
+//===- StandaloneExtension.cpp - Extension module -------------------------===//
+//
+// This is the nanobind version of the example module. There is also a pybind11
+// example in StandaloneExtensionPybind11.cpp.
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <nanobind/nanobind.h>
+
+#include "Standalone-c/Dialects.h"
+#include "mlir/Bindings/Python/NanobindAdaptors.h"
+
+namespace nb = nanobind;
+
+NB_MODULE(_standaloneDialectsNanobind, m) {
+ //===--------------------------------------------------------------------===//
+ // standalone dialect
+ //===--------------------------------------------------------------------===//
+ auto standaloneM = m.def_submodule("standalone");
+
+ standaloneM.def(
+ "register_dialect",
+ [](MlirContext context, bool load) {
+ MlirDialectHandle handle = mlirGetDialectHandle__standalone__();
+ mlirDialectHandleRegisterDialect(handle, context);
+ if (load) {
+ mlirDialectHandleLoadDialect(handle, context);
+ }
+ },
+ nb::arg("context").none() = nb::none(), nb::arg("load") = true);
+}
diff --git a/mlir/examples/standalone/python/StandaloneExtension.cpp b/mlir/examples/standalone/python/StandaloneExtensionPybind11.cpp
similarity index 81%
rename from mlir/examples/standalone/python/StandaloneExtension.cpp
rename to mlir/examples/standalone/python/StandaloneExtensionPybind11.cpp
index 5e83060cd48d82..397db4c20e7432 100644
--- a/mlir/examples/standalone/python/StandaloneExtension.cpp
+++ b/mlir/examples/standalone/python/StandaloneExtensionPybind11.cpp
@@ -1,4 +1,7 @@
-//===- StandaloneExtension.cpp - Extension module -------------------------===//
+//===- StandaloneExtensionPybind11.cpp - Extension module -----------------===//
+//
+// This is the pybind11 version of the example module. There is also a nanobind
+// example in StandaloneExtensionNanobind.cpp.
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -11,7 +14,7 @@
using namespace mlir::python::adaptors;
-PYBIND11_MODULE(_standaloneDialects, m) {
+PYBIND11_MODULE(_standaloneDialectsPybind11, m) {
//===--------------------------------------------------------------------===//
// standalone dialect
//===--------------------------------------------------------------------===//
diff --git a/mlir/examples/standalone/python/mlir_standalone/dialects/standalone.py b/mlir/examples/standalone/python/mlir_standalone/dialects/standalone_nanobind.py
similarity index 78%
rename from mlir/examples/standalone/python/mlir_standalone/dialects/standalone.py
rename to mlir/examples/standalone/python/mlir_standalone/dialects/standalone_nanobind.py
index c958b2ac193682..6218720951c82a 100644
--- a/mlir/examples/standalone/python/mlir_standalone/dialects/standalone.py
+++ b/mlir/examples/standalone/python/mlir_standalone/dialects/standalone_nanobind.py
@@ -3,4 +3,4 @@
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
from ._standalone_ops_gen import *
-from .._mlir_libs._standaloneDialects.standalone import *
+from .._mlir_libs._standaloneDialectsNanobind.standalone import *
diff --git a/mlir/examples/standalone/python/mlir_standalone/dialects/standalone_pybind11.py b/mlir/examples/standalone/python/mlir_standalone/dialects/standalone_pybind11.py
new file mode 100644
index 00000000000000..bfb98e404e13f2
--- /dev/null
+++ b/mlir/examples/standalone/python/mlir_standalone/dialects/standalone_pybind11.py
@@ -0,0 +1,6 @@
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+from ._standalone_ops_gen import *
+from .._mlir_libs._standaloneDialectsPybind11.standalone import *
diff --git a/mlir/examples/standalone/test/python/smoketest.py b/mlir/examples/standalone/test/python/smoketest.py
index 08e08cbd2fe24c..3872cd6f6fb9e0 100644
--- a/mlir/examples/standalone/test/python/smoketest.py
+++ b/mlir/examples/standalone/test/python/smoketest.py
@@ -1,7 +1,17 @@
-# RUN: %python %s | FileCheck %s
+# RUN: %python %s pybind11 | FileCheck %s
+# RUN: %python %s nanobind | FileCheck %s
+import sys
from mlir_standalone.ir import *
-from mlir_standalone.dialects import builtin as builtin_d, standalone as standalone_d
+from mlir_standalone.dialects import builtin as builtin_d
+
+if sys.argv[1] == "pybind11":
+ from mlir_standalone.dialects import standalone_pybind11 as standalone_d
+elif sys.argv[1] == "nanobind":
+ from mlir_standalone.dialects import standalone_nanobind as standalone_d
+else:
+ raise ValueError("Expected either pybind11 or nanobind as arguments")
+
with Context():
standalone_d.register_dialect()
diff --git a/mlir/include/mlir/Bindings/Python/Diagnostics.h b/mlir/include/mlir/Bindings/Python/Diagnostics.h
new file mode 100644
index 00000000000000..ecda96e5243b99
--- /dev/null
+++ b/mlir/include/mlir/Bindings/Python/Diagnostics.h
@@ -0,0 +1,59 @@
+//===- Diagnostics.h - Helpers for diagnostics in Python bindings ---------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_BINDINGS_PYTHON_DIAGNOSTICS_H
+#define MLIR_BINDINGS_PYTHON_DIAGNOSTICS_H
+
+#include <cassert>
+#include <string>
+
+#include "llvm/ADT/StringRef.h"
+#include "mlir-c/Diagnostics.h"
+#include "mlir-c/IR.h"
+
+namespace mlir {
+namespace python {
+
+/// RAII scope intercepting all diagnostics into a string. The message must be
+/// checked before this goes out of scope.
+class CollectDiagnosticsToStringScope {
+ public:
+ explicit CollectDiagnosticsToStringScope(MlirContext ctx) : context(ctx) {
+ handlerID = mlirContextAttachDiagnosticHandler(ctx, &handler, &errorMessage,
+ /*deleteUserData=*/nullptr);
+ }
+ ~CollectDiagnosticsToStringScope() {
+ assert(errorMessage.empty() && "unchecked error message");
+ mlirContextDetachDiagnosticHandler(context, handlerID);
+ }
+
+ [[nodiscard]] std::string takeMessage() { return std::move(errorMessage); }
+
+ private:
+ static MlirLogicalResult handler(MlirDiagnostic diag, void *data) {
+ auto printer = +[](MlirStringRef message, void *data) {
+ *static_cast<std::string *>(data) +=
+ llvm::StringRef(message.data, message.length);
+ };
+ MlirLocation loc = mlirDiagnosticGetLocation(diag);
+ *static_cast<std::string *>(data) += "at ";
+ mlirLocationPrint(loc, printer, data);
+ *static_cast<std::string *>(data) += ": ";
+ mlirDiagnosticPrint(diag, printer, data);
+ return mlirLogicalResultSuccess();
+ }
+
+ MlirContext context;
+ MlirDiagnosticHandlerID handlerID;
+ std::string errorMessage = "";
+};
+
+} // namespace python
+} // namespace mlir
+
+#endif // MLIR_BINDINGS_PYTHON_DIAGNOSTICS_H
diff --git a/mlir/include/mlir/Bindings/Python/NanobindAdaptors.h b/mlir/include/mlir/Bindings/Python/NanobindAdaptors.h
new file mode 100644
index 00000000000000..e62ea19f0f00eb
--- /dev/null
+++ b/mlir/include/mlir/Bindings/Python/NanobindAdaptors.h
@@ -0,0 +1,668 @@
+//===- NanobindAdaptors.h - Interop with MLIR APIs via nanobind -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// This file contains adaptors for clients of the core MLIR Python APIs to
+// interop via MLIR CAPI types, using nanobind. The facilities here do not
+// depend on implementation details of the MLIR Python API and do not introduce
+// C++-level dependencies with it (requiring only Python and CAPI-level
+// dependencies).
+//
+// It is encouraged to be used both in-tree and out-of-tree. For in-tree use
+// cases, it should be used for dialect implementations (versus relying on
+// Pybind-based internals of the core libraries).
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_BINDINGS_PYTHON_NANOBINDADAPTORS_H
+#define MLIR_BINDINGS_PYTHON_NANOBINDADAPTORS_H
+
+#include <nanobind/nanobind.h>
+#include <nanobind/stl/string.h>
+
+#include <cstdint>
+
+#include "llvm/ADT/Twine.h"
+#include "mlir-c/Bindings/Python/Interop.h"
+#include "mlir-c/Diagnostics.h"
+#include "mlir-c/IR.h"
+
+// Raw CAPI type casters need to be declared before use, so always include them
+// first.
+namespace nanobind {
+namespace detail {
+
+/// Helper to convert a presumed MLIR API object to a capsule, accepting either
+/// an explicit Capsule (which can happen when two C APIs are communicating
+/// directly via Python) or indirectly by querying the MLIR_PYTHON_CAPI_PTR_ATTR
+/// attribute (through which supp...
[truncated]
|
|
✅ With the latest revision this PR passed the Python code formatter. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
hawkinsp
force-pushed
the
nb
branch
2 times, most recently
from
999eab2 to
65acc72
Compare
|
Oh man, hit me with this four hours before I'm planning a two week vacation :) I'll take a look now. Generally supportive. Also supportive of migrating the whole to nanobind given that it is such better performance and ergonomics, but as you say -- a topic for a different day. |
There was a problem hiding this comment.
If history is a guide, this may be challenging to land because of build bot deps needing to be updated. Will see what the bots say about that.
One practical thing: every time we touch MLIRDetectPythonEnv, it causes a lot of pain and suffering downstream, and this is going to be another dep change/expansion that will trigger that. I think the way out in those cases may just be to add a CMake option to have MLIR inherit the python configuration from a super-project vs trying to probe and configure everything itself. Then such projects can set that option and just do it themselves vs fighting with MLIR over how to do the detection.
I'm going to fiddle with this now as it has been on the agenda for a long time.
|
|
||
| # It's a little silly to detect Python a second time, but nanobind's cmake | ||
| # code looks for Python_ not Python3_. | ||
| find_package(Python ${LLVM_MINIMUM_PYTHON_VERSION} |
There was a problem hiding this comment.
Agreed with the comment. I've run into this too but have also just done the pragmatic thing. Might be worth filing an issue in nanobind for guidance.
|
Please add |
Already there. BTW, is there documentation on how to run all the MLIR Python tests? Apparently CI is running some I didn't find. I looked at https://mlir.llvm.org/docs/Bindings/Python/#testing for guidance but to no avail. |
stellaraccident
added a commit
that referenced
this pull request
Nov 27, 2024
Adds a CMake option MLIR_CONFIGURE_PYTHON_DEV_PACKAGES which gates doing package discovery and configuration for Python dev packages by MLIR. The default Python setup that MLIR does has been a problem for super-projects that include LLVM for a long time because it forces a very specific package discovery mechanism that is not uniform in all uses. When reviewing #117922, I noted that this would effectively be a break the world event for downstreams, forcing them to adapt their nanobind dep to the exact way that MLIR does it. Adding the option to just wholesale skip the built-in configuration heuristics at least gives us a mechanism to tell downstreams to migrate to, giving them complete control and not requiring packaging workarounds. This seemed a better option than (once again) creating a situation where downstreams could not integrate the dep change without doing tricky infra upgrades, and it removes the burden from the author of that patch from needing to think about how this affects super-projects that include MLIR (i.e. they can just be told to do it themselves as needed vs being in a wedged state and unable to upgrade).
|
Mind having a look at: #117934 If we land that, I'll send out a PSA to users to enable the new option if they have a project that has an opinionated setup of their Python dev packages and give us one less thing to worry about landing this one. |
I believe it just happens by default. I see the python unit tests in the output from the linux-linux-x64 bot for example (https://buildkite.com/llvm-project/github-pull-requests/builds/123898#01936fb2-3b65-489a-9fc0-c29a7f5f6db8) |
Would appear to be setup to do the right thing: https://github.com/llvm/llvm-project/blob/main/.ci/monolithic-linux.sh#L48 It is failing (on the test you changed): |
stellaraccident
added a commit
that referenced
this pull request
Nov 27, 2024
Adds a CMake option MLIR_CONFIGURE_PYTHON_DEV_PACKAGES which gates doing package discovery and configuration for Python dev packages by MLIR. The default Python setup that MLIR does has been a problem for super-projects that include LLVM for a long time because it forces a very specific package discovery mechanism that is not uniform in all uses. When reviewing #117922, I noted that this would effectively be a break the world event for downstreams, forcing them to adapt their nanobind dep to the exact way that MLIR does it. Adding the option to just wholesale skip the built-in configuration heuristics at least gives us a mechanism to tell downstreams to migrate to, giving them complete control and not requiring packaging workarounds. This seemed a better option than (once again) creating a situation where downstreams could not integrate the dep change without doing tricky infra upgrades, and it removes the burden from the author of that patch from needing to think about how this affects super-projects that include MLIR (i.e. they can just be told to do it themselves as needed vs being in a wedged state and unable to upgrade).
|
I'm going to need to hand you over to @makslevental to get this in next week, but this looks reasonable to me. I think that we should land #117934 first and possibly send out a note telling people with non trivial super projects to use that option. That would require some minor indent-rebasing with this patch. |
|
I don't think the bots are using the requirements.txt from the repo still (unfortunately), we'll likely have to manually re-deploy images instead. |
I thought that as well, but it appears to be: https://github.com/llvm/llvm-project/blob/main/.ci/monolithic-linux.sh#L48 Is there somewhere else to look? |
|
This is the pre-merge CI, which runs on the PR right now. I was referring to the various buildbot which runs post-merge, the configs are defined in https://github.com/llvm/llvm-zorg/ |
Ah. I knew I was remembering something. Well, I'm about to be offline/vacation so will leave that to next week. |
hawkinsp
added a commit
to hawkinsp/llvm-project
that referenced
this pull request
Dec 14, 2024
This is a companion to llvm#118583, although it can be landed independently because since llvm#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. It would make sense to merge this PR after merging llvm#118583, if we have agreed that we are migrating the core to nanobind. This PR also: * removes CollectDiagnosticsToStringScope from NanobindAdaptors.h. This was overlooked in a previous PR and it is duplicated in Diagnostics.h.
From packaging for Linux distributions, I agree that there is a strong need to only consume dependencies that are packaged for the OS. However, as long as one can disable |
|
Right -- the version of nanobind packaged by the distro is only really a concern for the MLIR bindings packaged by the distro. People building MLIR themselves are free to install a newer version, e.g. using pip and requirements.txt. |
hawkinsp
added a commit
to hawkinsp/llvm-project
that referenced
this pull request
Dec 16, 2024
This is a companion to llvm#118583, although it can be landed independently because since llvm#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. It would make sense to merge this PR after merging llvm#118583, if we have agreed that we are migrating the core to nanobind. This PR also: * removes CollectDiagnosticsToStringScope from NanobindAdaptors.h. This was overlooked in a previous PR and it is duplicated in Diagnostics.h. * removes some extraneous semicolons in NanobindAdaptors.h
copybara-service bot
pushed a commit
to jax-ml/jax
that referenced
this pull request
Dec 17, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 17, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Dec 17, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 17, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 17, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Dec 17, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 17, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Dec 17, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 705197067
copybara-service bot
pushed a commit
to tensorflow/mlir-hlo
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 707537037
copybara-service bot
pushed a commit
to openxla/shardy
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 707537037
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 707537037
copybara-service bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Dec 18, 2024
I'm working towards moving the MLIR Python core code to use nanobind instead of pybind11: * llvm/llvm-project#117922, which was merged recently, allows downstream Python dialect extensions to be defined using either pybind11 or nanobind. * llvm/llvm-project#118583 is a PR in review that ports the Python core code to use nanobind instead of pybind11. This PR migrates StableHLO and related dialects to use nanobind rather than pybind11, with the goal of migrating JAX away from pybind11. PiperOrigin-RevId: 707537037
hawkinsp
added a commit
to hawkinsp/llvm-project
that referenced
this pull request
Dec 19, 2024
This is a companion to llvm#118583, although it can be landed independently because since llvm#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. It would make sense to merge this PR after merging llvm#118583, if we have agreed that we are migrating the core to nanobind. This PR also: * removes CollectDiagnosticsToStringScope from NanobindAdaptors.h. This was overlooked in a previous PR and it is duplicated in Diagnostics.h.
hawkinsp
added a commit
to hawkinsp/llvm-project
that referenced
this pull request
Dec 20, 2024
This is a companion to llvm#118583, although it can be landed independently because since llvm#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. It would make sense to merge this PR after merging llvm#118583, if we have agreed that we are migrating the core to nanobind. This PR also: * removes CollectDiagnosticsToStringScope from NanobindAdaptors.h. This was overlooked in a previous PR and it is duplicated in Diagnostics.h.
jpienaar
added a commit
to hawkinsp/llvm-project
that referenced
this pull request
Dec 21, 2024
This is a companion to llvm#118583, although it can be landed independently because since llvm#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. It would make sense to merge this PR after merging llvm#118583, if we have agreed that we are migrating the core to nanobind. 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 <[email protected]>
jpienaar
added a commit
that referenced
this pull request
Dec 21, 2024
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 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
Notes: