[Fuchsia][cmake] Allow using FatLTO when building runtimes #112277
Conversation
Created using spr 1.3.4
|
@petrhosek originally I had used a helper function to do add these flags in the libcxx top level cmake, but that didn't work (similar to this llvm-project/libcxx/CMakeLists.txt Line 567 in d0d54fa add_flags directly, or appending to the list, vs. doing something here.
|
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
|
Why cannot we just set |
I don't think we want to couple building libc++ with FATLTO to whether you want to build the compiler that way, do we? Isn't how libc++ built orthogonal to the other build options? Once we have a way to spell that in the build, then yes, we should update the cache file, and we'll probably key that off of But even if we do change the spelling to reuse |
|
I think I understand what you’re asking, now. I had the values default to ON for testing, but I’ll update them to default to OFF, and update the cache file so that they can be on in our build. |
|
We use It should be sufficient to simply set |
Created using spr 1.3.4
|
Oh, I finally see what you mean. In the per target runtimes, none of the top level flags get propagated unless you specify them w/ the correct target prefix. I was so confused about why things worked this way. |
|
@llvm/pr-subscribers-libcxxabi @llvm/pr-subscribers-clang Author: Paul Kirth (ilovepi) ChangesWe'd like to build libc++ using FatLTO (see https://llvm.org/docs/FatLTO.html Full diff: https://github.com/llvm/llvm-project/pull/112277.diff 1 Files Affected:
diff --git a/clang/cmake/caches/Fuchsia-stage2.cmake b/clang/cmake/caches/Fuchsia-stage2.cmake
index 5af98c7b3b3fba..e62f29ecbe6f45 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -192,6 +192,10 @@ foreach(target aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unkn
set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL "")
set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
+ # Enable FatLTO for Linux and baremetal runtimes
+ set(RUNTIMES_${target}_LLVM_ENABLE_LTO ON CACHE BOOL "")
+ set(RUNTIMES_${target}_LLVM_ENABLE_FATLTO ON CACHE BOOL "")
+
# Use .build-id link.
list(APPEND RUNTIME_BUILD_ID_LINK "${target}")
endif()
@@ -274,6 +278,10 @@ if(FUCHSIA_SDK)
set(RUNTIMES_${target}+asan+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
set(RUNTIMES_${target}+asan+noexcept_LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
+ # Enable FatLTO for Fuchsia runtimes
+ set(RUNTIMES_${target}_LLVM_ENABLE_LTO ON CACHE BOOL "")
+ set(RUNTIMES_${target}_LLVM_ENABLE_FATLTO ON CACHE BOOL "")
+
# Use .build-id link.
list(APPEND RUNTIME_BUILD_ID_LINK "${target}")
endforeach()
|
ilovepi
changed the title
|
ping. |
|
I'm now getting an error w/ ToT. Not sure why I'm running into it now, but it happens with any clean build, so its deterministic. The Relevant bits: ld.lld: error: input file '/usr/local/google/home/paulkirth/fuchsia-idk/arch/arm64/sysroot/lib/libzircon.so' added after LTO
ld.lld: error: input file '/usr/local/google/home/paulkirth/fuchsia-idk/arch/arm64/sysroot/lib/libzircon.so' added after LTOFull Error Log: [1761/1802] Linking CXX shared library /usr/local/google/home/paulkirth/llvm-fork/build/lib/aarch64-unknown-fuchsia/libc++abi.so.1.0
FAILED: /usr/local/google/home/paulkirth/llvm-fork/build/lib/aarch64-unknown-fuchsia/libc++abi.so.1.0
: && /usr/local/google/home/paulkirth/llvm-fork/build/./bin/clang++ --target=aarch64-unknown-fuchsia --sysroot=/usr/local/google/home/paulkirth/fuchsia-idk/arch/arm64/sysroot -fPIC --target=aarch64-unknown-fuchsia -I/usr/local/google/home/paulkirth/fuchsia-idk/pkg/sync/include -I/usr/local/google/home/paulkirth/fuchsia-idk/pkg/fdio/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -ffunction-sections -fdata-sections -flto -ffat-lto-objects -O2 -g -DNDEBUG -L/usr/local/google/home/paulkirth/fuchsia-idk/arch/arm64/lib -Wl,-z,defs -fuse-ld=lld -flto -ffat-lto-objects -nostdlib++ --unwindlib=none -shared -Wl,-soname,libc++abi.so.1 -o /usr/local/google/home/paulkirth/llvm-fork/build/lib/aarch64-unknown-fuchsia/libc++abi.so.1.0 libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_demangle.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_exception_storage.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_guard.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_handlers.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_vector.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_virtual.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/stdlib_exception.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/stdlib_stdexcept.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/stdlib_typeinfo.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/abort_message.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/fallback_malloc.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/private_typeinfo.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/stdlib_new_delete.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_exception.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_personality.cpp.obj libcxxabi/src/CMakeFiles/cxxabi_shared_objects.dir/cxa_thread_atexit.cpp.obj -lc /usr/local/google/home/paulkirth/llvm-fork/build/lib/aarch64-unknown-fuchsia/libunwind.so.1.0 && :
ld.lld: error: input file '/usr/local/google/home/paulkirth/fuchsia-idk/arch/arm64/sysroot/lib/libzircon.so' added after LTO
ld.lld: error: input file '/usr/local/google/home/paulkirth/fuchsia-idk/arch/arm64/sysroot/lib/libzircon.so' added after LTO
ld.lld: error: undefined symbol: _zx_system_get_features
>>> referenced by fuchsia.inc:9 (/usr/local/google/home/paulkirth/llvm-fork/compiler-rt/lib/builtins/cpu_model/aarch64/lse_atomics/fuchsia.inc:9)
>>> aarch64.c.obj:(init_have_lse_atomics) in archive /usr/local/google/home/paulkirth/llvm-fork/build/lib/clang/20/lib/aarch64-unknown-fuchsia/libclang_rt.builtins.a
>>> referenced by fuchsia.inc:12 (/usr/local/google/home/paulkirth/llvm-fork/compiler-rt/lib/builtins/cpu_model/aarch64/fmv/fuchsia.inc:12)
>>> aarch64.c.obj:(__init_cpu_features_resolver) in archive /usr/local/google/home/paulkirth/llvm-fork/build/lib/clang/20/lib/aarch64-unknown-fuchsia/libclang_rt.builtins.a
clang++: error: ld.lld command failed with exit code 1 (use -v to see invocation)
[1763/1802] Building CXX object libcxx/src/CMakeFiles/cxx_static.dir/locale.cpp.obj
ninja: build stopped: subcommand failed. |
|
I think this is due to which I believe is yet another instance of #56070. |
|
I'm pretty sure you're correct on the root cause. In this case its the This happens for LTO alone, and isn't something I think we can work around through FatLTO, unless we disable using LTO to link libcxxabi (eg. --no-fat-lto-objects). Since I don't think this is a libcxxabi specific problem, I'm not sure that's necessarily a good idea, and won't just break somewhere else (either in the toolchain build or when building Fuchsia code). Unsurprisingly, adding #56070 has enough to say about why this is hard to fix in LLD/generally, but IDK how to solve this for our usecase ATM. @mysterymath I recall you mentioned a new libcall mechanism that was being used that may help w/ some of deplibs mess. Do you have any thoughts on if that can be adapted in some way to help resolve these? |
|
The rule established in LLD is that all input files to the link must be known before LTO begins. This is because a set of properties computed from the input files is used as input to LTO codegen, and the results of codegen are not necessarily valid if those properties were to change by the addition of new input files. In the short term, I'd recommend CMake plumbing to ensure that In the longer term, as @ilovepi mentioned, we now have a relatively accurate list of libcalls available in LLD. This is already used to summarily pull in bitcode that defines potential-used libcalls. We could also scan the files that define potentially-used libcalls for deplibs, and summarily include those in the link. I have an old Phabricator patch for this. The main issue with it was IIRC the libcall listing mechanism it added to LLD, but that has been addressed by the libcall list work. |
Created using spr 1.3.4 [skip ci]
Created using spr 1.3.4
llvmbot
added
cmake
Created using spr 1.3.4 [skip ci]
Created using spr 1.3.4 [skip ci]
Created using spr 1.3.4
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
ilovepi
changed the base branch from
main
to
users/ilovepi/spr/main.fuchsiacmake-allow-using-fatlto-when-building-runtimes-3
| @@ -1285,7 +1285,7 @@ elseif(LLVM_ENABLE_LTO) | |||
| endif() | |||
| endif() | |||
|
|
|||
| if(LLVM_ENABLE_FATLTO AND UNIX AND NOT APPLE) | |||
| if(LLVM_ENABLE_FATLTO AND ((UNIX AND NOT APPLE) OR FUCHSIA)) | |||
There was a problem hiding this comment.
I don't understand why that diff is required since you're setting it properly in your cache files.
There was a problem hiding this comment.
Because Fuchsia Targets aren't UNIX. Without this change -ffat-lto-objects won't be passed to clang through CMake. The additional logic is basically to restrict the flag to ELF targets, and it missed Fuchsia when we added it, since we weren't building our runtimes with FatLTO initially, just the toolchain + tests.
There was a problem hiding this comment.
Sorry, I think I initially read if (LLVM_ENABLE_FATLTO OR ((UNIX AND NOT APPLE) OR FUCHSIA)).
| @@ -1285,7 +1285,7 @@ elseif(LLVM_ENABLE_LTO) | |||
| endif() | |||
| endif() | |||
|
|
|||
| if(LLVM_ENABLE_FATLTO AND UNIX AND NOT APPLE) | |||
| if(LLVM_ENABLE_FATLTO AND ((UNIX AND NOT APPLE) OR FUCHSIA)) | |||
There was a problem hiding this comment.
Sorry, I think I initially read if (LLVM_ENABLE_FATLTO OR ((UNIX AND NOT APPLE) OR FUCHSIA)).
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
ilovepi
changed the base branch from
users/ilovepi/spr/main.fuchsiacmake-allow-using-fatlto-when-building-runtimes-3
to
main
ilovepi
deleted the
users/ilovepi/spr/libcxxcmake-allow-using-fatlto-in-libc-builds
branch
…119252) Reverts #112277 This broke something on Fuchsia's Mac builders, so there's still something in the CMake that needs to be updated before we reland. Failed build: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-mac-xarm64/b8729005878443108801/overview
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/4569 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1797 Here is the relevant piece of the build log for the reference |
We'd like to build runtimes using FatLTO (see
https://llvm.org/docs/FatLTO.html for details). This gives us more
control over how libc++ can be consumed by users of our toolchain, like
the Fuchsia SDK.