Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fuchsia][cmake] Allow using FatLTO when building runtimes #112277

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ilovepi
Copy link
Contributor

@ilovepi ilovepi commented Oct 14, 2024

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.

Created using spr 1.3.4
@ilovepi
Copy link
Contributor Author

ilovepi commented Oct 14, 2024

@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

# RTTI flags ==================================================================
). I'm pretty confused why that approach failed, even when use 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
@petrhosek
Copy link
Member

Why cannot we just set LLVM_ENABLE_FATLTO in our cache files?

@ilovepi
Copy link
Contributor Author

ilovepi commented Oct 15, 2024

Why cannot we just set LLVM_ENABLE_FATLTO in our cache files?

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 LLVM_ENABLE_FATLTO.

But even if we do change the spelling to reuse LLVM_ENABLE_FATLTO, I don't understand is why I couldn't meaningfully change the build flags for libc++ by following the example linked above and adding a call to the new function in cxx_add_common_build_flags(). The fact that I could only get it to work this way seems wrong, and kind of hacky.

@ilovepi
Copy link
Contributor Author

ilovepi commented Oct 15, 2024

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.

@petrhosek
Copy link
Member

We use HandleLLVMOptions in the runtimes build and support existing LLVM options. That's why there's no LIBCXX_USE_SANITIZER (and LIBCXXABI_USE_SANITIZER or LIBUNWIND_USE_SANITIZER), instead we simply reuse LLVM_USE_SANITIZER. That's why LLVM_ENABLE_FATLTO should already work and we shouldn't be introducing new options like LIBCXX_ENABLE_FATLTO.

It should be sufficient to simply set LLVM_ENABLE_FATLTO in the cache file like https://github.com/llvm/llvm-project/blob/e55869ae8a4ef1ae2e898ff5cd66fb8ae6e099b8/clang/cmake/caches/Fuchsia-stage2.cmake and most importantly test that everything is working as expected (and address any issues as needed).

@ilovepi
Copy link
Contributor Author

ilovepi commented Oct 16, 2024

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.

@ilovepi ilovepi marked this pull request as ready for review October 16, 2024 18:24
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Oct 16, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 16, 2024

@llvm/pr-subscribers-clang

Author: Paul Kirth (ilovepi)

Changes

We'd like to build libc++ 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.


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

1 Files Affected:

  • (modified) clang/cmake/caches/Fuchsia-stage2.cmake (+8)
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 ilovepi changed the title [libcxx][cmake] Allow using FatLTO in libc++ builds [Fuchsia][cmake] Allow using FatLTO when building runtimes Oct 16, 2024
@ilovepi
Copy link
Contributor Author

ilovepi commented Oct 30, 2024

ping.

@ilovepi
Copy link
Contributor Author

ilovepi commented Oct 31, 2024

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 LTO

Full 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants