Skip to content

[libc][libcxx] Support for building libc++ against LLVM libc #99287

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

Merged
merged 8 commits into from
Jul 19, 2024

Conversation

petrhosek
Copy link
Member

Provide an option to build libc++ against LLVM libc and set the CMake compile and link options appropriately when the option is enabled.

Provide an option to build libc++ against LLVM libc and set the CMake
compile and link options appropriately when the option is enabled.
@petrhosek petrhosek added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc labels Jul 17, 2024
@petrhosek petrhosek requested a review from a team as a code owner July 17, 2024 07:53
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jul 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 17, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-libc

Author: Petr Hosek (petrhosek)

Changes

Provide an option to build libc++ against LLVM libc and set the CMake compile and link options appropriately when the option is enabled.


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

7 Files Affected:

  • (modified) clang/cmake/caches/Fuchsia-stage2.cmake (+2)
  • (modified) libc/cmake/modules/CheckCompilerFeatures.cmake (+3)
  • (modified) libc/include/CMakeLists.txt (+3)
  • (modified) libc/lib/CMakeLists.txt (+4-1)
  • (modified) libcxx/CMakeLists.txt (+1)
  • (modified) libcxx/include/CMakeLists.txt (+3)
  • (modified) libcxx/src/CMakeLists.txt (+10)
diff --git a/clang/cmake/caches/Fuchsia-stage2.cmake b/clang/cmake/caches/Fuchsia-stage2.cmake
index b4561e6c87ba5..dc9b596b4ba8f 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -338,6 +338,7 @@ foreach(target armv6m-unknown-eabi;armv7m-unknown-eabi;armv8m.main-unknown-eabi)
   set(RUNTIMES_${target}_LIBCXX_CXX_ABI none CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC ON CACHE BOOL "")
+  set(RUNTIMES_${target}_LIBCXX_USE_LLVM_LIBC ON CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_LOCALIZATION OFF CACHE BOOL "")
@@ -388,6 +389,7 @@ foreach(target riscv32-unknown-elf)
   set(RUNTIMES_${target}_LIBCXX_CXX_ABI none CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC ON CACHE BOOL "")
+  set(RUNTIMES_${target}_LIBCXX_USE_LLVM_LIBC ON CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_LOCALIZATION OFF CACHE BOOL "")
diff --git a/libc/cmake/modules/CheckCompilerFeatures.cmake b/libc/cmake/modules/CheckCompilerFeatures.cmake
index a6d793d495c45..d64d9de97af3c 100644
--- a/libc/cmake/modules/CheckCompilerFeatures.cmake
+++ b/libc/cmake/modules/CheckCompilerFeatures.cmake
@@ -102,3 +102,6 @@ check_cxx_compiler_flag("-nostdlib++" LIBC_CC_SUPPORTS_NOSTDLIBPP)
 
 # clang-3.0+
 check_cxx_compiler_flag("-nostdlibinc" LIBC_CC_SUPPORTS_NOSTDLIBINC)
+
+# clang-3.0+
+check_cxx_compiler_flag("-nolibc" LIBC_CC_SUPPORTS_NOLIBC)
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 2cf7206f3a625..d521e205df8c3 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -664,6 +664,9 @@ get_all_install_header_targets(all_install_header_targets ${TARGET_PUBLIC_HEADER
 add_library(libc-headers INTERFACE)
 add_dependencies(libc-headers ${all_install_header_targets})
 target_include_directories(libc-headers SYSTEM INTERFACE ${LIBC_INCLUDE_DIR})
+if(LIBC_CC_SUPPORTS_NOSTDLIBINC)
+  target_compile_options(libc-headers INTERFACE "-nostdlibinc")
+endif()
 
 foreach(target IN LISTS all_install_header_targets)
   get_target_property(header_file ${target} HEADER_FILE_PATH)
diff --git a/libc/lib/CMakeLists.txt b/libc/lib/CMakeLists.txt
index 37acf3950b460..7e18f35e7d60e 100644
--- a/libc/lib/CMakeLists.txt
+++ b/libc/lib/CMakeLists.txt
@@ -30,7 +30,10 @@ foreach(archive IN ZIP_LISTS
       ARCHIVE_OUTPUT_NAME ${archive_0}
   )
   if(LLVM_LIBC_FULL_BUILD)
-    target_link_libraries(${archive_1} PUBLIC libc-headers)
+    target_link_libraries(${archive_1} INTERFACE libc-headers)
+    if(LIBC_CC_SUPPORTS_NOLIBC)
+      target_link_options(${archive_1} INTERFACE "-nolibc")
+    endif()
     if(TARGET libc-startup)
       add_dependencies(${archive_1} libc-startup)
     endif()
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 190a97db9462f..5a568e95b239e 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -293,6 +293,7 @@ option(LIBCXX_ENABLE_THREADS "Build libc++ with support for threads." ON)
 option(LIBCXX_ENABLE_MONOTONIC_CLOCK
   "Build libc++ with support for a monotonic clock.
    This option may only be set to OFF when LIBCXX_ENABLE_THREADS=OFF." ON)
+option(LIBCXX_USE_LLVM_LIBC "Build libc++ against LLVM libc." OFF)
 option(LIBCXX_HAS_MUSL_LIBC "Build libc++ with support for the Musl C library" OFF)
 option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)
 option(LIBCXX_HAS_WIN32_THREAD_API "Ignore auto-detection and force use of win32 thread API" OFF)
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index cd64fe91449c2..31a819932d521 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -1043,6 +1043,9 @@ add_custom_target(generate-cxx-headers ALL DEPENDS ${_all_includes})
 
 add_library(cxx-headers INTERFACE)
 target_link_libraries(cxx-headers INTERFACE libcxx-abi-headers)
+if (LIBCXX_USE_LLVM_LIBC)
+  target_link_libraries(cxx-headers INTERFACE libc-headers)
+endif()
 add_dependencies(cxx-headers generate-cxx-headers)
 # It's important that the arch directory be included first so that its header files
 # which interpose on the default include dir be included instead of the default ones.
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 0ae58a10c879c..536a5a16ad7e1 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -229,6 +229,11 @@ if (LIBCXX_ENABLE_SHARED)
     target_link_libraries(cxx_shared PUBLIC libcxx-abi-shared)
   endif()
 
+  # Link against LLVM libc.
+  if (LIBCXX_USE_LLVM_LIBC)
+    target_link_libraries(cxx_shared PUBLIC libc libm)
+  endif()
+
   # Maybe re-export symbols from libc++abi
   # In particular, we don't re-export the symbols if libc++abi is merged statically
   # into libc++ because in that case there's no dylib to re-export from.
@@ -324,6 +329,11 @@ if (LIBCXX_ENABLE_STATIC)
   if (LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY)
     target_link_libraries(cxx_static PRIVATE libcxx-abi-static-objects)
   endif()
+
+  # Link against LLVM libc.
+  if (LIBCXX_USE_LLVM_LIBC)
+    target_link_libraries(cxx_static PUBLIC libc libm)
+  endif()
 endif()
 
 # Add a meta-target for both libraries.

@llvmbot
Copy link
Member

llvmbot commented Jul 17, 2024

@llvm/pr-subscribers-libcxx

Author: Petr Hosek (petrhosek)

Changes

Provide an option to build libc++ against LLVM libc and set the CMake compile and link options appropriately when the option is enabled.


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

7 Files Affected:

  • (modified) clang/cmake/caches/Fuchsia-stage2.cmake (+2)
  • (modified) libc/cmake/modules/CheckCompilerFeatures.cmake (+3)
  • (modified) libc/include/CMakeLists.txt (+3)
  • (modified) libc/lib/CMakeLists.txt (+4-1)
  • (modified) libcxx/CMakeLists.txt (+1)
  • (modified) libcxx/include/CMakeLists.txt (+3)
  • (modified) libcxx/src/CMakeLists.txt (+10)
diff --git a/clang/cmake/caches/Fuchsia-stage2.cmake b/clang/cmake/caches/Fuchsia-stage2.cmake
index b4561e6c87ba5..dc9b596b4ba8f 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -338,6 +338,7 @@ foreach(target armv6m-unknown-eabi;armv7m-unknown-eabi;armv8m.main-unknown-eabi)
   set(RUNTIMES_${target}_LIBCXX_CXX_ABI none CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC ON CACHE BOOL "")
+  set(RUNTIMES_${target}_LIBCXX_USE_LLVM_LIBC ON CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_LOCALIZATION OFF CACHE BOOL "")
@@ -388,6 +389,7 @@ foreach(target riscv32-unknown-elf)
   set(RUNTIMES_${target}_LIBCXX_CXX_ABI none CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC ON CACHE BOOL "")
+  set(RUNTIMES_${target}_LIBCXX_USE_LLVM_LIBC ON CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_LOCALIZATION OFF CACHE BOOL "")
diff --git a/libc/cmake/modules/CheckCompilerFeatures.cmake b/libc/cmake/modules/CheckCompilerFeatures.cmake
index a6d793d495c45..d64d9de97af3c 100644
--- a/libc/cmake/modules/CheckCompilerFeatures.cmake
+++ b/libc/cmake/modules/CheckCompilerFeatures.cmake
@@ -102,3 +102,6 @@ check_cxx_compiler_flag("-nostdlib++" LIBC_CC_SUPPORTS_NOSTDLIBPP)
 
 # clang-3.0+
 check_cxx_compiler_flag("-nostdlibinc" LIBC_CC_SUPPORTS_NOSTDLIBINC)
+
+# clang-3.0+
+check_cxx_compiler_flag("-nolibc" LIBC_CC_SUPPORTS_NOLIBC)
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 2cf7206f3a625..d521e205df8c3 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -664,6 +664,9 @@ get_all_install_header_targets(all_install_header_targets ${TARGET_PUBLIC_HEADER
 add_library(libc-headers INTERFACE)
 add_dependencies(libc-headers ${all_install_header_targets})
 target_include_directories(libc-headers SYSTEM INTERFACE ${LIBC_INCLUDE_DIR})
+if(LIBC_CC_SUPPORTS_NOSTDLIBINC)
+  target_compile_options(libc-headers INTERFACE "-nostdlibinc")
+endif()
 
 foreach(target IN LISTS all_install_header_targets)
   get_target_property(header_file ${target} HEADER_FILE_PATH)
diff --git a/libc/lib/CMakeLists.txt b/libc/lib/CMakeLists.txt
index 37acf3950b460..7e18f35e7d60e 100644
--- a/libc/lib/CMakeLists.txt
+++ b/libc/lib/CMakeLists.txt
@@ -30,7 +30,10 @@ foreach(archive IN ZIP_LISTS
       ARCHIVE_OUTPUT_NAME ${archive_0}
   )
   if(LLVM_LIBC_FULL_BUILD)
-    target_link_libraries(${archive_1} PUBLIC libc-headers)
+    target_link_libraries(${archive_1} INTERFACE libc-headers)
+    if(LIBC_CC_SUPPORTS_NOLIBC)
+      target_link_options(${archive_1} INTERFACE "-nolibc")
+    endif()
     if(TARGET libc-startup)
       add_dependencies(${archive_1} libc-startup)
     endif()
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 190a97db9462f..5a568e95b239e 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -293,6 +293,7 @@ option(LIBCXX_ENABLE_THREADS "Build libc++ with support for threads." ON)
 option(LIBCXX_ENABLE_MONOTONIC_CLOCK
   "Build libc++ with support for a monotonic clock.
    This option may only be set to OFF when LIBCXX_ENABLE_THREADS=OFF." ON)
+option(LIBCXX_USE_LLVM_LIBC "Build libc++ against LLVM libc." OFF)
 option(LIBCXX_HAS_MUSL_LIBC "Build libc++ with support for the Musl C library" OFF)
 option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)
 option(LIBCXX_HAS_WIN32_THREAD_API "Ignore auto-detection and force use of win32 thread API" OFF)
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index cd64fe91449c2..31a819932d521 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -1043,6 +1043,9 @@ add_custom_target(generate-cxx-headers ALL DEPENDS ${_all_includes})
 
 add_library(cxx-headers INTERFACE)
 target_link_libraries(cxx-headers INTERFACE libcxx-abi-headers)
+if (LIBCXX_USE_LLVM_LIBC)
+  target_link_libraries(cxx-headers INTERFACE libc-headers)
+endif()
 add_dependencies(cxx-headers generate-cxx-headers)
 # It's important that the arch directory be included first so that its header files
 # which interpose on the default include dir be included instead of the default ones.
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 0ae58a10c879c..536a5a16ad7e1 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -229,6 +229,11 @@ if (LIBCXX_ENABLE_SHARED)
     target_link_libraries(cxx_shared PUBLIC libcxx-abi-shared)
   endif()
 
+  # Link against LLVM libc.
+  if (LIBCXX_USE_LLVM_LIBC)
+    target_link_libraries(cxx_shared PUBLIC libc libm)
+  endif()
+
   # Maybe re-export symbols from libc++abi
   # In particular, we don't re-export the symbols if libc++abi is merged statically
   # into libc++ because in that case there's no dylib to re-export from.
@@ -324,6 +329,11 @@ if (LIBCXX_ENABLE_STATIC)
   if (LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY)
     target_link_libraries(cxx_static PRIVATE libcxx-abi-static-objects)
   endif()
+
+  # Link against LLVM libc.
+  if (LIBCXX_USE_LLVM_LIBC)
+    target_link_libraries(cxx_static PUBLIC libc libm)
+  endif()
 endif()
 
 # Add a meta-target for both libraries.

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just about to do something similar, so I'm glad you got to it before me.

I think we'll need a CMake check on whether or not libc was enabled as a runtime if this option is enabled in libcxx, but beyond that it looks very reasonable. All we're doing is suppressing the system includes with -nostdlibinc and then adding an interface include to the libc targets.

@jhuber6 jhuber6 requested review from mordante and philnik777 July 17, 2024 11:48
Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this locally and it worked quite well thanks.

My nit is that we should add a check that mirrors the already existing LIBCXXABI_USE_LLVM_UNWINDER flag, which prints an error if libunwind isn't in the runtimes list. After that LG from the libc side, but I'd wait for a word from the libcxx side.

@@ -293,6 +293,7 @@ option(LIBCXX_ENABLE_THREADS "Build libc++ with support for threads." ON)
option(LIBCXX_ENABLE_MONOTONIC_CLOCK
"Build libc++ with support for a monotonic clock.
This option may only be set to OFF when LIBCXX_ENABLE_THREADS=OFF." ON)
option(LIBCXX_USE_LLVM_LIBC "Build libc++ against LLVM libc." OFF)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to avoid introducing a CMake setting like this. Instead, we should do something similar to the way we handle ABI choices and provide a pseudo-target that represents the libc we're building against. We can then have a multi-valued option like LIBCXX_LIBC="llvm-libc|system-libc|musl|etc...".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my original thought, but I don't think the different C libraries have enough granularity for this to be anything but a binary flag in practice. What this flag truly means is whether or not to use the LLVM libc built in-tree or not. It's basically the exact same use-case as the already present LIBCXXABI_USE_LLVM_UNWINDER. If someone installed the LLVM libc globally, then the existing logic would just pick it up as the libc without this flag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benefit of that approach is that we can define how to build against arbitrary new C stdlibs very easily, and we avoid adding more logic than there already is in the CMakeLists.txt. I don't mind if we start with only two configurations for this flag, what matters to me is that we use the right approach to introduce this new functionality.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I don't think that config easily exposes what we're trying to express, since you could say llvm-libc and it wouldn't disambiguate between llvm-libc installed on the system globally and llvm-libc that exists as a target inside the current CMake project. So, we'd still need a flag in that case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a solution to the same problem for selecting the ABI library: set(LIBCXX_SUPPORTED_ABI_LIBRARIES none libcxxabi system-libcxxabi libcxxrt libstdc++ libsupc++ vcruntime).

There's libcxxabi (in-tree) and system-libcxxabi (whatever libc++abi is installed on the system).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, then we should have system system-llvm-libc llvm-libc, where system is the current default (Just use whatever you find in the default include path).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how system-llvm-libc would differ from system, but yeah that sounds about right.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary for now, was just thinking how if we started adding new ones like musl or glibc then we'd need to disambiguate. Right now we probably just want system as the default and llvm-libc which mirrors this patch's binary selection well enough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced LIBCXX_LIBC which currently accepts system or llvm-libc. system is currently no-op but that can be expanded later. In the future we may also want to introduce additional options e.g. musl replacing the existing LIBCXX_HAS_MUSL_LIBC option but I'd prefer to do it in a separate change.

I'm not sure if it's better to set the -nostdlibinc and -nolibc on the libc side (as interface flags on the targets) or on the libc++ side (as interface flags on the proxy targets), I can see an argument for doing either way. Right now I set those on the libc side which has the upside of not needing to duplicate those flags in libunwind and libc++abi which should eventually support LLVM libc as well.

In the future we'll probably want to move HandleLibC.cmake to the top-level cmake/Modules directory so they can be shared by libc++abi and libunwind, although we'll need to rethink the target and variables names because the LIBCXX_ and libcxx- prefix is undesirable (maybe we can use RUNTIMES_ and runtimes-)?

jhuber6 added a commit to jhuber6/llvm-project that referenced this pull request Jul 17, 2024
Summary:
This patch adds a CMake cache config file for the GPU build. This cache
will set the default required options when used from the LLVM runtime
interface or directly. These options pretty much disable everything the
GPU can't handle.

With this and the fllowing patches: llvm#99259, llvm#99243, llvm#99287, and llvm#99333,
we will be able to build `libc++` targeting the GPU with an invocation
like this.

```
$ cmake ../llvm
-C${LLVM_ROOT}/libcxx/cmake/caches/GPU.cmake                                \
-DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx \
-DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx   \
-DLLVM_RUNTIME_TARGETS=amdgcn-amd-amdhsa;nvptx64-nvidia-cuda                \
```

This will then install the libraries and headers into the appropriate
locations for use with `clang`.
Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems fine from the libc side

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this approach much better. In a future refactor, I would move -nostdlibinc and -nostdlib to the interface target, and eventually libc++abi and libunwind should use those targets too when building. But this is a fine first step.

Comment on lines 231 to 233
set(LIBCXX_DEFAULT_C_LIBRARY system)
set(LIBCXX_SUPPORTED_C_LIBRARIES system llvm-libc)
set(LIBCXX_LIBC "${LIBCXX_DEFAULT_C_LIBRARY}" CACHE STRING "Specify C library to use. Supported values are ${LIBCXX_SUPPORTED_C_LIBRARIES}.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
set(LIBCXX_DEFAULT_C_LIBRARY system)
set(LIBCXX_SUPPORTED_C_LIBRARIES system llvm-libc)
set(LIBCXX_LIBC "${LIBCXX_DEFAULT_C_LIBRARY}" CACHE STRING "Specify C library to use. Supported values are ${LIBCXX_SUPPORTED_C_LIBRARIES}.")
set(LIBCXX_SUPPORTED_C_LIBRARIES system llvm-libc)
set(LIBCXX_LIBC "system" CACHE STRING "Specify C library to use. Supported values are ${LIBCXX_SUPPORTED_C_LIBRARIES}.")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

# - libcxx-libc-headers: An interface target that allows getting access to the
# headers of the selected C library.
# - libcxx-libc-shared: A target representing the selected shared C library.
# - libcxx-libm-shared: A target representing the selected shared C math library.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not possible to roll this up into the libc target? From the libc++ side, we'd like to have the illusion that libm is nothing but an implementation detail of libc, kind of like -latomic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines 797 to 799
if (LIBCXX_LIBC STREQUAL "llvm-libc")
config_define(ON _LIBCPP_HAS_LLVM_LIBC)
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not going to block this patch on fixing this, but we don't want to add a new __config_site definition every time we add support for a new library / platform. Instead, we should provide control over the implementation of localization used by the library at the CMake level, or a similar solution to prevent the proliferation of boolean flags.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed these and will address it more correctly in a follow up PR.

@petrhosek
Copy link
Member Author

I like this approach much better. In a future refactor, I would move -nostdlibinc and -nostdlib to the interface target, and eventually libc++abi and libunwind should use those targets too when building. But this is a fine first step.

I believe I address all comments, the latest version is more minimal but I think it's a good start.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this patch. It introduces the hook we need to clean up a lot of stuff in our CMake and it also adds the flexibility you need to unblock LLVM libc work. I think we ended up in a good place.

@petrhosek petrhosek merged commit 3b78dfa into llvm:main Jul 19, 2024
53 of 56 checks passed
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
Provide an option to build libc++ against LLVM libc and set the CMake
compile and link options appropriately when the option is enabled.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251440
jhuber6 added a commit to jhuber6/llvm-project that referenced this pull request Jul 31, 2024
Summary:
This patch adds a CMake cache config file for the GPU build. This cache
will set the default required options when used from the LLVM runtime
interface or directly. These options pretty much disable everything the
GPU can't handle.

With this and the fllowing patches: llvm#99259, llvm#99243, llvm#99287, and llvm#99333,
we will be able to build `libc++` targeting the GPU with an invocation
like this.

```
$ cmake ../llvm
-C${LLVM_ROOT}/libcxx/cmake/caches/GPU.cmake                                \
-DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx \
-DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx   \
-DLLVM_RUNTIME_TARGETS=amdgcn-amd-amdhsa;nvptx64-nvidia-cuda                \
```

This will then install the libraries and headers into the appropriate
locations for use with `clang`.
petrhosek added a commit to petrhosek/llvm-project that referenced this pull request Aug 2, 2024
This is analogous to llvm#99287 and provides an option to build libc++abi
and libunwind against LLVM libc when selected.
jhuber6 added a commit to jhuber6/llvm-project that referenced this pull request Aug 2, 2024
Summary:
This patch adds a CMake cache config file for the GPU build. This cache
will set the default required options when used from the LLVM runtime
interface or directly. These options pretty much disable everything the
GPU can't handle.

With this and the fllowing patches: llvm#99259, llvm#99243, llvm#99287, and llvm#99333,
we will be able to build `libc++` targeting the GPU with an invocation
like this.

```
$ cmake ../llvm
-C${LLVM_ROOT}/libcxx/cmake/caches/GPU.cmake                                \
-DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx \
-DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx   \
-DLLVM_RUNTIME_TARGETS=amdgcn-amd-amdhsa;nvptx64-nvidia-cuda                \
```

This will then install the libraries and headers into the appropriate
locations for use with `clang`.
jhuber6 added a commit to jhuber6/llvm-project that referenced this pull request Aug 14, 2024
Summary:
This patch adds a CMake cache config file for the GPU build. This cache
will set the default required options when used from the LLVM runtime
interface or directly. These options pretty much disable everything the
GPU can't handle.

With this and the fllowing patches: llvm#99259, llvm#99243, llvm#99287, and llvm#99333,
we will be able to build `libc++` targeting the GPU with an invocation
like this.

```
$ cmake ../llvm
-C${LLVM_ROOT}/libcxx/cmake/caches/GPU.cmake                                \
-DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx \
-DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx   \
-DLLVM_RUNTIME_TARGETS=amdgcn-amd-amdhsa;nvptx64-nvidia-cuda                \
```

This will then install the libraries and headers into the appropriate
locations for use with `clang`.
jhuber6 added a commit to jhuber6/llvm-project that referenced this pull request Aug 15, 2024
Summary:
This patch adds a CMake cache config file for the GPU build. This cache
will set the default required options when used from the LLVM runtime
interface or directly. These options pretty much disable everything the
GPU can't handle.

With this and the fllowing patches: llvm#99259, llvm#99243, llvm#99287, and llvm#99333,
we will be able to build `libc++` targeting the GPU with an invocation
like this.

```
$ cmake ../llvm
-C${LLVM_ROOT}/libcxx/cmake/caches/GPU.cmake                                \
-DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx \
-DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx   \
-DLLVM_RUNTIME_TARGETS=amdgcn-amd-amdhsa;nvptx64-nvidia-cuda                \
```

This will then install the libraries and headers into the appropriate
locations for use with `clang`.

Move to separate files
jhuber6 added a commit to jhuber6/llvm-project that referenced this pull request Aug 21, 2024
Summary:
This patch adds a CMake cache config file for the GPU build. This cache
will set the default required options when used from the LLVM runtime
interface or directly. These options pretty much disable everything the
GPU can't handle.

With this and the fllowing patches: llvm#99259, llvm#99243, llvm#99287, and llvm#99333,
we will be able to build `libc++` targeting the GPU with an invocation
like this.

```
$ cmake ../llvm
-C${LLVM_ROOT}/libcxx/cmake/caches/GPU.cmake                                \
-DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx \
-DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx   \
-DLLVM_RUNTIME_TARGETS=amdgcn-amd-amdhsa;nvptx64-nvidia-cuda                \
```

This will then install the libraries and headers into the appropriate
locations for use with `clang`.
jhuber6 added a commit that referenced this pull request Aug 21, 2024
Summary:
This patch adds a CMake cache config file for the GPU build. This cache
will set the default required options when used from the LLVM runtime
interface or directly. These options pretty much disable everything the
GPU can't handle.

With this and the following patches: #99259, #99243, #99287, and #99333,
we will be able to build `libc++` targeting the GPU with an invocation
like this.

```
$ cmake ../llvm
-DRUNTIMES_nvptx64-nvidia-cuda_CACHE_FILES=${LLVM_SRC}/../libcxx/cmake/caches/NVPTX.cmake \
-DRUNTIMES_amdgcn-amd-amdhsa_CACHE_FILES=${LLVM_SRC}/../libcxx/cmake/caches/AMDGPU.cmake \                            
-DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx \
-DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx   \
-DLLVM_RUNTIME_TARGETS=amdgcn-amd-amdhsa;nvptx64-nvidia-cuda                \
```

This will then install the libraries and headers into the appropriate
locations for use with `clang`.
cjdb pushed a commit to cjdb/llvm-project that referenced this pull request Aug 23, 2024
Summary:
This patch adds a CMake cache config file for the GPU build. This cache
will set the default required options when used from the LLVM runtime
interface or directly. These options pretty much disable everything the
GPU can't handle.

With this and the following patches: llvm#99259, llvm#99243, llvm#99287, and llvm#99333,
we will be able to build `libc++` targeting the GPU with an invocation
like this.

```
$ cmake ../llvm
-DRUNTIMES_nvptx64-nvidia-cuda_CACHE_FILES=${LLVM_SRC}/../libcxx/cmake/caches/NVPTX.cmake \
-DRUNTIMES_amdgcn-amd-amdhsa_CACHE_FILES=${LLVM_SRC}/../libcxx/cmake/caches/AMDGPU.cmake \                            
-DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx \
-DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=compiler-rt;libc;libcxx   \
-DLLVM_RUNTIME_TARGETS=amdgcn-amd-amdhsa;nvptx64-nvidia-cuda                \
```

This will then install the libraries and headers into the appropriate
locations for use with `clang`.
petrhosek added a commit to petrhosek/llvm-project that referenced this pull request Apr 8, 2025
This generalizes the support added in llvm#99287 renaming the option to
RUNTIMES_USE_LIBC and integrating the module into libc++abi and
libunwind as well.
petrhosek added a commit to petrhosek/llvm-project that referenced this pull request Apr 8, 2025
This generalizes the support added in llvm#99287 renaming the option to
RUNTIMES_USE_LIBC and integrating the module into libc++abi and
libunwind as well.
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 libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants