[Mlir-commits] [mlir] [mlir] Fix MLIR dylib CMake config when LLVM_BUILD_LLVM_DYLIB is OFF (PR #155488)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-execution-engine

Author: Bryan Tan (Technius)

<details>
<summary>Changes</summary>

Currently, MLIR fails to build when LLVM_BUILD_LLVM_DYLIB is OFF and MLIR_LINK_MLIR_DYLIB is ON. The reason for this is that the MLIR shared library target is only defined when LLVM_BUILD_LLVM_DYLIB is set to ON, but the `mlir_target_link_libraries` CMake function will attempt to link its given target against the MLIR shared library target whenever the MLIR_LINK_MLIR_DYLIB option is ON.

Furthermore, there is a second issue where the MLIR target is referenced by `mlir_target_link_libraries` calls _before_ it is defined by `tools/mlir-shlib/CMakeLists.txt`.

To address the above problems, this commit introduces a new MLIR_BUILD_MLIR_DYLIB CMake option that is used to determine whether the MLIR shared libary target should be built. Similar to LLVM_BUILD_LLVM_DYLIB, the corresponding MLIR option is automatically enabled whenever MLIR_LINK_MLIR_DYLIB is ON.

The `mlir_target_link_libraries` now links against an interface target MLIRDylib, which "foward declares" the MLIR shared library target, as the latter cannot be defined until after all the individual libraries are.

---
Full diff: https://github.com/llvm/llvm-project/pull/155488.diff


4 Files Affected:

- (modified) mlir/CMakeLists.txt (+14-2) 
- (modified) mlir/cmake/modules/AddMLIR.cmake (+3-3) 
- (modified) mlir/lib/ExecutionEngine/CMakeLists.txt (+1-1) 
- (modified) mlir/tools/mlir-shlib/CMakeLists.txt (+2-1) 


``````````diff
diff --git a/mlir/CMakeLists.txt b/mlir/CMakeLists.txt
index f58a4c6f506ec..371740292323a 100644
--- a/mlir/CMakeLists.txt
+++ b/mlir/CMakeLists.txt
@@ -172,8 +172,11 @@ set(MLIR_INSTALL_AGGREGATE_OBJECTS 1 CACHE BOOL
 
 set(MLIR_BUILD_MLIR_C_DYLIB 0 CACHE BOOL "Builds libMLIR-C shared library.")
 
-set(MLIR_LINK_MLIR_DYLIB ${LLVM_LINK_LLVM_DYLIB} CACHE BOOL
-    "Link tools against libMLIR.so")
+set(MLIR_BUILD_MLIR_DYLIB 0 CACHE BOOL "Builds the libMLIR shared library")
+set(MLIR_LINK_MLIR_DYLIB ${LLVM_LINK_LLVM_DYLIB} CACHE BOOL "Link tools against libMLIR.so")
+if (MLIR_LINK_MLIR_DYLIB)
+  set(MLIR_BUILD_MLIR_DYLIB 1)
+endif()
 
 configure_file(
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Config/mlir-config.h.cmake
@@ -235,6 +238,15 @@ set(MLIR_PDLL_TABLEGEN_TARGET "${MLIR_PDLL_TABLEGEN_TARGET}" CACHE INTERNAL "")
 set(MLIR_SRC_SHARDER_TABLEGEN_EXE "${MLIR_SRC_SHARDER_TABLEGEN_EXE}" CACHE INTERNAL "")
 set(MLIR_SRC_SHARDER_TABLEGEN_TARGET "${MLIR_SRC_SHARDER_TABLEGEN_TARGET}" CACHE INTERNAL "")
 
+# Add MLIR dylib target here, as calls to mlir_target_link_libraries (used by
+# individual libs below) assume that such a target may exist,
+# but we cannot define the actual dylib until after all individual libs
+# are defined.
+if (MLIR_LINK_MLIR_DYLIB)
+  add_library(MLIRDylib INTERFACE)
+  add_mlir_library_install(MLIRDylib)
+endif()
+
 add_subdirectory(include/mlir)
 add_subdirectory(lib)
 # C API needs all dialects for registration, but should be built before tests.
diff --git a/mlir/cmake/modules/AddMLIR.cmake b/mlir/cmake/modules/AddMLIR.cmake
index 38839679ef8b1..9545eab265db3 100644
--- a/mlir/cmake/modules/AddMLIR.cmake
+++ b/mlir/cmake/modules/AddMLIR.cmake
@@ -354,11 +354,11 @@ function(add_mlir_library name)
     # Yes, because the target "obj.${name}" is referenced.
     set(NEEDS_OBJECT_LIB ON)
   endif ()
-  if(LLVM_BUILD_LLVM_DYLIB AND NOT ARG_EXCLUDE_FROM_LIBMLIR AND NOT XCODE)
+  if(MLIR_BUILD_MLIR_DYLIB AND NOT ARG_EXCLUDE_FROM_LIBMLIR AND NOT XCODE)
     # Yes, because in addition to the shared library, the object files are
     # needed for linking into libMLIR.so (see mlir/tools/mlir-shlib/CMakeLists.txt).
     # For XCode, -force_load is used instead.
-    # Windows is not supported (LLVM_BUILD_LLVM_DYLIB=ON will cause an error).
+    # Windows is not supported (MLIR_BUILD_MLIR_DYLIB=ON will cause an error).
     set(NEEDS_OBJECT_LIB ON)
     set_property(GLOBAL APPEND PROPERTY MLIR_STATIC_LIBS ${name})
     set_property(GLOBAL APPEND PROPERTY MLIR_LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS})
@@ -745,7 +745,7 @@ function(mlir_target_link_libraries target type)
   endif()
 
   if (MLIR_LINK_MLIR_DYLIB)
-    target_link_libraries(${target} ${type} MLIR)
+    target_link_libraries(${target} ${type} MLIRDylib)
   else()
     target_link_libraries(${target} ${type} ${ARGN})
   endif()
diff --git a/mlir/lib/ExecutionEngine/CMakeLists.txt b/mlir/lib/ExecutionEngine/CMakeLists.txt
index fdeb4dacf9278..85cbb1ef3bc4e 100644
--- a/mlir/lib/ExecutionEngine/CMakeLists.txt
+++ b/mlir/lib/ExecutionEngine/CMakeLists.txt
@@ -103,7 +103,7 @@ mlir_target_link_libraries(MLIRExecutionEngine PUBLIC
   MLIRTargetLLVMIRExport
   )
 
-if(LLVM_BUILD_LLVM_DYLIB AND NOT (WIN32 OR MINGW OR CYGWIN)) # Does not build on windows currently, see #106859
+if(MLIR_BUILD_MLIR_DYLIB AND NOT (WIN32 OR MINGW OR CYGWIN)) # Does not build on windows currently, see #106859
   # Build a shared library for the execution engine. Some downstream projects
   # use this library to build their own CPU runners while preserving dynamic
   # linkage.
diff --git a/mlir/tools/mlir-shlib/CMakeLists.txt b/mlir/tools/mlir-shlib/CMakeLists.txt
index a33c70c5807be..166bab744de36 100644
--- a/mlir/tools/mlir-shlib/CMakeLists.txt
+++ b/mlir/tools/mlir-shlib/CMakeLists.txt
@@ -30,7 +30,7 @@ if(MLIR_LINK_MLIR_DYLIB)
   set(INSTALL_WITH_TOOLCHAIN INSTALL_WITH_TOOLCHAIN)
 endif()
 
-if(LLVM_BUILD_LLVM_DYLIB)
+if(MLIR_BUILD_MLIR_DYLIB)
   add_mlir_library(
     MLIR
     SHARED
@@ -45,6 +45,7 @@ if(LLVM_BUILD_LLVM_DYLIB)
     ${mlir_llvm_link_components}
   )
   target_link_libraries(MLIR PRIVATE ${LLVM_PTHREAD_LIB})
+  target_link_libraries(MLIRDylib INTERFACE MLIR)
 endif()
 
 #message("Libraries included in libMLIR.so: ${mlir_libs}")

``````````

</details>


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