llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lld

Author: None (nextsilicon-itay-bookstein)

<details>
<summary>Changes</summary>

Previously, transitively inherited calls to
`target_include_directories(foo SYSTEM ...)` were being squashed into a flat 
list of includes, effectively stripping off `-isystem` and unintentionally 
forwarding warnings from such dependencies.

To correctly propagate `SYSTEM` dependencies, use
`target_link_libraries` to forward the parent target's link dependencies to the 
OBJECT library (similar to the `_static` flow below). Unlike a flat 
`target_include_directories`, this lets CMake resolve transitive SYSTEM include 
directories through the proper dependency chain.

Note that `target_link_libraries` on an OBJECT library propagates all usage 
requirements, not just includes. This also brings in transitive 
`INTERFACE_COMPILE_DEFINITIONS`, `INTERFACE_COMPILE_OPTIONS`, and 
`INTERFACE_COMPILE_FEATURES`. This is arguably more correct, as the OBJECT 
library compiles the same sources and should see the same flags.

The existing `target_include_directories` call is retained for include 
directories set directly on the target (not through link dependencies). CMake 
deduplicates include directories that appear through both paths. Compile 
definitions and options may technically appear twice (once via the OBJECT 
library, once via the consuming target), but duplicate `-D` and flag entries 
are harmless in practice.

Also fix `clang_target_link_libraries` and `mlir_target_link_libraries` to 
forward the link type (PUBLIC/PRIVATE/INTERFACE) to `obj.*` targets. Previously 
the type keyword was silently dropped, resulting in plain- signature 
`target_link_libraries` calls. This is now required because the new 
keyword-signature call in `llvm_add_library` would otherwise conflict (CMake 
requires all calls on a target to use the same signature).

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


5 Files Affected:

- (modified) clang/cmake/modules/AddClang.cmake (+1-1) 
- (modified) lld/tools/lld/CMakeLists.txt (+1-1) 
- (modified) llvm/cmake/modules/AddLLVM.cmake (+17-12) 
- (modified) llvm/cmake/modules/LLVM-Config.cmake (+4-1) 
- (modified) mlir/cmake/modules/AddMLIR.cmake (+1-1) 


``````````diff
diff --git a/clang/cmake/modules/AddClang.cmake 
b/clang/cmake/modules/AddClang.cmake
index c811b7f459126..e2112d7c326e2 100644
--- a/clang/cmake/modules/AddClang.cmake
+++ b/clang/cmake/modules/AddClang.cmake
@@ -217,7 +217,7 @@ endmacro()
 
 function(clang_target_link_libraries target type)
   if (TARGET obj.${target})
-    target_link_libraries(obj.${target} ${ARGN})
+    target_link_libraries(obj.${target} ${type} ${ARGN})
   endif()
 
   get_property(LLVM_DRIVER_TOOLS GLOBAL PROPERTY LLVM_DRIVER_TOOLS)
diff --git a/lld/tools/lld/CMakeLists.txt b/lld/tools/lld/CMakeLists.txt
index 8498a91597a93..1d1a7ff448b7c 100644
--- a/lld/tools/lld/CMakeLists.txt
+++ b/lld/tools/lld/CMakeLists.txt
@@ -13,7 +13,7 @@ export_executable_symbols_for_plugins(lld)
 
 function(lld_target_link_libraries target type)
   if (TARGET obj.${target})
-    target_link_libraries(obj.${target} ${ARGN})
+    target_link_libraries(obj.${target} ${type} ${ARGN})
   endif()
 
   get_property(LLVM_DRIVER_TOOLS GLOBAL PROPERTY LLVM_DRIVER_TOOLS)
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index f59002bc6c837..251fe90e3e83a 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -6,14 +6,14 @@ include(DetermineGCCCompatible)
 
 # get_subproject_title(titlevar)
 #   Set ${outvar} to the title of the current LLVM subproject (Clang, MLIR ...)
-# 
+#
 # The title is set in the subproject's top-level using the variable
 # LLVM_SUBPROJECT_TITLE. If it does not exist, it is assumed it is LLVM itself.
 # The title is not semantically significant, but use to create folders in
 # CMake-generated IDE projects (Visual Studio/XCode).
 function(get_subproject_title outvar)
   if (LLVM_SUBPROJECT_TITLE)
-    set(${outvar} "${LLVM_SUBPROJECT_TITLE}" PARENT_SCOPE) 
+    set(${outvar} "${LLVM_SUBPROJECT_TITLE}" PARENT_SCOPE)
   else ()
     set(${outvar} "LLVM" PARENT_SCOPE)
   endif ()
@@ -636,7 +636,12 @@ function(llvm_add_library name)
     # Do add_dependencies(obj) later due to CMake issue 14747.
     list(APPEND objlibs ${obj_name})
 
-    # Bring in the target include directories from our original target.
+    # Bring in the target include directories and link info from our original
+    # target. target_link_libraries propagates transitive dependencies with
+    # proper SYSTEM include handling from IMPORTED targets.
+    # target_include_directories propagates include directories set directly on
+    # the target.
+    target_link_libraries(${obj_name} PRIVATE 
$<TARGET_PROPERTY:${name},LINK_LIBRARIES>)
     target_include_directories(${obj_name} PRIVATE 
$<TARGET_PROPERTY:${name},INCLUDE_DIRECTORIES>)
 
     set_target_properties(${obj_name} PROPERTIES FOLDER 
"${subproject_title}/Object Libraries")
@@ -702,11 +707,11 @@ function(llvm_add_library name)
   endif()
   set_target_properties(${name} PROPERTIES FOLDER 
"${subproject_title}/Libraries")
 
-  ## If were compiling with clang-cl use /Zc:dllexportInlines- to exclude 
inline 
+  ## If were compiling with clang-cl use /Zc:dllexportInlines- to exclude 
inline
   ## class members from being dllexport'ed to reduce compile time.
   ## This will also keep us below the 64k exported symbol limit
   ## 
https://blog.llvm.org/2018/11/30-faster-windows-builds-with-clang-cl_14.html
-  if(LLVM_BUILD_LLVM_DYLIB AND NOT LLVM_DYLIB_EXPORT_INLINES AND 
+  if(LLVM_BUILD_LLVM_DYLIB AND NOT LLVM_DYLIB_EXPORT_INLINES AND
      MSVC AND CMAKE_CXX_COMPILER_ID MATCHES Clang)
     target_compile_options(${name} PUBLIC /Zc:dllexportInlines-)
     if(TARGET ${obj_name})
@@ -1117,7 +1122,7 @@ macro(generate_llvm_objects name)
 
       set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_TOOLS ${name})
       set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_TOOL_ALIASES_${name} 
${name})
-      target_link_libraries(${obj_name} ${LLVM_PTHREAD_LIB})
+      target_link_libraries(${obj_name} PUBLIC ${LLVM_PTHREAD_LIB})
       llvm_config(${obj_name} ${USE_SHARED} ${LLVM_LINK_COMPONENTS} )
     endif()
   endif()
@@ -1599,8 +1604,8 @@ macro(llvm_add_tool project name)
                 RUNTIME DESTINATION ${${project}_TOOLS_INSTALL_DIR}
                 COMPONENT ${name})
         if (LLVM_ENABLE_PDB)
-          install(FILES $<TARGET_PDB_FILE:${name}> 
-                DESTINATION "${${project}_TOOLS_INSTALL_DIR}" COMPONENT 
${name} 
+          install(FILES $<TARGET_PDB_FILE:${name}>
+                DESTINATION "${${project}_TOOLS_INSTALL_DIR}" COMPONENT ${name}
                 OPTIONAL)
         endif()
 
@@ -1634,8 +1639,8 @@ macro(add_llvm_example name)
   if( LLVM_BUILD_EXAMPLES )
     install(TARGETS ${name} RUNTIME DESTINATION "${LLVM_EXAMPLES_INSTALL_DIR}")
     if (LLVM_ENABLE_PDB)
-      install(FILES $<TARGET_PDB_FILE:${name}> 
-              DESTINATION "${LLVM_EXAMPLES_INSTALL_DIR}" COMPONENT ${name} 
+      install(FILES $<TARGET_PDB_FILE:${name}>
+              DESTINATION "${LLVM_EXAMPLES_INSTALL_DIR}" COMPONENT ${name}
               OPTIONAL)
     endif()
   endif()
@@ -1673,8 +1678,8 @@ macro(add_llvm_utility name)
               RUNTIME DESTINATION ${LLVM_UTILS_INSTALL_DIR}
               COMPONENT ${name})
       if (LLVM_ENABLE_PDB)
-        install(FILES $<TARGET_PDB_FILE:${name}> 
-                DESTINATION "${LLVM_UTILS_INSTALL_DIR}" COMPONENT ${name} 
+        install(FILES $<TARGET_PDB_FILE:${name}>
+                DESTINATION "${LLVM_UTILS_INSTALL_DIR}" COMPONENT ${name}
                 OPTIONAL)
       endif()
 
diff --git a/llvm/cmake/modules/LLVM-Config.cmake 
b/llvm/cmake/modules/LLVM-Config.cmake
index 96ccf20aa89bd..7da7e40f3b0fe 100644
--- a/llvm/cmake/modules/LLVM-Config.cmake
+++ b/llvm/cmake/modules/LLVM-Config.cmake
@@ -106,7 +106,10 @@ function(explicit_llvm_config executable)
   get_target_property(t ${executable} TYPE)
   if(t STREQUAL "STATIC_LIBRARY")
     target_link_libraries(${executable} INTERFACE ${LIBRARIES})
-  elseif(t STREQUAL "EXECUTABLE" OR t STREQUAL "SHARED_LIBRARY" OR t STREQUAL 
"MODULE_LIBRARY")
+  elseif(t STREQUAL "EXECUTABLE" OR
+         t STREQUAL "SHARED_LIBRARY" OR
+         t STREQUAL "MODULE_LIBRARY" OR
+         t STREQUAL "OBJECT_LIBRARY")
     target_link_libraries(${executable} PRIVATE ${LIBRARIES})
   else()
     # Use plain form for legacy user.
diff --git a/mlir/cmake/modules/AddMLIR.cmake b/mlir/cmake/modules/AddMLIR.cmake
index b240aeba23109..60b73876d53fe 100644
--- a/mlir/cmake/modules/AddMLIR.cmake
+++ b/mlir/cmake/modules/AddMLIR.cmake
@@ -769,7 +769,7 @@ endfunction(mlir_check_all_link_libraries)
 # used.
 function(mlir_target_link_libraries target type)
   if (TARGET obj.${target})
-    target_link_libraries(obj.${target} ${ARGN})
+    target_link_libraries(obj.${target} ${type} ${ARGN})
   endif()
 
   if (MLIR_LINK_MLIR_DYLIB)

``````````

</details>


https://github.com/llvm/llvm-project/pull/184201
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to