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

[libc][cmake] append per obj compile options instead of prepending #77126

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

nickdesaulniers
Copy link
Member

This allows individual object files to override the common compile commands in
their local CMakeLists' add_object_library call.

For example, the common compile commands contain -Wall and -Wextra. Before
this patch, the per object COMPILE_OPTIONS were prepended to these, so that
builds of individual object files could not individually disable specific
diagnostics from those groups explicitly.

After this patch, the per-object file compile objects are appended to the list
of compiler flags, enabling this use case.

ARGN is a bit of cmake magic; let's be explicit in the APPEND that we're
appending the compile options.

Link: #77007

This allows individual object files to override the common compile commands in
their local CMakeLists' add_object_library call.

For example, the common compile commands contain -Wall and -Wextra.  Before
this patch, the per object COMPILE_OPTIONS were prepended to these, so that
builds of individual object files could not individually disable specific
diagnostics from those groups explicitly.

After this patch, the per-object file compile objects are appended to the list
of compiler flags, enabling this use case.

ARGN is a bit of cmake magic; let's be explicit in the APPEND that we're
appending the compile options.

Link: llvm#77007
@llvmbot llvmbot added the libc label Jan 5, 2024
@llvmbot
Copy link

llvmbot commented Jan 5, 2024

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

This allows individual object files to override the common compile commands in
their local CMakeLists' add_object_library call.

For example, the common compile commands contain -Wall and -Wextra. Before
this patch, the per object COMPILE_OPTIONS were prepended to these, so that
builds of individual object files could not individually disable specific
diagnostics from those groups explicitly.

After this patch, the per-object file compile objects are appended to the list
of compiler flags, enabling this use case.

ARGN is a bit of cmake magic; let's be explicit in the APPEND that we're
appending the compile options.

Link: #77007


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

1 Files Affected:

  • (modified) libc/cmake/modules/LLVMLibCObjectRules.cmake (+5-11)
diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index 5fbbfd58db2d07..c3e3fa2dccfe53 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -26,7 +26,7 @@ function(_get_common_compile_options output_var flags)
     set(ADD_PREFER_GENERIC_FLAG TRUE)
   endif()
 
-  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${ARGN})
+  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT})
   if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
     list(APPEND compile_options "-fpie")
 
@@ -357,11 +357,8 @@ function(create_object_library fq_target_name)
     set(internal_target_name ${fq_target_name})
   endif()
 
-  _get_common_compile_options(
-    compile_options
-    "${ADD_OBJECT_FLAGS}"
-    ${ADD_OBJECT_COMPILE_OPTIONS}
-  )
+  _get_common_compile_options(compile_options "${ADD_OBJECT_FLAGS}")
+  list(APPEND compile_options ${ADD_OBJECT_COMPILE_OPTIONS})
 
   # GPU builds require special handling for the objects because we want to
   # export several different targets at once, e.g. for both Nvidia and AMD.
@@ -640,11 +637,8 @@ function(create_entrypoint_object fq_target_name)
     set(ADD_ENTRYPOINT_OBJ_CXX_STANDARD ${CMAKE_CXX_STANDARD})
   endif()
 
-  _get_common_compile_options(
-    common_compile_options
-    "${ADD_ENTRYPOINT_OBJ_FLAGS}"
-    ${ADD_ENTRYPOINT_OBJ_COMPILE_OPTIONS}
-  )
+  _get_common_compile_options(common_compile_options "${ADD_ENTRYPOINT_OBJ_FLAGS}")
+  list(APPEND common_compile_options ${ADD_ENTRYPOINT_OBJ_COMPILE_OPTIONS})
   get_fq_deps_list(fq_deps_list ${ADD_ENTRYPOINT_OBJ_DEPENDS})
   set(full_deps_list ${fq_deps_list} libc.src.__support.common)
 

@nickdesaulniers nickdesaulniers merged commit dcdb4a3 into llvm:main Jan 5, 2024
5 checks passed
@nickdesaulniers nickdesaulniers deleted the cmake branch January 5, 2024 21:43
@nickdesaulniers
Copy link
Member Author

Looks like the riscv64 fullbuild might be affected by this change? Digging.

https://lab.llvm.org/buildbot/#/builders/257/builds/7446

justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…lvm#77126)

This allows individual object files to override the common compile
commands in
their local CMakeLists' add_object_library call.

For example, the common compile commands contain -Wall and -Wextra.
Before
this patch, the per object COMPILE_OPTIONS were prepended to these, so
that
builds of individual object files could not individually disable
specific
diagnostics from those groups explicitly.

After this patch, the per-object file compile objects are appended to
the list
of compiler flags, enabling this use case.

ARGN is a bit of cmake magic; let's be explicit in the APPEND that we're
appending the compile options.

Link: llvm#77007
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants