[llvm-libgcc][CMake] Refactor llvm-libgcc #65455
Conversation
Commit c5a20b5 ([llvm-libgcc] initial commit) uses `$<TARGET_OBJECTS:unwind_static>` to get libunwind objects, which is empty. The built library is actually a shared version of libclang_rt.builtins.
This commit removes the limitation that `llvm-libgcc` cannot co-exist with `compiler-rt` and `libunwind` when configuring llvm builds. There's still a limitation that `-DCOMPILER_RT_BUILTINS_HIDE_SYMBOLS=No` is required when `compiler-rt` is listed in `LLVM_ENABLE_RUNTIMES`. Because `llvm-libgcc` will be added after `compiler-rt` in `runtimes/CMakeLists.txt`, thus flags set in `llvm-libgcc/CMakeLists.txt` will not take effect. There might be further refactoring which removes `llvm-libgcc` subproject, merges coresponding components into `compiler-rt` and `libunwind` and adds options like `COMPILER_RT_ENABLE_LIBGCC`, `LIBUNWIND_ENABLE_LIBGCC_EH` and `LIBUNWIND_ENABLE_LIBGCC_S`, with old `llvm-libgcc` in `LLVM_ENABLE_RUNTIMES` and `LLVM_LIBGCC_EXPLICIT_OPT_IN` preserved to keep compatibility.
|
CC: @cjdb |
|
FWIW I did notice that the build had been broken (likely for a while) when I recently removed some This patch makes Perhaps you can fix this as well. |
When configuring with `llvm/CMakeLists.txt`, target `llvm-libgcc` requires a corresponding target in `llvm-libgcc/CMakeLists.txt`. This commit fixes the mismatched target names.
This removed llvm-libunwind from the reviewer list. I wish that somebody familiar with CMake may take a look. |
There was a problem hiding this comment.
Thanks for working on this!
IIRC it was @compnerd who convinced me that llvm-libgcc should not be compatible with an independent compiler-rt and libunwind, and that was because you only need one or the other. I still agree with that since llvm-libgcc is only intended for toolchain vendors, and not for casual use.
It would be good to get some exposition on why this PR changes this, and input from @compnerd regarding that design decision too.
| "${LLVM_COMMON_CMAKE_UTILS}" | ||
| "${LLVM_COMMON_CMAKE_UTILS}/Modules" | ||
| ) | ||
|
|
||
| project(llvm-libgcc LANGUAGES C CXX ASM) |
There was a problem hiding this comment.
libunwind/CMakeLists.txt does not contains a project(libunwind LANGUAGES C CXX ASM). To keep consistent with libunwind/CMakeLists.txt, project(llvm-libgcc LANGUAGES C CXX ASM) is not added.
There was a problem hiding this comment.
compiler-rt does have it, and I'd prefer to keep this regardless.
There was a problem hiding this comment.
Now llvm-libgcc use the same approach as compiler-rt in 60eccd0.
There was a problem hiding this comment.
I'm not following what you mean, sorry. Since this is the driver for compiler-rt and libunwind, it makes sense to me for this project to indicate this information.
There was a problem hiding this comment.
compiler-rt/CMakeLists.txt detects whether compiler-rt is built as a subproject of runtimes or a standalone project. In standalone mode it sets options to detect llvm and declares the project name with project(CompilerRT C CXX ASM).
Now llvm-libgcc/CMakeLists.txt does the same: in standalone mode, it tells added compiler-rt that we are not built as a subproject of runtimes and we have to detect llvm ourselves.
Here "standalone" means cmake -B build -S compiler-rt or cmake -B build -S llvm-libgcc, not cmake -B build -S runtimes nor cmake -B build -S llvm. Even in the whole llvm-project source tree, directly using compiler-rt/CMakeLists.txt or llvm-libgcc/CMakeLists.txt is "standalone".
If included by other CMakeLists.txt, runtimes/CMakeLists.txt for example, it does not matter whether there is a project(CompilerRT C CXX ASM) or project(llvm-libgcc LANGUAGES C CXX ASM) declaration actually performed. To keep consistent with compiler-rt, llvm-libgcc/CMakeLists.txt is adjusted correspondingly in 60eccd0.
Correspondingly lines from compiler-rt/CMakeLists.txt :
# Check if compiler-rt is built as a standalone project.
if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR OR COMPILER_RT_STANDALONE_BUILD)
project(CompilerRT C CXX ASM)
set(COMPILER_RT_STANDALONE_BUILD TRUE)
set_property(GLOBAL PROPERTY USE_FOLDERS ON)
endif()Correspondingly lines in llvm-libgcc/CMakeLists.txt:
# Check if llvm-libgcc is built as a standalone project
if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR OR LLVM_LIBGCC_STANDALONE_BUILD)
project(llvm-libgcc LANGUAGES C CXX ASM)
set(COMPILER_RT_STANDALONE_BUILD ON)
set_property(GLOBAL PROPERTY USE_FOLDERS ON)
set(LLVM_LIBGCC_COMPILER_RT_BINARY_DIR "compiler-rt")
set(LLVM_LIBGCC_LIBUNWIND_BINARY_DIR "libunwind")
else()
set(LLVM_LIBGCC_COMPILER_RT_BINARY_DIR "../compiler-rt")
set(LLVM_LIBGCC_LIBUNWIND_BINARY_DIR "../libunwind")
endif()And I am a bit confused how should we indicate "this is the driver for compiler-rt and libunwind". It is completely acceptable to perform project(llvm-libgcc LANGUAGES C CXX ASM) declaration unconditionally, but does it makes sense in this scenario?
There was a problem hiding this comment.
Understood, thanks for highlighting the change to llvm-libgcc/CMakeLists.txt, which I'd missed.
llvm-libgcc/CMakeLists.txt
Outdated
| set(COMPILER_RT_BUILD_BUILTINS On) | ||
| set(COMPILER_RT_BUILTINS_HIDE_SYMBOLS Off) | ||
| add_subdirectory("../compiler-rt" "compiler-rt") | ||
| if(NOT HAVE_COMPILER_RT) |
There was a problem hiding this comment.
This (and libunwind) need to be unconditional, since you shouldn't be building compiler-rt and libunwind independently when building llvm-libgcc.
There was a problem hiding this comment.
Yes, they are added unconditionally. HAVE_COMPILER_RT and HAVE_LIBUNWIND are set by runtimes/CMakeLists.txt if LLVM_ENABLE_RUNTIMES contains compiler-rt and libunwind. These checks are added to avoid target clashes when they are explicitly specified by users.
There was a problem hiding this comment.
The design is that you should not be adding compiler-rt or libunwind to LLVM_ENABLE_RUNTIMES. Why is this being changed?
There was a problem hiding this comment.
It was changed, and now it is restored in b61aed7.
I tried to make them co-exist happily and found that it is quite hard to set necessary options without further modifying the order of runtime projects in runtimes/CMakeLists.txt.
This finding is documented in the commit message to explain why we chose this approach.
llvm-libgcc/lib/blank.c
Outdated
There was a problem hiding this comment.
I'm happy you're able to delete this file.
There was a problem hiding this comment.
In the refactored approach, we modify libunwind.so added in libunwind directly and no custom shared library is required, so the dummy source file can be deleted.
|
@cjdb @compnerd I am sorry for the misleading statement in this PR's initial comment.
This statement shoule be refined: "refactors
At first, I tried to build Then I found that the original
My first refactored
Now And I found the target clashes were no longer irreconcilable, via detecting So there is no independent |
This commit adds targets of `llvm-libgcc` symlinks into the component, which fixes broken `llvm-libgcc` when installing only the component `llvm-libgcc`.
…nd compiler-rt/libunwind It is quite hard to set necessary options without further modifying the order of runtime projects in `runtimes/CMakeLists.txt`. This commit restores to inhibit co-existence of `llvm-libgcc` and `compiler-rt`/`libunwind` in commit c5a20b5 ([llvm-libgcc] initial commit).
…bgcc `libunwind.so` in `llvm-libgcc` is linked with clang_rt.builtins objects, thus dependency on compiler-rt or libgcc is uncessary and should be removed.
ur4t
force-pushed
the
refactor-llvm-libgcc
branch
from
641f643 to
656d900
Compare
llvmbot
added
libc++
|
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-libunwind ChangesThis PR:
Full diff: https://github.com/llvm/llvm-project/pull/65455.diff 5 Files Affected:
diff --git a/llvm-libgcc/CMakeLists.txt b/llvm-libgcc/CMakeLists.txt
index de42d908c37119d..013c9ca2e330766 100644
--- a/llvm-libgcc/CMakeLists.txt
+++ b/llvm-libgcc/CMakeLists.txt
@@ -1,19 +1,38 @@
-cmake_minimum_required(VERSION 3.20.0)
+#===============================================================================
+# Setup Project
+#===============================================================================
-if (NOT IS_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}/../libunwind")
- message(FATAL_ERROR "llvm-libgcc requires being built in a monorepo layout with libunwind available")
-endif()
+cmake_minimum_required(VERSION 3.20.0)
set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
-list(APPEND CMAKE_MODULE_PATH
+# Check if llvm-libgcc is built as a standalone project
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR OR LLVM_LIBGCC_STANDALONE_BUILD)
+ project(llvm-libgcc LANGUAGES C CXX ASM)
+ set(COMPILER_RT_STANDALONE_BUILD ON)
+ set_property(GLOBAL PROPERTY USE_FOLDERS ON)
+ set(LLVM_LIBGCC_COMPILER_RT_BINARY_DIR "compiler-rt")
+ set(LLVM_LIBGCC_LIBUNWIND_BINARY_DIR "libunwind")
+else()
+ set(LLVM_LIBGCC_COMPILER_RT_BINARY_DIR "../compiler-rt")
+ set(LLVM_LIBGCC_LIBUNWIND_BINARY_DIR "../libunwind")
+endif()
+
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
"${CMAKE_CURRENT_SOURCE_DIR}/cmake"
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules"
+ "${CMAKE_CURRENT_SOURCE_DIR}/../runtimes/cmake/Modules"
"${LLVM_COMMON_CMAKE_UTILS}"
"${LLVM_COMMON_CMAKE_UTILS}/Modules"
- )
+)
+
+set(LLVM_LIBGCC_LIBUNWIND_PATH "${CMAKE_CURRENT_LIST_DIR}/../libunwind"
+ CACHE PATH "Specify path to libunwind source.")
+set(LLVM_LIBGCC_COMPILER_RT_PATH "${CMAKE_CURRENT_LIST_DIR}/../compiler-rt"
+ CACHE PATH "Specify path to compiler-rt source.")
-project(llvm-libgcc LANGUAGES C CXX ASM)
+include(GNUInstallDirs)
if(NOT LLVM_LIBGCC_EXPLICIT_OPT_IN)
message(FATAL_ERROR
@@ -25,18 +44,114 @@ if(NOT LLVM_LIBGCC_EXPLICIT_OPT_IN)
"to your CMake invocation and try again.")
endif()
-set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/lib${LLVMLIB_DIR_SUFFIX}")
-set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/lib${LLVMLIB_DIR_SUFFIX}")
-set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin")
+if(HAVE_COMPILER_RT)
+ message(FATAL_ERROR
+ "Attempting to build both compiler-rt and llvm-libgcc will cause irreconcilable "
+ "target clashes. Please choose one or the other, but not both.")
+endif()
+
+if(HAVE_LIBUNWIND)
+ message(FATAL_ERROR
+ "Attempting to build both libunwind and llvm-libgcc will cause irreconcilable "
+ "target clashes. Please choose one or the other, but not both.")
+endif()
+
+#===============================================================================
+# Configure System
+#===============================================================================
+
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+ set(LLVM_LIBGCC_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
+ set(LLVM_LIBGCC_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE} CACHE PATH
+ "Path where built llvm-libgcc libraries should be installed.")
+ if(LIBCXX_LIBDIR_SUBDIR)
+ string(APPEND LLVM_LIBGCC_LIBRARY_DIR /${LLVM_LIBGCC_LIBDIR_SUBDIR})
+ string(APPEND LLVM_LIBGCC_INSTALL_LIBRARY_DIR /${LLVM_LIBGCC_LIBDIR_SUBDIR})
+ endif()
+else()
+ if(LLVM_LIBRARY_OUTPUT_INTDIR)
+ set(LLVM_LIBGCC_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
+ else()
+ set(LLVM_LIBGCC_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LLVM_LIBGCC_LIBDIR_SUFFIX})
+ endif()
+ set(LLVM_LIBGCC_INSTALL_LIBRARY_DIR lib${LLVM_LIBGCC_LIBDIR_SUFFIX} CACHE PATH
+ "Path where built llvm-libgcc libraries should be installed.")
+endif()
+
+set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_LIBGCC_LIBRARY_DIR})
+set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_LIBGCC_LIBRARY_DIR})
+set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_LIBGCC_LIBRARY_DIR})
+
+#===============================================================================
+# Build libraries
+#===============================================================================
+
+set(COMPILER_RT_BUILD_BUILTINS ON)
+set(COMPILER_RT_BUILTINS_HIDE_SYMBOLS OFF)
+add_subdirectory(${LLVM_LIBGCC_COMPILER_RT_PATH} ${LLVM_LIBGCC_COMPILER_RT_BINARY_DIR})
+
+set(LIBUNWIND_ENABLE_STATIC ON)
+set(LIBUNWIND_ENABLE_SHARED ON)
+set(LIBUNWIND_USE_COMPILER_RT OFF)
+set(LIBUNWIND_HAS_GCC_LIB OFF)
+set(LIBUNWIND_HAS_GCC_S_LIB OFF)
+add_subdirectory(${LLVM_LIBGCC_LIBUNWIND_PATH} ${LLVM_LIBGCC_LIBUNWIND_BINARY_DIR})
+
+add_custom_target(gcc_s.ver
+ SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/gcc_s.ver.in
+ COMMAND ${CMAKE_C_COMPILER} -E
+ -xc ${CMAKE_CURRENT_SOURCE_DIR}/gcc_s.ver.in
+ -o ${CMAKE_CURRENT_BINARY_DIR}/gcc_s.ver
+)
+
+add_dependencies(unwind_shared gcc_s.ver)
+
+construct_compiler_rt_default_triple()
+
+target_link_options(unwind_shared PUBLIC
+ -Wl,--version-script,${CMAKE_CURRENT_BINARY_DIR}/gcc_s.ver
+)
+
+target_link_libraries(unwind_shared PUBLIC
+ $<TARGET_OBJECTS:clang_rt.builtins-${COMPILER_RT_DEFAULT_TARGET_ARCH}>
+ m
+)
+
+#===============================================================================
+# Install Symlinks
+#===============================================================================
+
+get_compiler_rt_install_dir(${COMPILER_RT_DEFAULT_TARGET_ARCH} install_dir_builtins)
+string(REGEX REPLACE "^lib/" "" install_dir_builtins "${install_dir_builtins}")
+string(FIND "${install_dir_builtins}" "clang" install_path_contains_triple)
+if(install_path_contains_triple EQUAL -1)
+ set(builtins_suffix "-${COMPILER_RT_DEFAULT_TARGET_ARCH}")
+else()
+ string(PREPEND install_dir_builtins "../")
+endif()
+set(LLVM_LIBGCC_COMPILER_RT ${install_dir_builtins}/libclang_rt.builtins${builtins_suffix}.a)
+
+add_custom_target(llvm-libgcc ALL
+ DEPENDS unwind_shared unwind_static clang_rt.builtins-${COMPILER_RT_DEFAULT_TARGET_ARCH}
+ COMMAND ${CMAKE_COMMAND} -E create_symlink ${LLVM_LIBGCC_COMPILER_RT} libgcc.a
+ COMMAND ${CMAKE_COMMAND} -E create_symlink libunwind.a libgcc_eh.a
+ COMMAND ${CMAKE_COMMAND} -E create_symlink libunwind.so libgcc_s.so.1.0
+ COMMAND ${CMAKE_COMMAND} -E create_symlink libgcc_s.so.1.0 libgcc_s.so.1
+ COMMAND ${CMAKE_COMMAND} -E create_symlink libgcc_s.so.1 libgcc_s.so
+)
-set(COMPILER_RT_BUILD_BUILTINS On)
-set(COMPILER_RT_BUILTINS_HIDE_SYMBOLS Off)
-add_subdirectory("../compiler-rt" "compiler-rt")
+install(TARGETS unwind_shared unwind_static
+ LIBRARY DESTINATION ${LLVM_LIBGCC_INSTALL_LIBRARY_DIR} COMPONENT llvm-libgcc
+ ARCHIVE DESTINATION ${LLVM_LIBGCC_INSTALL_LIBRARY_DIR} COMPONENT llvm-libgcc
+ RUNTIME DESTINATION ${LLVM_LIBGCC_INSTALL_RUNTIME_DIR} COMPONENT llvm-libgcc)
-set(LIBUNWIND_ENABLE_STATIC On)
-set(LIBUNWIND_ENABLE_SHARED Off)
-set(LIBUNWIND_HAS_COMMENT_LIB_PRAGMA Off)
-set(LIBUNWIND_USE_COMPILER_RT On)
-add_subdirectory("../libunwind" "libunwind")
+install(TARGETS clang_rt.builtins-${COMPILER_RT_DEFAULT_TARGET_ARCH}
+ LIBRARY DESTINATION ${LLVM_LIBGCC_INSTALL_LIBRARY_DIR}/${install_dir_builtins} COMPONENT llvm-libgcc
+ ARCHIVE DESTINATION ${LLVM_LIBGCC_INSTALL_LIBRARY_DIR}/${install_dir_builtins} COMPONENT llvm-libgcc
+ RUNTIME DESTINATION ${LLVM_LIBGCC_INSTALL_RUNTIME_DIR}/${install_dir_builtins} COMPONENT llvm-libgcc)
-add_subdirectory(lib)
+foreach(VAR libgcc.a libgcc_eh.a libgcc_s.so.1.0 libgcc_s.so.1 libgcc_s.so)
+ install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${VAR}
+ DESTINATION ${LLVM_LIBGCC_INSTALL_LIBRARY_DIR}
+ COMPONENT llvm-libgcc)
+endforeach()
diff --git a/llvm-libgcc/lib/gcc_s.ver b/llvm-libgcc/gcc_s.ver.in
similarity index 100%
rename from llvm-libgcc/lib/gcc_s.ver
rename to llvm-libgcc/gcc_s.ver.in
diff --git a/llvm-libgcc/lib/CMakeLists.txt b/llvm-libgcc/lib/CMakeLists.txt
deleted file mode 100644
index d895a21554b03c9..000000000000000
--- a/llvm-libgcc/lib/CMakeLists.txt
+++ /dev/null
@@ -1,86 +0,0 @@
-include(CheckLibraryExists)
-include(GNUInstallDirs)
-include(ExtendPath)
-
-string(REPLACE "-Wl,-z,defs" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}")
-
-add_custom_target(gcc_s_ver ALL DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/gcc_s.ver")
-set(LLVM_LIBGCC_GCC_S_VER "${CMAKE_CURRENT_BINARY_DIR}/gcc_s.ver")
-
-add_custom_target(gcc_s.ver ALL
- DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/gcc_s.ver"
- COMMAND
- "${CMAKE_C_COMPILER}"
- "-E"
- "-xc" "${CMAKE_CURRENT_SOURCE_DIR}/gcc_s.ver"
- "-o" "${CMAKE_CURRENT_BINARY_DIR}/gcc_s.ver"
-)
-set_target_properties(gcc_s.ver PROPERTIES
- OUTPUT_PATH "${CMAKE_CURRENT_BINARY_DIR}/gcc_s.ver")
-
-add_library(libgcc_s SHARED blank.c)
-add_dependencies(libgcc_s gcc_s_ver)
-set_target_properties(libgcc_s
- PROPERTIES
- LINKER_LANGUAGE C
- OUTPUT_NAME "unwind"
- VERSION "1.0"
- SOVERSION "1"
- POSITION_INDEPENDENT_CODE ON)
-string(REGEX MATCH "[^-]+" LLVM_LIBGCC_TARGET_ARCH ${CMAKE_C_COMPILER_TARGET})
-target_link_libraries(libgcc_s PRIVATE
- $<TARGET_OBJECTS:unwind_static>
- $<TARGET_OBJECTS:clang_rt.builtins-${LLVM_LIBGCC_TARGET_ARCH}>
-)
-target_link_options(libgcc_s PRIVATE
- -nostdlib
- -Wl,--version-script,$<TARGET_PROPERTY:gcc_s.ver,OUTPUT_PATH>)
-
-check_library_exists(m sin "" LLVM_LIBGCC_HAS_LIBM)
-target_link_libraries(libgcc_s PRIVATE
- $<$<BOOL:LLVM_LIBGCC_HAS_LIBM>:m>
- c
-)
-
-extend_path(LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT "${CMAKE_INSTALL_PREFIX}" "${LIBUNWIND_INSTALL_LIBRARY_DIR}")
-#string(REPLACE "${CMAKE_INSTALL_FULL_LIBDIR}/" "" LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT "${LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT}")
-
-install(TARGETS libgcc_s
- LIBRARY DESTINATION "${LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT}" COMPONENT unwind
- ARCHIVE DESTINATION "${LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT}" COMPONENT unwind
- RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}" COMPONENT unwind)
-
-get_compiler_rt_install_dir(${LLVM_LIBGCC_TARGET_ARCH} install_dir_builtins)
-string(REGEX REPLACE "^lib/" "" install_dir_builtins "${install_dir_builtins}")
-string(FIND "${install_dir_builtins}" "clang" install_path_contains_triple)
-if(install_path_contains_triple EQUAL -1)
- set(builtins_suffix "-${LLVM_LIBGCC_TARGET_ARCH}")
-else()
- string(PREPEND install_dir_builtins "../")
-endif()
-install(CODE "execute_process(
- COMMAND \"\${CMAKE_COMMAND}\" -E
- create_symlink ${install_dir_builtins}/libclang_rt.builtins${builtins_suffix}.a libgcc.a
- WORKING_DIRECTORY \"\$ENV{DESTDIR}${LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT}\")"
- COMPONENT unwind)
-
-install(CODE "execute_process(
- COMMAND \"\${CMAKE_COMMAND}\" -E
- create_symlink libunwind.a libgcc_eh.a
- WORKING_DIRECTORY \"\$ENV{DESTDIR}${LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT}\")"
- COMPONENT unwind)
-install(CODE "execute_process(
- COMMAND \"\${CMAKE_COMMAND}\" -E
- create_symlink libunwind.so libgcc_s.so.1.0
- WORKING_DIRECTORY \"\$ENV{DESTDIR}${LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT}\")"
- COMPONENT unwind)
-install(CODE "execute_process(
- COMMAND \"\${CMAKE_COMMAND}\" -E
- create_symlink libgcc_s.so.1.0 libgcc_s.so.1
- WORKING_DIRECTORY \"\$ENV{DESTDIR}${LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT}\")"
- COMPONENT unwind)
-install(CODE "execute_process(
- COMMAND \"\${CMAKE_COMMAND}\" -E
- create_symlink libgcc_s.so.1 libgcc_s.so
- WORKING_DIRECTORY \"\$ENV{DESTDIR}${LLVM_LIBGCC_LIBUNWIND_STATIC_ROOT}\")"
- COMPONENT unwind)
diff --git a/llvm-libgcc/lib/blank.c b/llvm-libgcc/lib/blank.c
deleted file mode 100644
index e69de29bb2d1d64..000000000000000
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 599529852688f25..06f51243827cdd7 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -212,23 +212,6 @@ if(LLVM_INCLUDE_TESTS)
umbrella_lit_testsuite_begin(check-runtimes)
endif()
-# llvm-libgcc incorporates both compiler-rt and libunwind as subprojects with very
-# specific flags, which causes clashes when they're independently built too.
-if("llvm-libgcc" IN_LIST runtimes)
- if("compiler-rt" IN_LIST runtimes OR "compiler-rt" IN_LIST LLVM_ENABLE_PROJECTS)
- message(FATAL_ERROR
- "Attempting to build both compiler-rt and llvm-libgcc will cause irreconcilable "
- "target clashes. Please choose one or the other, but not both.")
- endif()
-
- if("libunwind" IN_LIST runtimes)
- message(
- FATAL_ERROR
- "Attempting to build both libunwind and llvm-libgcc will cause irreconcilable "
- "target clashes. Please choose one or the other, but not both.")
- endif()
-endif()
-
# We do this in two loops so that HAVE_* is set for each runtime before the
# other runtimes are added.
foreach(entry ${runtimes})
|
|
Are we waiting on anything to merge this? I think folks' expectations of how patches are merged might be unclear right now due to the Github PR transition. With Phabricator the patch author would push the change and the "PR" would be closed automatically. Now with Github PRs someone has to press "squash and merge" but it might not be clear whose responsibility it is to do this. @cjdb @MaskRay We're ready to land this, right? If so, I think either of you can "Squash and merge" (or @ur4t if they are able to). |
|
Right, it's ready to land. Thanks for pointing out that the workflow is different now. |
|
@ur4t before I squash and merge, could you please provide me with a commit message that explains why the changes were necessary (as a comment)? I've looked at all the commit messages and text, but I don't see anything that captures the full message. If you've already written something, a link to that will be fine :) |
|
I agree with a better description is needed.
I feel the author is the best one to perform the action, as they can adjust the description to be clearer, rebase as needed, or do other minor adjustments that do not need re-review. |
FWIW that may not always be possible for folks who don't have direct commit access to the repo. But feel free to handle that however makes the most sense to you. For libc++/libc++abi, I often merge on behalf of folks to make sure that the PR isn't forgotten, etc. |
|
@cjdb Sorry for the circuitous commit history. I am glad to provide a summary. [llvm-libgcc][CMake] Refactor llvm-libgcc There are some issues in Commit c5a20b5 ([llvm-libgcc] initial commit) When configuring with Per target installation is not handled by This patch fixes those issues by: Reusing target Adding target Mimicking It is quite hard to set necessary options without further modifying the order of |
There are some issues in `llvm-libgcc` before this patch: Commit c5a20b5 ([llvm-libgcc] initial commit) uses `$<TARGET_OBJECTS:unwind_static>` to get libunwind objects, which is empty. The built library is actually a shared version of libclang_rt.builtins. When configuring with `llvm/CMakeLists.txt`, target `llvm-libgcc` requires a corresponding target in `llvm-libgcc/CMakeLists.txt`. Per target installation is not handled by `llvm-libgcc`, which is not consistent with `libunwind`. This patch fixes those issues by: Reusing target `unwind_shared` in `libunwind`, linking `compiler-rt.builtins` objects into it, and applying version script. Adding target `llvm-libgcc`, creating symlinks, and utilizing cmake's dependency and component mechanism to ensure link targets will be built and installed along with symlinks. Mimicking `libunwind` to handle per target installation. It is quite hard to set necessary options without further modifying the order of runtime projects in `runtimes/CMakeLists.txt`. So though this patch reveals the possibility of co-existence of `llvm-libgcc` and `compiler-rt`/`libunwind` in `LLVM_ENABLE_RUNTIMES`, we still inhibit it to minimize influence on other projects, considering that `llvm-libgcc` is only intended for toolchain vendors, and not for casual use.
|
@MaskRay It seems that Github enforce a column limit of 70 characters in "Squash and Merge" commit messages (refined-github#1475). Commit messages targeting a 80 character limitation will be truncated again when texts are copied and pasted directly with original line breakages (7ac1f60 and af935cf). Should we mention this behaviour in documents for the Github PR transition? |
Thanks for mentioning this behavior. Perhaps comment on https://discourse.llvm.org/t/hows-it-going-with-pull-requests/73467 or a related github pr transition post? If github has such a truncation problem, i'll try to avoid the web ui completely... Due to email spam from github notifications, I almost missed your comment... |
There are some issues in
llvm-libgccbefore this patch:Commit c5a20b5 ([llvm-libgcc] initial commit)
uses
$<TARGET_OBJECTS:unwind_static>to get libunwind objects, which is empty.The built library is actually a shared version of libclang_rt.builtins.
When configuring with
llvm/CMakeLists.txt, targetllvm-libgccrequires acorresponding target in
llvm-libgcc/CMakeLists.txt.Per target installation is not handled by
llvm-libgcc, which is not consistentwith
libunwind.This patch fixes those issues by:
Reusing target
unwind_sharedinlibunwind, linkingcompiler-rt.builtinsobjects into it, and applying version script.
Adding target
llvm-libgcc, creating symlinks, and utilizing cmake's dependencyand component mechanism to ensure link targets will be built and installed along
with symlinks.
Mimicking
libunwindto handle per target installation.It is quite hard to set necessary options without further modifying the order of
runtime projects in
runtimes/CMakeLists.txt. So though this patch reveals thepossibility of co-existence of
llvm-libgccandcompiler-rt/libunwindinLLVM_ENABLE_RUNTIMES, we still inhibit it to minimize influence on otherprojects, considering that
llvm-libgccis only intended for toolchain vendors,and not for casual use.