[libc] Redo the install targets #78795
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prior to this change, we wouldn't build headers that aren't referenced by other parts of the libc which would result in a build error during installation. To address this, we make the header target a dependency of the libc archive. Additionally, we also redo the install targets, moving the install targets closer to build targets and simplifying the hierarchy and generally matching what we do for other runtimes.
petrhosek
requested review from
nickdesaulniers,
jhuber6,
lntue and
michaelrj-google
|
@llvm/pr-subscribers-libc Author: Petr Hosek (petrhosek) ChangesPrior to this change, we wouldn't build headers that aren't referenced by other parts of the libc which would result in a build error during installation. To address this, we make the header target a dependency of the libc archive. Additionally, we also redo the install targets, moving the install targets closer to build targets and simplifying the hierarchy and generally matching what we do for other runtimes. Full diff: https://github.com/llvm/llvm-project/pull/78795.diff 4 Files Affected:
diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index 114d2dc65dec21..6d385849b6a64f 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -348,19 +348,6 @@ foreach(entrypoint IN LISTS TARGET_LLVMLIBC_ENTRYPOINTS)
list(APPEND TARGET_ENTRYPOINT_NAME_LIST ${entrypoint_name})
endforeach()
-set(LIBC_INSTALL_DEPENDS)
-if(LLVM_LIBC_FULL_BUILD)
- set(LIBC_INSTALL_DEPENDS "install-libc-static-archives;install-libc-headers")
- if(NOT LIBC_TARGET_OS_IS_BAREMETAL)
- # For now we will disable libc-startup installation for baremetal. The
- # correct way to do it would be to make a hookable startup for baremetal
- # and install it as part of the libc installation.
- list(APPEND LIBC_INSTALL_DEPENDS "libc-startup")
- endif()
-else()
- set(LIBC_INSTALL_DEPENDS install-libc-static-archives)
-endif()
-
add_subdirectory(include)
add_subdirectory(config)
add_subdirectory(src)
@@ -388,18 +375,3 @@ endif()
if (LIBC_INCLUDE_DOCS)
add_subdirectory(docs)
endif()
-
-
-if(LLVM_LIBC_FULL_BUILD)
- add_llvm_install_targets(
- install-libc-headers
- DEPENDS libc-headers
- COMPONENT libc-headers
- )
-endif()
-
-add_llvm_install_targets(
- install-libc
- DEPENDS ${LIBC_INSTALL_DEPENDS}
- COMPONENT libc
-)
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 14aa9ec6d73f3e..cec8b86516b51f 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -606,3 +606,13 @@ foreach(target IN LISTS all_install_header_targets)
COMPONENT libc-headers)
endif()
endforeach()
+
+if(LLVM_LIBC_FULL_BUILD)
+ add_custom_target(install-libc-headers
+ DEPENDS libc-headers
+ COMMAND "${CMAKE_COMMAND}"
+ -DCMAKE_INSTALL_COMPONENT=libc-headers
+ -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+ # Stripping is a no-op for headers
+ add_custom_target(install-libc-headers-stripped DEPENDS install-libc-headers)
+endif()
diff --git a/libc/lib/CMakeLists.txt b/libc/lib/CMakeLists.txt
index f257582ed9bdf2..66aaa34632b395 100644
--- a/libc/lib/CMakeLists.txt
+++ b/libc/lib/CMakeLists.txt
@@ -33,6 +33,9 @@ foreach(archive IN ZIP_LISTS
PROPERTIES
ARCHIVE_OUTPUT_NAME ${archive_0}
)
+ if(LLVM_LIBC_FULL_BUILD)
+ target_link_libraries(${archive_1} PUBLIC libc-headers)
+ endif()
list(APPEND added_archive_targets ${archive_1})
endforeach()
@@ -48,11 +51,32 @@ endif()
install(
TARGETS ${added_archive_targets}
ARCHIVE DESTINATION ${LIBC_INSTALL_LIBRARY_DIR}
- COMPONENT libc-static-archives
+ COMPONENT libc
)
-add_llvm_install_targets(
- install-libc-static-archives
- DEPENDS ${added_archive_targets}
- COMPONENT libc-static-archives
-)
+if(NOT LIBC_TARGET_OS_IS_BAREMETAL)
+ # For now we will disable libc-startup installation for baremetal. The
+ # correct way to do it would be to make a hookable startup for baremetal
+ # and install it as part of the libc installation.
+ set(startup_target "libc-startup")
+endif()
+
+if(LLVM_LIBC_FULL_BUILD)
+ set(header_install_target install-libc-headers)
+endif()
+
+add_custom_target(install-libc
+ DEPENDS ${added_archive_targets}
+ ${startup_target}
+ ${header_install_target}
+ COMMAND "${CMAKE_COMMAND}"
+ -DCMAKE_INSTALL_COMPONENT=libc
+ -P "${LIBCXX_BINARY_DIR}/cmake_install.cmake")
+add_custom_target(install-libc-stripped
+ DEPENDS ${added_archive_targets}
+ ${startup_target}
+ ${header_install_target}
+ COMMAND "${CMAKE_COMMAND}"
+ -DCMAKE_INSTALL_COMPONENT=libc
+ -DCMAKE_INSTALL_DO_STRIP=1
+ -P "${LIBCXX_BINARY_DIR}/cmake_install.cmake")
diff --git a/libc/utils/gpu/server/CMakeLists.txt b/libc/utils/gpu/server/CMakeLists.txt
index a1c4229d386f89..3d9b2bcab4dbce 100644
--- a/libc/utils/gpu/server/CMakeLists.txt
+++ b/libc/utils/gpu/server/CMakeLists.txt
@@ -14,7 +14,7 @@ target_compile_definitions(llvmlibc_rpc_server PUBLIC
# Install the server and associated header.
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/rpc_server.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/gpu-none-llvm/
- COMPONENT libc)
+ COMPONENT libc-headers)
install(TARGETS llvmlibc_rpc_server
ARCHIVE DESTINATION "lib${LLVM_LIBDIR_SUFFIX}"
COMPONENT libc)
|
|
It seems that this somehow broke the libc on GPU tests in https://lab.llvm.org/staging/#/builders/11/builds/675 @jhuber6 can you have a look at it, please? Thanks! |
I'll look into it. We really need to get a production BB so I don't need to triage this every time something lands. |
This patch definitely didn't update something that it expects. |
|
I looked at the https://lab.llvm.org/staging/#/builders/11 builder but I don't see that failure, which step is failing? |
This should be addressed by 04c8558. |
The failure observed on the bot was unrelated, but there's another issue where when I do |
Beat my reply, thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this change, we wouldn't build headers that aren't referenced by other parts of the libc which would result in a build error during installation. To address this, we make the header target a dependency of the libc archive. Additionally, we also redo the install targets, moving the install targets closer to build targets and simplifying the hierarchy and generally matching what we do for other runtimes.