[flang-commits] [clang] [flang] [mlir] Avoid object libraries in the VS IDE (PR #93519)

via flang-commits flang-commits at lists.llvm.org
Mon Jun 3 16:08:17 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-llvm

Author: Michael Kruse (Meinersbur)

<details>
<summary>Changes</summary>

As discussed in #<!-- -->89743, when using the Visual Studio solution generators, object library projects are displayed as a collection of non-editable *.obj files. To look for the corresponding source files, one has to browse (or search) to the library's obj.libname project. This patch tries to avoid this as much as possible.

For Clang, there is already an exception for XCode. We handle MSVC_IDE the same way.

For MLIR, this is more complicated. There are explicit references to the obj.libname target that only work when there is an object library. This patch cleans up the reasons for why an object library is needed:

1. The obj.libname is modified in the calling CMakeLists.txt. Note that with use-only references, `add_library(<name> ALIAS <target>)` could have been used. 

2. An libMLIR.so (mlir-shlib) is also created. This works by adding linking the object libraries' object file into libMLIR.so (in addition to the library's own .so/.a). XCode is handled using the `-force_load` linker option instead. Windows is not even supported. This mechanism is different from LLVM's llvm-shlib that is created by linking static libraries with `-Wl,--whole-archive` (and `-Wl,-all_load` on MacOS).

3. The library might be added to an aggregate library. In-tree, the seems to be only libMLIR-C.so and the standalone example. It uses the object library and `-force_load` mechanism as above. Again, this is different from libLLVM-C.so. 

4. Build an object library whenever it was before this patch, except when generating a Visual Studio solution. This condition could be removed, but I am trying to avoid build breakages of whatever configurations others use.

This seem to not have worked with XCode because of the explicit references to obj.libname. I don't have access to XCode, but I tried to preserve the current working. IMHO there should be a common mechanism to build aggregate libraries for all LLVM projects instead of the 4 that we have now.

As far as I can see, this means for LLVM there are the following changes on whether object libraries are created:

1. An object library is created even in XCode if FORCE_OBJECT_LIBRARY is set. I do not know how XCode handles it, but I also know CMake will abort otherwise.

2. An object library is created even for explicitly SHARED libraries for building libMLIR.so. Again, mlir-shlib does not work otherwise. libMLIR.so itself is created using SHARED so this is marking it as EXCLUDE_FROM_LIBMLIR.

3. For the second condition, it is now sensitive to whether the mlir-shlib is built at all (LLVM_BUILD_LLVM_DYLIB). However, an object library is still built using the fourth condition unless using the MSVC solution generator. That is, except with MSVC_IDE, when an object library was built before, it will also be an object library now.

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


7 Files Affected:

- (modified) clang/cmake/modules/AddClang.cmake (+5-1) 
- (modified) clang/lib/Support/CMakeLists.txt (+1-1) 
- (modified) flang/cmake/modules/AddFlang.cmake (+15-6) 
- (modified) mlir/cmake/modules/AddMLIR.cmake (+42-20) 
- (modified) mlir/lib/Dialect/GPU/CMakeLists.txt (+2) 
- (modified) mlir/lib/Target/LLVM/CMakeLists.txt (+4) 
- (modified) mlir/tools/mlir-shlib/CMakeLists.txt (+1) 


``````````diff
diff --git a/clang/cmake/modules/AddClang.cmake b/clang/cmake/modules/AddClang.cmake
index a5ef639187d9d..9d09be1936847 100644
--- a/clang/cmake/modules/AddClang.cmake
+++ b/clang/cmake/modules/AddClang.cmake
@@ -96,8 +96,12 @@ macro(add_clang_library name)
     else()
       set(LIBTYPE STATIC)
     endif()
-    if(NOT XCODE)
+    if(NOT XCODE AND NOT MSVC_IDE)
       # The Xcode generator doesn't handle object libraries correctly.
+      # The Visual Studio CMake generator does handle object libraries
+      # correctly, but it is preferable to list the libraries with their
+      # source files (instead of the object files and the source files in
+      # a separate target in the "Object Libraries" folder)
       list(APPEND LIBTYPE OBJECT)
     endif()
     set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
diff --git a/clang/lib/Support/CMakeLists.txt b/clang/lib/Support/CMakeLists.txt
index 8ea5620052ed8..de06271e914ae 100644
--- a/clang/lib/Support/CMakeLists.txt
+++ b/clang/lib/Support/CMakeLists.txt
@@ -15,7 +15,7 @@ set(clangSupport_sources
 
 add_clang_library(clangSupport ${clangSupport_sources})
 
-if (NOT XCODE)
+if (TARGET obj.clangSupport)
   add_library(clangSupport_tablegen ALIAS obj.clangSupport)
 elseif (NOT LLVM_LINK_LLVM_DYLIB)
   add_library(clangSupport_tablegen ALIAS clangSupport)
diff --git a/flang/cmake/modules/AddFlang.cmake b/flang/cmake/modules/AddFlang.cmake
index 3a5119b83831f..aeb4d862cf780 100644
--- a/flang/cmake/modules/AddFlang.cmake
+++ b/flang/cmake/modules/AddFlang.cmake
@@ -51,19 +51,28 @@ function(add_flang_library name)
       
   endif()
 
-  if (ARG_SHARED)
+  if(ARG_SHARED AND ARG_STATIC)
+    set(LIBTYPE SHARED STATIC)
+  elseif(ARG_SHARED)
     set(LIBTYPE SHARED)
   else()
     # llvm_add_library ignores BUILD_SHARED_LIBS if STATIC is explicitly set,
     # so we need to handle it here.
-    if (BUILD_SHARED_LIBS AND NOT ARG_STATIC)
-      set(LIBTYPE SHARED OBJECT)
+    if(BUILD_SHARED_LIBS)
+      set(LIBTYPE SHARED)
     else()
-      set(LIBTYPE STATIC OBJECT)
+      set(LIBTYPE STATIC)
     endif()
-    set_property(GLOBAL APPEND PROPERTY FLANG_STATIC_LIBS ${name})
+    if(NOT XCODE AND NOT MSVC_IDE)
+      # The Xcode generator doesn't handle object libraries correctly.
+      # The Visual Studio CMake generator does handle object libraries
+      # correctly, but it is preferable to list the libraries with their
+      # source files (instead of the object files and the source files in
+      # a separate target in the "Object Libraries" folder)
+      list(APPEND LIBTYPE OBJECT)
+    endif()
+    set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
   endif()
-
   llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
 
   clang_target_link_libraries(${name} PRIVATE ${ARG_CLANG_LIBS})
diff --git a/mlir/cmake/modules/AddMLIR.cmake b/mlir/cmake/modules/AddMLIR.cmake
index a685277209598..a3324705c525c 100644
--- a/mlir/cmake/modules/AddMLIR.cmake
+++ b/mlir/cmake/modules/AddMLIR.cmake
@@ -306,26 +306,23 @@ endfunction()
 #   Don't include this library in libMLIR.so.  This option should be used
 #   for test libraries, executable-specific libraries, or rarely used libraries
 #   with large dependencies.
+# OBJECT
+#   The library's object library is referenced using "obj.${name}". For this to
+#   work reliably, this flag ensures that the OBJECT library exists.
 # ENABLE_AGGREGATION
-#   Forces generation of an OBJECT library, exports additional metadata,
+#   Exports additional metadata,
 #   and installs additional object files needed to include this as part of an
 #   aggregate shared library.
 #   TODO: Make this the default for all MLIR libraries once all libraries
 #   are compatible with building an object library.
 function(add_mlir_library name)
   cmake_parse_arguments(ARG
-    "SHARED;INSTALL_WITH_TOOLCHAIN;EXCLUDE_FROM_LIBMLIR;DISABLE_INSTALL;ENABLE_AGGREGATION"
+    "SHARED;INSTALL_WITH_TOOLCHAIN;EXCLUDE_FROM_LIBMLIR;DISABLE_INSTALL;ENABLE_AGGREGATION;OBJECT"
     ""
     "ADDITIONAL_HEADERS;DEPENDS;LINK_COMPONENTS;LINK_LIBS"
     ${ARGN})
   _set_mlir_additional_headers_as_srcs(${ARG_ADDITIONAL_HEADERS})
 
-  # Is an object library needed.
-  set(NEEDS_OBJECT_LIB OFF)
-  if(ARG_ENABLE_AGGREGATION)
-    set(NEEDS_OBJECT_LIB ON)
-  endif()
-
   # Determine type of library.
   if(ARG_SHARED)
     set(LIBTYPE SHARED)
@@ -337,18 +334,39 @@ function(add_mlir_library name)
     else()
       set(LIBTYPE STATIC)
     endif()
-    # Test libraries and such shouldn't be include in libMLIR.so
-    if(NOT ARG_EXCLUDE_FROM_LIBMLIR)
-      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})
-      set_property(GLOBAL APPEND PROPERTY MLIR_LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS})
-    endif()
   endif()
 
-  if(NEEDS_OBJECT_LIB AND NOT XCODE)
-    # The Xcode generator doesn't handle object libraries correctly.
-    # We special case xcode when building aggregates.
+  # Is an object library needed...?
+  # Note that the XCode generator doesn't handle object libraries correctly and
+  # usability is degraded in the Visual Studio solution generators.
+  # llvm_add_library may also itself decide to create an object library.
+  set(NEEDS_OBJECT_LIB OFF)
+  if(ARG_OBJECT)
+    # 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)
+    # 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).
+    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})
+    set_property(GLOBAL APPEND PROPERTY MLIR_LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS})
+  endif ()
+  if(ARG_ENABLE_AGGREGATION AND NOT XCODE)
+    # Yes, because this library is added to an aggergate library such as
+    # libMLIR-C.so which is links together all the object files.
+    # For XCode, -force_load is used instead.
+    set(NEEDS_OBJECT_LIB ON)
+  endif()
+  if (NOT ARG_SHARED AND NOT ARG_EXCLUDE_FROM_LIBMLIR AND NOT XCODE AND NOT MSVC_IDE)
+    # Yes, but only for legacy reasons. Also avoid object libraries for
+    # Visual Studio solutions.
+    set(NEEDS_OBJECT_LIB ON)
+  endif()
+  if(NEEDS_OBJECT_LIB)
     list(APPEND LIBTYPE OBJECT)
   endif()
 
@@ -380,9 +398,12 @@ function(add_mlir_library name)
       # XCode has limited support for object libraries. Instead, add dep flags
       # that force the entire library to be embedded.
       list(APPEND AGGREGATE_DEPS "-force_load" "${name}")
-    else()
+    elseif(TARGET obj.${name})
+      # FIXME: *.obj can also be added via target_link_libraries since CMake 3.12.
       list(APPEND AGGREGATE_OBJECTS "$<TARGET_OBJECTS:obj.${name}>")
       list(APPEND AGGREGATE_OBJECT_LIB "obj.${name}")
+    else()
+      message(SEND_ERROR "Aggregate library not supported on this platform")
     endif()
 
     # For each declared dependency, transform it into a generator expression
@@ -402,7 +423,7 @@ function(add_mlir_library name)
 
     # In order for out-of-tree projects to build aggregates of this library,
     # we need to install the OBJECT library.
-    if(MLIR_INSTALL_AGGREGATE_OBJECTS AND NOT ARG_DISABLE_INSTALL)
+    if(TARGET "obj.${name}" AND MLIR_INSTALL_AGGREGATE_OBJECTS AND NOT ARG_DISABLE_INSTALL)
       add_mlir_library_install(obj.${name})
     endif()
   endif()
@@ -615,6 +636,7 @@ endfunction()
 function(add_mlir_public_c_api_library name)
   add_mlir_library(${name}
     ${ARGN}
+    OBJECT
     EXCLUDE_FROM_LIBMLIR
     ENABLE_AGGREGATION
     ADDITIONAL_HEADER_DIRS
diff --git a/mlir/lib/Dialect/GPU/CMakeLists.txt b/mlir/lib/Dialect/GPU/CMakeLists.txt
index 61ab298ebfb98..8c64c2098b315 100644
--- a/mlir/lib/Dialect/GPU/CMakeLists.txt
+++ b/mlir/lib/Dialect/GPU/CMakeLists.txt
@@ -57,6 +57,8 @@ add_mlir_dialect_library(MLIRGPUTransforms
   Transforms/SPIRVAttachTarget.cpp
   Transforms/SubgroupReduceLowering.cpp
   Transforms/Utils.cpp
+  
+  OBJECT
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Dialect/GPU
diff --git a/mlir/lib/Target/LLVM/CMakeLists.txt b/mlir/lib/Target/LLVM/CMakeLists.txt
index 5a3fa160850b4..1c709e18cbfda 100644
--- a/mlir/lib/Target/LLVM/CMakeLists.txt
+++ b/mlir/lib/Target/LLVM/CMakeLists.txt
@@ -32,6 +32,8 @@ endif()
 add_mlir_dialect_library(MLIRNVVMTarget
   NVVM/Target.cpp
 
+  OBJECT
+
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Dialect/LLVMIR
 
@@ -109,6 +111,8 @@ endif()
 add_mlir_dialect_library(MLIRROCDLTarget
   ROCDL/Target.cpp
 
+  OBJECT
+
   LINK_COMPONENTS
   MCParser
   ${AMDGPU_LIBS}
diff --git a/mlir/tools/mlir-shlib/CMakeLists.txt b/mlir/tools/mlir-shlib/CMakeLists.txt
index 32fe833cee4ea..a33c70c5807be 100644
--- a/mlir/tools/mlir-shlib/CMakeLists.txt
+++ b/mlir/tools/mlir-shlib/CMakeLists.txt
@@ -34,6 +34,7 @@ if(LLVM_BUILD_LLVM_DYLIB)
   add_mlir_library(
     MLIR
     SHARED
+    EXCLUDE_FROM_LIBMLIR
     ${INSTALL_WITH_TOOLCHAIN}
     mlir-shlib.cpp
     ${_OBJECTS}

``````````

</details>


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


More information about the flang-commits mailing list