[Mlir-commits] [mlir] 2607209 - Remove libMLIRPublicAPI DSO.

Stella Laurenzo llvmlistbot at llvm.org
Tue Jul 20 17:59:11 PDT 2021


Author: Stella Laurenzo
Date: 2021-07-20T17:58:28-07:00
New Revision: 2607209b3fffc0a01e02df3623cf2a46b2be2bc0

URL: https://github.com/llvm/llvm-project/commit/2607209b3fffc0a01e02df3623cf2a46b2be2bc0
DIFF: https://github.com/llvm/llvm-project/commit/2607209b3fffc0a01e02df3623cf2a46b2be2bc0.diff

LOG: Remove libMLIRPublicAPI DSO.

libMLIRPublicAPI.so came into existence early when the Python and C-API were being co-developed because the Python extensions need a single DSO which exports the C-API to link against. It really should never have been exported as a mondo library in the first place, which has caused no end of problems in different linking modes, etc (i.e. the CAPI tests depended on it).

This patch does a mechanical move that:

* Makes the C-API tests link directly to their respective libraries.
* Creates a libMLIRPythonCAPI as part of the Python bindings which assemble to exact DSO that they need.

This has the effect that the C-API is no longer monolithic and can be subset and used piecemeal in a modular fashion, which is necessary for downstreams to only pay for what they use. There are additional, more fundamental changes planned for how the Python API is assembled which should make it more out of tree friendly, but this minimal first step is necessary to break the fragile dependency between the C-API and Python API.

Downstream actions required:

* If using the C-API and linking against MLIRPublicAPI, you must instead link against its constituent components. As a reference, the Python API dependencies are in lib/Bindings/Python/CMakeLists.txt and approximate the full set of dependencies available.
* If you have a Python API project that was previously linking against MLIRPublicAPI (i.e. to add its own C-API DSO), you will want to `s/MLIRPublicAPI/MLIRPythonCAPI/` and all should be as it was. There are larger changes coming in this area but this part is incremental.

Reviewed By: mehdi_amini

Differential Revision: https://reviews.llvm.org/D106369

Added: 
    

Modified: 
    mlir/cmake/modules/AddMLIR.cmake
    mlir/cmake/modules/AddMLIRPython.cmake
    mlir/include/mlir-c/Support.h
    mlir/lib/Bindings/Python/CMakeLists.txt
    mlir/lib/Bindings/Python/Conversions/CMakeLists.txt
    mlir/lib/Bindings/Python/Transforms/CMakeLists.txt
    mlir/lib/CAPI/CMakeLists.txt
    mlir/python/mlir/_cext_loader.py
    mlir/test/CAPI/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/mlir/cmake/modules/AddMLIR.cmake b/mlir/cmake/modules/AddMLIR.cmake
index 109ac46ab6d26..76f6841f772e9 100644
--- a/mlir/cmake/modules/AddMLIR.cmake
+++ b/mlir/cmake/modules/AddMLIR.cmake
@@ -162,10 +162,7 @@ function(add_mlir_library_install name)
   set_property(GLOBAL APPEND PROPERTY MLIR_EXPORTS ${name})
 endfunction()
 
-# Declare an mlir library which is part of the public C-API and will be
-# compiled and exported into libMLIRPublicAPI.so/MLIRPublicAPI.dll.
-# This shared library is built regardless of the overall setting of building
-# libMLIR.so (which exports the C++ implementation).
+# Declare an mlir library which is part of the public C-API.
 function(add_mlir_public_c_api_library name)
   add_mlir_library(${name}
     ${ARGN}
@@ -186,7 +183,6 @@ function(add_mlir_public_c_api_library name)
     PRIVATE
     -DMLIR_CAPI_BUILDING_LIBRARY=1
   )
-  set_property(GLOBAL APPEND PROPERTY MLIR_PUBLIC_C_API_LIBS ${name})
 endfunction()
 
 # Declare the library associated with a dialect.

diff  --git a/mlir/cmake/modules/AddMLIRPython.cmake b/mlir/cmake/modules/AddMLIRPython.cmake
index 06d2a9a278bd5..046acbc085dd7 100644
--- a/mlir/cmake/modules/AddMLIRPython.cmake
+++ b/mlir/cmake/modules/AddMLIRPython.cmake
@@ -35,7 +35,8 @@ 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).
-  add_library(${libname} ${PYEXT_LINK_MODE}
+  add_library(${libname}
+    ${PYEXT_LINK_MODE}
     ${ARG_SOURCES}
   )
 
@@ -99,8 +100,6 @@ function(add_mlir_python_extension libname extname)
   # if further dependencies are added explicitly.
   target_link_libraries(${libname}
     PRIVATE
-    MLIRPublicAPI
-    LLVMSupport
     ${ARG_LINK_LIBS}
     ${PYEXT_LIBADD}
   )

diff  --git a/mlir/include/mlir-c/Support.h b/mlir/include/mlir-c/Support.h
index 340f8ec8bebfa..315f6c4564eba 100644
--- a/mlir/include/mlir-c/Support.h
+++ b/mlir/include/mlir-c/Support.h
@@ -22,9 +22,17 @@
 //===----------------------------------------------------------------------===//
 // Visibility annotations.
 // Use MLIR_CAPI_EXPORTED for exported functions.
+//
+// On Windows, if MLIR_CAPI_ENABLE_WINDOWS_DLL_DECLSPEC is defined, then
+// __declspec(dllexport) and __declspec(dllimport) will be generated. This
+// can only be enabled if actually building DLLs. It is generally, mutually
+// exclusive with the use of other mechanisms for managing imports/exports
+// (i.e. CMake's WINDOWS_EXPORT_ALL_SYMBOLS feature).
 //===----------------------------------------------------------------------===//
 
-#if defined(MLIR_CAPI_DISABLE_VISIBILITY_ANNOTATIONS)
+#if (defined(_WIN32) || defined(__CYGWIN__)) &&                                \
+    !defined(MLIR_CAPI_ENABLE_WINDOWS_DLL_DECLSPEC)
+// Visibility annotations disabled.
 #define MLIR_CAPI_EXPORTED
 #elif defined(_WIN32) || defined(__CYGWIN__)
 // Windows visibility declarations.

diff  --git a/mlir/lib/Bindings/Python/CMakeLists.txt b/mlir/lib/Bindings/Python/CMakeLists.txt
index 173cf48c0d5c9..8a112b7f43525 100644
--- a/mlir/lib/Bindings/Python/CMakeLists.txt
+++ b/mlir/lib/Bindings/Python/CMakeLists.txt
@@ -1,6 +1,59 @@
 include(AddMLIRPython)
 add_custom_target(MLIRBindingsPythonExtension)
 
+################################################################################
+# All python extensions must link through one DSO which exports the CAPI, and
+# this must have a globally unique name amongst all embeddors of the python
+# library since it will effectively have global scope.
+#
+# The presence of this aggregate library is part of the long term plan, but its
+# use needs to be made more flexible.
+################################################################################
+
+set(public_api_libs
+  MLIRCAPIConversion
+  MLIRCAPIDebug
+  MLIRCEXECUTIONENGINE
+  MLIRCAPIIR
+  MLIRCAPIRegistration
+  MLIRCAPITransforms
+
+  # Dialects
+  MLIRCAPIAsync
+  MLIRCAPIGPU
+  MLIRCAPILinalg
+  MLIRCAPILLVM
+  MLIRCAPIShape
+  MLIRCAPISparseTensor
+  MLIRCAPIStandard
+  MLIRCAPISCF
+  MLIRCAPITensor
+)
+
+foreach(lib ${public_api_libs})
+  if(XCODE)
+    # Xcode doesn't support object libraries, so we have to trick it into
+    # linking the static libraries instead.
+    list(APPEND _DEPS "-force_load" ${lib})
+  else()
+    list(APPEND _OBJECTS $<TARGET_OBJECTS:obj.${lib}>)
+  endif()
+  # Accumulate transitive deps of each exported lib into _DEPS.
+  list(APPEND _DEPS  $<TARGET_PROPERTY:${lib},LINK_LIBRARIES>)
+endforeach()
+
+add_mlir_library(MLIRPythonCAPI
+  PARTIAL_SOURCES_INTENDED
+  SHARED
+  ${_OBJECTS}
+  EXCLUDE_FROM_LIBMLIR
+  LINK_LIBS
+  ${_DEPS}
+)
+if(MSVC)
+  set_property(TARGET MLIRPythonCAPI PROPERTY WINDOWS_EXPORT_ALL_SYMBOLS ON)
+endif()
+
 ################################################################################
 # Build core python extension
 ################################################################################
@@ -19,6 +72,9 @@ add_mlir_python_extension(MLIRCoreBindingsPythonExtension _mlir
     PybindUtils.cpp
     Pass.cpp
     ExecutionEngine.cpp
+  LINK_LIBS PRIVATE
+    LLVMSupport
+    MLIRPythonCAPI
 )
 add_dependencies(MLIRBindingsPythonExtension MLIRCoreBindingsPythonExtension)
 
@@ -30,6 +86,9 @@ add_mlir_python_extension(MLIRAllPassesRegistrationBindingsPythonExtension _mlir
     python
   SOURCES
     AllPassesRegistration.cpp
+  LINK_LIBS PRIVATE
+    LLVMSupport
+    MLIRPythonCAPI
 )
 add_dependencies(MLIRBindingsPythonExtension MLIRAllPassesRegistrationBindingsPythonExtension)
 
@@ -38,6 +97,9 @@ add_mlir_python_extension(MLIRAsyncPassesBindingsPythonExtension _mlirAsyncPasse
     python
   SOURCES
     AsyncPasses.cpp
+  LINK_LIBS PRIVATE
+    LLVMSupport
+    MLIRPythonCAPI
 )
 add_dependencies(MLIRBindingsPythonExtension MLIRAsyncPassesBindingsPythonExtension)
 
@@ -46,6 +108,9 @@ add_mlir_python_extension(MLIRSparseTensorPassesBindingsPythonExtension _mlirSpa
     python
   SOURCES
     SparseTensorPasses.cpp
+  LINK_LIBS PRIVATE
+    LLVMSupport
+    MLIRPythonCAPI
 )
 add_dependencies(MLIRBindingsPythonExtension MLIRSparseTensorPassesBindingsPythonExtension)
 
@@ -54,6 +119,9 @@ add_mlir_python_extension(MLIRGPUPassesBindingsPythonExtension _mlirGPUPasses
     python
   SOURCES
     GPUPasses.cpp
+  LINK_LIBS PRIVATE
+    LLVMSupport
+    MLIRPythonCAPI
 )
 add_dependencies(MLIRBindingsPythonExtension MLIRGPUPassesBindingsPythonExtension)
 
@@ -62,5 +130,8 @@ add_mlir_python_extension(MLIRLinalgPassesBindingsPythonExtension _mlirLinalgPas
     python
   SOURCES
     LinalgPasses.cpp
+  LINK_LIBS PRIVATE
+    LLVMSupport
+    MLIRPythonCAPI
 )
 add_dependencies(MLIRBindingsPythonExtension MLIRLinalgPassesBindingsPythonExtension)

diff  --git a/mlir/lib/Bindings/Python/Conversions/CMakeLists.txt b/mlir/lib/Bindings/Python/Conversions/CMakeLists.txt
index ad2aeefca6fdb..e39707d0c21de 100644
--- a/mlir/lib/Bindings/Python/Conversions/CMakeLists.txt
+++ b/mlir/lib/Bindings/Python/Conversions/CMakeLists.txt
@@ -7,4 +7,6 @@ add_mlir_python_extension(MLIRConversionsBindingsPythonExtension _mlirConversion
     python
   SOURCES
   Conversions.cpp
+  LINK_LIBS PRIVATE
+    MLIRPythonCAPI
 )

diff  --git a/mlir/lib/Bindings/Python/Transforms/CMakeLists.txt b/mlir/lib/Bindings/Python/Transforms/CMakeLists.txt
index 8b53f03d42b94..b33d1503b8859 100644
--- a/mlir/lib/Bindings/Python/Transforms/CMakeLists.txt
+++ b/mlir/lib/Bindings/Python/Transforms/CMakeLists.txt
@@ -7,4 +7,6 @@ add_mlir_python_extension(MLIRTransformsBindingsPythonExtension _mlirTransforms
     python
   SOURCES
   Transforms.cpp
-)
\ No newline at end of file
+  LINK_LIBS PRIVATE
+    MLIRPythonCAPI
+)

diff  --git a/mlir/lib/CAPI/CMakeLists.txt b/mlir/lib/CAPI/CMakeLists.txt
index cd119554f3dc8..eed3f38d1a46e 100644
--- a/mlir/lib/CAPI/CMakeLists.txt
+++ b/mlir/lib/CAPI/CMakeLists.txt
@@ -5,46 +5,3 @@ add_subdirectory(ExecutionEngine)
 add_subdirectory(IR)
 add_subdirectory(Registration)
 add_subdirectory(Transforms)
-
-
-################################################################################
-# libMLIRPublicAPI shared library/DLL.
-################################################################################
-
-get_property(public_api_libs GLOBAL PROPERTY MLIR_PUBLIC_C_API_LIBS)
-
-foreach(lib ${public_api_libs})
-  if(XCODE)
-    # Xcode doesn't support object libraries, so we have to trick it into
-    # linking the static libraries instead.
-    list(APPEND _DEPS "-force_load" ${lib})
-  else()
-    list(APPEND _OBJECTS $<TARGET_OBJECTS:obj.${lib}>)
-  endif()
-  # Accumulate transitive deps of each exported lib into _DEPS.
-  list(APPEND _DEPS  $<TARGET_PROPERTY:${lib},LINK_LIBRARIES>)
-endforeach()
-
-if(MINGW)
-    set(MLIR_LINK_MLIR_DYLIB 0)
-else()
-    set(MLIR_LINK_MLIR_DYLIB ${LLVM_BUILD_LLVM_DYLIB})
-endif()
-
-add_mlir_library(MLIRPublicAPI
-  SHARED
-  ${_OBJECTS}
-  EXCLUDE_FROM_LIBMLIR
-  LINK_LIBS
-  # Dependency on the implementation shared library.
-  $<$<BOOL:${MLIR_LINK_MLIR_DYLIB}>:MLIR>
-  ${_DEPS}
-)
-
-target_link_options(
-  MLIRPublicAPI
-  PRIVATE
-    # On Linux, disable re-export of any static linked libraries that
-    # came through.
-    $<$<PLATFORM_ID:Linux>:LINKER:--exclude-libs,ALL>
-)

diff  --git a/mlir/python/mlir/_cext_loader.py b/mlir/python/mlir/_cext_loader.py
index 3a9cde380354c..c691fccb47375 100644
--- a/mlir/python/mlir/_cext_loader.py
+++ b/mlir/python/mlir/_cext_loader.py
@@ -24,7 +24,7 @@ def _load_extension(name):
   _load_extension = _mlir_libs.load_extension
   _preload_dependency = _mlir_libs.preload_dependency
 
-_preload_dependency("MLIRPublicAPI")
+_preload_dependency("MLIRPythonCAPI")
 
 # Expose the corresponding C-Extension module with a well-known name at this
 # top-level module. This allows relative imports like the following to

diff  --git a/mlir/test/CAPI/CMakeLists.txt b/mlir/test/CAPI/CMakeLists.txt
index a0a812936a99f..25653ced926d9 100644
--- a/mlir/test/CAPI/CMakeLists.txt
+++ b/mlir/test/CAPI/CMakeLists.txt
@@ -2,7 +2,7 @@ function(_add_capi_test_executable name)
   cmake_parse_arguments(ARG
     ""
     ""
-    ""
+    "LINK_LIBS"
     ${ARGN})
   set(LLVM_LINK_COMPONENTS
     )
@@ -11,28 +11,45 @@ function(_add_capi_test_executable name)
     ${ARG_UNPARSED_ARGUMENTS})
   llvm_update_compile_flags(${name})
   target_link_libraries(${name}
-    PRIVATE
-    MLIRPublicAPI)
+    ${ARG_LINK_LIBS})
 endfunction(_add_capi_test_executable)
 
 _add_capi_test_executable(mlir-capi-execution-engine-test
   execution_engine.c
-DEPENDS
-  MLIRConversionPassIncGen
+LINK_LIBS PRIVATE
+  MLIRCAPIConversion
+  MLIRCEXECUTIONENGINE
+  MLIRCAPIRegistration
 )
 
 _add_capi_test_executable(mlir-capi-ir-test
   ir.c
+  LINK_LIBS PRIVATE
+    MLIRCAPIIR
+    MLIRCAPIStandard
+    MLIRCAPIRegistration
 )
 
 _add_capi_test_executable(mlir-capi-llvm-test
   llvm.c
+  LINK_LIBS PRIVATE
+    MLIRCAPIIR
+    MLIRCAPILLVM
+    MLIRCAPIRegistration
 )
 
 _add_capi_test_executable(mlir-capi-pass-test
   pass.c
+  LINK_LIBS PRIVATE
+    MLIRCAPIIR
+    MLIRCAPIRegistration
+    MLIRCAPITransforms
 )
 
 _add_capi_test_executable(mlir-capi-sparse-tensor-test
   sparse_tensor.c
+  LINK_LIBS PRIVATE
+    MLIRCAPIIR
+    MLIRCAPIRegistration
+    MLIRCAPISparseTensor
 )


        


More information about the Mlir-commits mailing list