Skip to content

[runtimes] Improve the documentation for LIBCXX_ADDITIONAL_COMPILE_FLAGS #112733

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 1 commit into from
Oct 17, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Oct 17, 2024

This clarifies how that option is meant to be used to avoid confusion. As a drive-by, also fix an incorrect usage in the recently-added GPU caches.

This clarifies how that option is meant to be used to avoid confusion.
As a drive-by, also fix an incorrect usage in the recently-added GPU
caches.
@ldionne ldionne requested review from a team as code owners October 17, 2024 15:47
@llvmbot llvmbot added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc++abi libc++abi C++ Runtime Library. Not libc++. libunwind backend:AMDGPU labels Oct 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2024

@llvm/pr-subscribers-libunwind
@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-libcxxabi

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

This clarifies how that option is meant to be used to avoid confusion. As a drive-by, also fix an incorrect usage in the recently-added GPU caches.


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

5 Files Affected:

  • (modified) libcxx/CMakeLists.txt (+3-1)
  • (modified) libcxx/cmake/caches/AMDGPU.cmake (+2-2)
  • (modified) libcxx/docs/VendorDocumentation.rst (+4-8)
  • (modified) libcxxabi/CMakeLists.txt (+1-2)
  • (modified) libunwind/CMakeLists.txt (+1-2)
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 75c926f5432aea..723863ddfd54d4 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -459,7 +459,9 @@ set(LIBCXX_COMPILE_FLAGS "")
 set(LIBCXX_LINK_FLAGS "")
 set(LIBCXX_LIBRARIES "")
 set(LIBCXX_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING
-    "Additional Compile only flags which can be provided in cache")
+    "Additional compile flags to use when building libc++. This should be a CMake ;-delimited list of individual
+     compiler options to use. For options that must be passed as-is to the compiler without deduplication (e.g.
+     `-Xclang -foo` option groups), consider using `SHELL:` (https://cmake.org/cmake/help/latest/command/add_compile_options.html#option-de-duplication).")
 set(LIBCXX_ADDITIONAL_LIBRARIES "" CACHE STRING
     "Additional libraries libc++ is linked to which can be provided in cache")
 
diff --git a/libcxx/cmake/caches/AMDGPU.cmake b/libcxx/cmake/caches/AMDGPU.cmake
index c7d6afc854a54c..1a6bfd85a50be3 100644
--- a/libcxx/cmake/caches/AMDGPU.cmake
+++ b/libcxx/cmake/caches/AMDGPU.cmake
@@ -29,7 +29,7 @@ set(LIBCXXABI_USE_LLVM_UNWINDER OFF CACHE BOOL "")
 
 # Necessary compile flags for AMDGPU.
 set(LIBCXX_ADDITIONAL_COMPILE_FLAGS
-    "-nogpulib;-flto;-fconvergent-functions;-Xclang;-mcode-object-version=none" CACHE STRING "")
+    "-nogpulib;-flto;-fconvergent-functions;SHELL:-Xclang -mcode-object-version=none" CACHE STRING "")
 set(LIBCXXABI_ADDITIONAL_COMPILE_FLAGS
-    "-nogpulib;-flto;-fconvergent-functions;-Xclang;-mcode-object-version=none" CACHE STRING "")
+    "-nogpulib;-flto;-fconvergent-functions;SHELL:-Xclang -mcode-object-version=none" CACHE STRING "")
 set(CMAKE_REQUIRED_FLAGS "-nogpulib" CACHE STRING "")
diff --git a/libcxx/docs/VendorDocumentation.rst b/libcxx/docs/VendorDocumentation.rst
index 3a3d1cdb1ea7ff..3795381264c93b 100644
--- a/libcxx/docs/VendorDocumentation.rst
+++ b/libcxx/docs/VendorDocumentation.rst
@@ -213,11 +213,13 @@ General purpose options
 
   Output name for the shared libc++ runtime library.
 
-.. option:: LIBCXX_ADDITIONAL_COMPILE_FLAGS:STRING
+.. option:: {LIBCXX,LIBCXXABI,LIBUNWIND}_ADDITIONAL_COMPILE_FLAGS:STRING
 
   **Default**: ``""``
 
-  Additional Compile only flags which can be provided in cache.
+  Additional compile flags to use when building the runtimes. This should be a CMake ``;``-delimited list of individual
+  compiler options to use. For options that must be passed as-is to the compiler without deduplication (e.g.
+  ``-Xclang -foo`` option groups), consider using ``SHELL:`` as `documented here <https://cmake.org/cmake/help/latest/command/add_compile_options.html#option-de-duplication>`_.
 
 .. option:: LIBCXX_ADDITIONAL_LIBRARIES:STRING
 
@@ -346,12 +348,6 @@ The following options allow building libc++ for a different ABI version.
   Build and use the LLVM unwinder. Note: This option can only be used when
   libc++abi is the C++ ABI library used.
 
-.. option:: LIBCXXABI_ADDITIONAL_COMPILE_FLAGS:STRING
-
-  **Default**: ``""``
-
-  Additional Compile only flags which can be provided in cache.
-
 .. option:: LIBCXXABI_ADDITIONAL_LIBRARIES:STRING
 
   **Default**: ``""``
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index ac1ee69d5f11c9..da0e8b286cddc1 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -222,8 +222,7 @@ set(LIBCXXABI_CXX_FLAGS "")
 set(LIBCXXABI_COMPILE_FLAGS "")
 set(LIBCXXABI_LINK_FLAGS "")
 set(LIBCXXABI_LIBRARIES "")
-set(LIBCXXABI_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING
-    "Additional Compile only flags which can be provided in cache")
+set(LIBCXXABI_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING "See documentation LIBCXX_ADDITIONAL_COMPILE_FLAGS")
 set(LIBCXXABI_ADDITIONAL_LIBRARIES "" CACHE STRING
     "Additional libraries libc++abi is linked to which can be provided in cache")
 
diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index b911f482fc26b2..ea06dc8a67b949 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -162,8 +162,7 @@ set(LIBUNWIND_C_FLAGS "")
 set(LIBUNWIND_CXX_FLAGS "")
 set(LIBUNWIND_COMPILE_FLAGS "")
 set(LIBUNWIND_LINK_FLAGS "")
-set(LIBUNWIND_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING
-    "Additional Compile only flags which can be provided in cache")
+set(LIBUNWIND_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING "See documentation for LIBCXX_ADDITIONAL_COMPILE_FLAGS")
 set(LIBUNWIND_ADDITIONAL_LIBRARIES "" CACHE STRING
     "Additional libraries libunwind is linked to which can be provided in cache")
 

@ldionne ldionne merged commit e674424 into llvm:main Oct 17, 2024
73 checks passed
@ldionne ldionne deleted the review/document-additional_compile_flags branch October 17, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU libc++abi libc++abi C++ Runtime Library. Not libc++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libunwind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants