[mlir] Limit Python dependency to Development.Module when possible.

[mikeurbach]

Summary

After CMake 3.18, we are able to limit the scope of the search to just
Development.Module. Searching for Development will fail in situations
where the Python libraries are not available. When possible, limit to
just Development.Module. See:
https://pybind11.readthedocs.io/en/stable/compiling.html#findpython-mode

[mikeurbach]

Take two. I tested this one locally and I think it should work. Do the buildbots run after commit to main or is there anywhere I can check before committing?

mlir/CMakeLists.txt

110

There are different ways to address this, but predicating this with OR MLIR_BINDINGS_PYTHON_LOCK_VERSION seems like an appropriate use of the existing option. Curious what you think @stellaraccident .

[stellaraccident]

mlir/CMakeLists.txt

110

I think the right thing is to just delete MLIR_BINDINGS_PYTHON_LOCK_VERSION and retrofit add_mlir_python_extension (from AddMLIRPython.cmake) with the version I have in this patch: https://reviews.llvm.org/D111513

If this gets us an easily revertible baby step towards that, then I think my next step would be to pick just the changes to add_mlir_python_extension into a standalone path and nuke MLIR_BINDINGS_PYTHON_LOCK_VERSION entirely (it was really only in that patch because I needed to adjust some unrelated things for building the Standalone project).

So if this works like this, I'm fine with it as a next step.

[mikeurbach]

mlir/CMakeLists.txt

110

Let me take a deeper look at your patch, I owe a proper review there.

[mikeurbach]

Get rid of MLIR_BINDINGS_PYTHON_LOCK_VERSION and take add_mlir_python_extension from https://reviews.llvm.org/D111513.

mlir/CMakeLists.txt

110

I commented on the other patch about something I found confusing when using this in CIRCT, but I think this works.

[stellaraccident]

Thanks - if this passes presubmit, looks good to me.

[mikeurbach]

Update commit message.

mlir/CMakeLists.txt

 option(MLIR_INCLUDE_INTEGRATION_TESTS
        "Generate build targets for the MLIR integration tests.")
 
 #-------------------------------------------------------------------------------
 # Python Bindings Configuration
 # Requires:
 #   The pybind11 library can be found (set with -DPYBIND_DIR=...)
 #   The python executable is correct (set with -DPython3_EXECUTABLE=...)
-#
-# Version locking
-# ---------------
-# By default, python extensions are version locked to specific Python libraries.
-# This linking mode is somewhat more consistent across platforms and surfaces
-# undefined symbols at link time (vs runtime). It is suitable for development
-# workflows but can be disabled for more flexible deployment by
-# setting -DMLIR_BINDINGS_PYTHON_LOCK_VERSION=OFF
 #-------------------------------------------------------------------------------
 
 set(MLIR_ENABLE_BINDINGS_PYTHON 0 CACHE BOOL
        "Enables building of Python bindings.")
-set(MLIR_BINDINGS_PYTHON_LOCK_VERSION 1 CACHE BOOL
-       "Links to specific python libraries, resolving all symbols.")
 
 if(MLIR_ENABLE_BINDINGS_PYTHON)
   include(MLIRDetectPythonEnv)
+  # After CMake 3.18, we are able to limit the scope of the search to just
+  # Development.Module. Searching for Development will fail in situations where
+  # the Python libraries are not available. When possible, limit to just
+  # Development.Module.
+  # See https://pybind11.readthedocs.io/en/stable/compiling.html#findpython-mode
+  if(CMAKE_VERSION VERSION_LESS "3.18.0")
+    set(_python_development_component Development)
+  else()
+    set(_python_development_component Development.Module)
+  endif()
   find_package(Python3 ${LLVM_MINIMUM_PYTHON_VERSION}
-    COMPONENTS Interpreter Development NumPy REQUIRED)
+    COMPONENTS Interpreter ${_python_development_component} NumPy REQUIRED)
+  unset(_python_development_component)
   message(STATUS "Found python include dirs: ${Python3_INCLUDE_DIRS}")
   message(STATUS "Found python libraries: ${Python3_LIBRARIES}")
   message(STATUS "Found numpy v${Python3_NumPy_VERSION}: ${Python3_NumPy_INCLUDE_DIRS}")
   mlir_detect_pybind11_install()
   find_package(pybind11 2.6 CONFIG REQUIRED)
   message(STATUS "Found pybind11 v${pybind11_VERSION}: ${pybind11_INCLUDE_DIR}")
   message(STATUS "Python prefix = '${PYTHON_MODULE_PREFIX}', "
                  "suffix = '${PYTHON_MODULE_SUFFIX}', "

mlir/cmake/modules/AddMLIRPython.cmake

function(add_mlir_python_extension libname extname)
   ${ARGN})
   if (ARG_UNPARSED_ARGUMENTS)
     message(FATAL_ERROR " Unhandled arguments to add_mlir_python_extension(${libname}, ... : ${ARG_UNPARSED_ARGUMENTS}")
   endif()
   if ("${ARG_SOURCES}" STREQUAL "")
     message(FATAL_ERROR " Missing SOURCES argument to add_mlir_python_extension(${libname}, ...")
   endif()
 
-  # Build-time RPath layouts require to be a directory one up from the
-  # binary root.
-  # TODO: Don't reference the LLVM_BINARY_DIR here: the invariant is that
-  # the output directory must be at the same level of the lib directory
-  # where libMLIR.so is installed. This is presently not optimal from a
-  # project separation perspective and a discussion on how to better
-  # segment MLIR libraries needs to happen.
-  # TODO: Remove this when downstreams are moved off of it.
-  if(NOT ARG_OUTPUT_DIRECTORY)
-    set(ARG_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/python)
-  endif()
-
-  # Normally on unix-like platforms, extensions are built as "MODULE" libraries
-  # and do not explicitly link to the python shared object. This allows for
-  # some greater deployment flexibility since the extension will bind to
-  # symbols in the python interpreter on load. However, it also keeps the
-  # linker from erroring on undefined symbols, leaving this to (usually obtuse)
-  # runtime errors. Building in "SHARED" mode with an explicit link to the
-  # python libraries allows us to build with the expectation of no undefined
-  # symbols, which is better for development. Note that not all python
-  # configurations provide build-time libraries to link against, in which
-  # case, we fall back to MODULE linking.
-  if(Python3_LIBRARIES STREQUAL "" OR NOT MLIR_BINDINGS_PYTHON_LOCK_VERSION)
-    set(PYEXT_LINK_MODE MODULE)
-    set(PYEXT_LIBADD)
-  else()
-    set(PYEXT_LINK_MODE SHARED)
-    set(PYEXT_LIBADD ${Python3_LIBRARIES})
-  endif()
-
   # 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).
-  add_library(${libname}
-    ${PYEXT_LINK_MODE}
+  pybind11_add_module(${libname}
     ${ARG_SOURCES}
   )
 
-  target_include_directories(${libname} PRIVATE
-    "${Python3_INCLUDE_DIRS}"
-    "${pybind11_INCLUDE_DIR}"
-  )
-
-  target_link_directories(${libname} PRIVATE
-    "${Python3_LIBRARY_DIRS}"
-  )
-
   # The extension itself must be compiled with RTTI and exceptions enabled.
   # Also, some warning classes triggered by pybind11 are disabled.
   target_compile_options(${libname} PRIVATE
     $<$<OR:$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>,$<CXX_COMPILER_ID:GNU>>:
       # Enable RTTI and exceptions.
       -frtti -fexceptions
-      # Noisy pybind warnings
-      -Wno-unused-value
-      -Wno-covered-switch-default
     >
     $<$<CXX_COMPILER_ID:MSVC>:
       # Enable RTTI and exceptions.
       /EHsc /GR>
   )
 
   # Configure the output to match python expectations.
   set_target_properties(
     ${libname} PROPERTIES
     LIBRARY_OUTPUT_DIRECTORY ${ARG_OUTPUT_DIRECTORY}
     OUTPUT_NAME "${extname}"
-    PREFIX "${PYTHON_MODULE_PREFIX}"
-    SUFFIX "${PYTHON_MODULE_SUFFIX}${PYTHON_MODULE_EXTENSION}"
     NO_SONAME ON
   )
 
   if(WIN32)
     # Need to also set the RUNTIME_OUTPUT_DIRECTORY on Windows in order to
     # control where the .dll gets written.
     set_target_properties(
       ${libname} PROPERTIES
       RUNTIME_OUTPUT_DIRECTORY ${ARG_OUTPUT_DIRECTORY}
       ARCHIVE_OUTPUT_DIRECTORY ${ARG_OUTPUT_DIRECTORY}
     )
   endif()
 
-  # pybind11 requires binding code to be compiled with -fvisibility=hidden
-  # For static linkage, better code can be generated if the entire project
-  # compiles that way, but that is not enforced here. Instead, include a linker
-  # script that explicitly hides anything but the PyInit_* symbols, allowing gc
-  # to take place.
-  set_target_properties(${libname} PROPERTIES CXX_VISIBILITY_PRESET "hidden")
-
   # Python extensions depends *only* on the public API and LLVMSupport unless
   # if further dependencies are added explicitly.
   target_link_libraries(${libname}
     PRIVATE
     ${ARG_LINK_LIBS}
     ${PYEXT_LIBADD}
   )
 

mlir/docs/Bindings/Python.md

 
 * **`Python3_EXECUTABLE`**:`STRING`
 
   Specifies the `python` executable used for the LLVM build, including for
   determining header/link flags for the Python bindings. On systems with
   multiple Python implementations, setting this explicitly to the preferred
   `python3` executable is strongly recommended.
 
-* **`MLIR_BINDINGS_PYTHON_LOCK_VERSION`**`:BOOL`
-
-  Links the native extension against the Python runtime library, which is
-  optional on some platforms. While setting this to `OFF` can yield some greater
-  deployment flexibility, linking in this way allows the linker to report
-  compile time errors for unresolved symbols on all platforms, which makes for a
-  smoother development workflow. Defaults to `ON`.
-
 ### Recommended development practices
 
 It is recommended to use a python virtual environment. Many ways exist for this,
 but the following is the simplest:
 
 ```shell
 # Make sure your 'python' is what you expect. Note that on multi-python
 # systems, this may have a version suffix, and on many Linuxes and MacOS where