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.

Sign up for GitHub

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

Jump to bottom

[libc] Move compile options to new cmake file #81917

Merged
merged 2 commits into from
Feb 16, 2024
Merged

[libc] Move compile options to new cmake file #81917

merged 2 commits into from
Feb 16, 2024

Conversation

michaelrj-google
Copy link
Contributor

The cmake test generator needed to be updated to support the same flags
as the source. To simplify the code, I moved it to a new file.

@michaelrj-google
[libc] Move compile options to new cmake file
f3aa2c0 
The cmake test generator needed to be updated to support the same flags
as the source. To simplify the code, I moved it to a new file.
@michaelrj-google michaelrj-google marked this pull request as ready for review February 16, 2024 00:06
@llvmbot llvmbot added the libc label Feb 16, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

The cmake test generator needed to be updated to support the same flags
as the source. To simplify the code, I moved it to a new file.

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

5 Files Affected:

  • (added) libc/cmake/modules/LLVMLibCCompileOptionRules.cmake (+220)
  • (modified) libc/cmake/modules/LLVMLibCObjectRules.cmake (-149)
  • (modified) libc/cmake/modules/LLVMLibCRules.cmake (+1)
  • (modified) libc/cmake/modules/LLVMLibCTestRules.cmake (+5-30)
  • (modified) libc/test/UnitTest/CMakeLists.txt (+3-2)
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
new file mode 100644
index 00000000000000..f1f919f8c77eee
--- /dev/null
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -0,0 +1,220 @@
+function(_get_compile_options_from_flags output_var)
+  set(compile_options "")
+
+  if(LIBC_TARGET_ARCHITECTURE_IS_RISCV64 OR(LIBC_CPU_FEATURES MATCHES "FMA"))
+    check_flag(ADD_FMA_FLAG ${FMA_OPT_FLAG} ${flags})
+  endif()
+  check_flag(ADD_SSE4_2_FLAG ${ROUND_OPT_FLAG} ${flags})
+  check_flag(ADD_EXPLICIT_SIMD_OPT_FLAG ${EXPLICIT_SIMD_OPT_FLAG} ${flags})
+  
+  if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
+    if(ADD_FMA_FLAG)
+      if(LIBC_TARGET_ARCHITECTURE_IS_X86)
+        list(APPEND compile_options "-mavx2")
+        list(APPEND compile_options "-mfma")
+      elseif(LIBC_TARGET_ARCHITECTURE_IS_RISCV64)
+        list(APPEND compile_options "-D__LIBC_RISCV_USE_FMA")
+      endif()
+    endif()
+    if(ADD_SSE4_2_FLAG)
+      list(APPEND compile_options "-msse4.2")
+    endif()
+    if(ADD_EXPLICIT_SIMD_OPT_FLAG)
+      list(APPEND compile_options "-D__LIBC_EXPLICIT_SIMD_OPT")
+    endif()
+  elseif(MSVC)
+    if(ADD_FMA_FLAG)
+      list(APPEND compile_options "/arch:AVX2")
+    endif()
+    if(ADD_EXPLICIT_SIMD_OPT_FLAG)
+      list(APPEND compile_options "/D__LIBC_EXPLICIT_SIMD_OPT")
+    endif()
+  endif()
+
+  set(${output_var} ${compile_options} PARENT_SCOPE)
+endfunction(_get_compile_options_from_flags)
+
+function(_get_common_compile_options output_var flags)
+  _get_compile_options_from_flags(compile_flags ${flags})
+
+  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${compile_flags})
+
+  if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
+    list(APPEND compile_options "-fpie")
+
+    if(LLVM_LIBC_FULL_BUILD)
+      # Only add -ffreestanding flag in full build mode.
+      list(APPEND compile_options "-ffreestanding")
+    endif()
+
+    if(LIBC_COMPILER_HAS_FIXED_POINT)
+      list(APPEND compile_options "-ffixed-point")
+    endif()
+
+    list(APPEND compile_options "-fno-builtin")
+    list(APPEND compile_options "-fno-exceptions")
+    list(APPEND compile_options "-fno-lax-vector-conversions")
+    list(APPEND compile_options "-fno-unwind-tables")
+    list(APPEND compile_options "-fno-asynchronous-unwind-tables")
+    list(APPEND compile_options "-fno-rtti")
+    if (LIBC_CC_SUPPORTS_PATTERN_INIT)
+      list(APPEND compile_options "-ftrivial-auto-var-init=pattern")
+    endif()
+    list(APPEND compile_options "-Wall")
+    list(APPEND compile_options "-Wextra")
+    # -DLIBC_WNO_ERROR=ON if you can't build cleanly with -Werror.
+    if(NOT LIBC_WNO_ERROR)
+      list(APPEND compile_options "-Werror")
+    endif()
+    list(APPEND compile_options "-Wconversion")
+    list(APPEND compile_options "-Wno-sign-conversion")
+    list(APPEND compile_options "-Wimplicit-fallthrough")
+    list(APPEND compile_options "-Wwrite-strings")
+    list(APPEND compile_options "-Wextra-semi")
+    if(NOT CMAKE_COMPILER_IS_GNUCXX)
+      list(APPEND compile_options "-Wnewline-eof")
+      list(APPEND compile_options "-Wnonportable-system-include-path")
+      list(APPEND compile_options "-Wstrict-prototypes")
+      list(APPEND compile_options "-Wthread-safety")
+      list(APPEND compile_options "-Wglobal-constructors")
+    endif()
+  elseif(MSVC)
+    list(APPEND compile_options "/EHs-c-")
+    list(APPEND compile_options "/GR-")
+  endif()
+  if (LIBC_TARGET_ARCHITECTURE_IS_GPU)
+    list(APPEND compile_options "-nogpulib")
+    list(APPEND compile_options "-fvisibility=hidden")
+    list(APPEND compile_options "-fconvergent-functions")
+
+    # Manually disable all standard include paths and include the resource
+    # directory to prevent system headers from being included.
+    list(APPEND compile_options "-isystem${COMPILER_RESOURCE_DIR}/include")
+    list(APPEND compile_options "-nostdinc")
+  endif()
+  set(${output_var} ${compile_options} PARENT_SCOPE)
+endfunction()
+
+function(_get_common_test_compile_options output_var flags)
+  _get_compile_options_from_flags(compile_flags ${flags})
+
+  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${compile_flags})
+
+  if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
+    list(APPEND compile_options "-fpie")
+
+    if(LLVM_LIBC_FULL_BUILD)
+      # Only add -ffreestanding flag in full build mode.
+      list(APPEND compile_options "-ffreestanding")
+      list(APPEND compile_options "-fno-exceptions")
+      list(APPEND compile_options "-fno-unwind-tables")
+      list(APPEND compile_options "-fno-asynchronous-unwind-tables")
+      list(APPEND compile_options "-fno-rtti")
+    endif()
+
+    if(LIBC_COMPILER_HAS_FIXED_POINT)
+      list(APPEND compile_options "-ffixed-point")
+    endif()
+
+    # list(APPEND compile_options "-Wall")
+    # list(APPEND compile_options "-Wextra")
+    # -DLIBC_WNO_ERROR=ON if you can't build cleanly with -Werror.
+    if(NOT LIBC_WNO_ERROR)
+      # list(APPEND compile_options "-Werror")
+    endif()
+    # list(APPEND compile_options "-Wconversion")
+    # list(APPEND compile_options "-Wno-sign-conversion")
+    # list(APPEND compile_options "-Wimplicit-fallthrough")
+    # list(APPEND compile_options "-Wwrite-strings")
+    # list(APPEND compile_options "-Wextra-semi")
+    # if(NOT CMAKE_COMPILER_IS_GNUCXX)
+    #   list(APPEND compile_options "-Wnewline-eof")
+    #   list(APPEND compile_options "-Wnonportable-system-include-path")
+    #   list(APPEND compile_options "-Wstrict-prototypes")
+    #   list(APPEND compile_options "-Wthread-safety")
+    #   list(APPEND compile_options "-Wglobal-constructors")
+    # endif()
+  endif()
+  if (LIBC_TARGET_ARCHITECTURE_IS_GPU)
+    # TODO: Set these flags
+    # list(APPEND compile_options "-nogpulib")
+    # list(APPEND compile_options "-fvisibility=hidden")
+    # list(APPEND compile_options "-fconvergent-functions")
+
+    # # Manually disable all standard include paths and include the resource
+    # # directory to prevent system headers from being included.
+    # list(APPEND compile_options "-isystem${COMPILER_RESOURCE_DIR}/include")
+    # list(APPEND compile_options "-nostdinc")
+  endif()
+  set(${output_var} ${compile_options} PARENT_SCOPE)
+endfunction()
+
+
+# Obtains NVPTX specific arguments for compilation.
+# The PTX feature is primarily based on the CUDA toolchain version. We want to
+# be able to target NVPTX without an existing CUDA installation, so we need to
+# set this manually. This simply sets the PTX feature to the minimum required
+# for the features we wish to use on that target. The minimum PTX features used
+# here roughly corresponds to the CUDA 9.0 release.
+# Adjust as needed for desired PTX features.
+function(get_nvptx_compile_options output_var gpu_arch)
+  set(nvptx_options "")
+  list(APPEND nvptx_options "-march=${gpu_arch}")
+  list(APPEND nvptx_options "-Wno-unknown-cuda-version")
+  list(APPEND nvptx_options "SHELL:-mllvm -nvptx-emit-init-fini-kernel=false")
+  if(${gpu_arch} STREQUAL "sm_35")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_37")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_50")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_52")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_53")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_60")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_61")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_62")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_70")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_72")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_75")
+    list(APPEND nvptx_options "--cuda-feature=+ptx63")
+  elseif(${gpu_arch} STREQUAL "sm_80")
+    list(APPEND nvptx_options "--cuda-feature=+ptx72")
+  elseif(${gpu_arch} STREQUAL "sm_86")
+    list(APPEND nvptx_options "--cuda-feature=+ptx72")
+  elseif(${gpu_arch} STREQUAL "sm_89")
+    list(APPEND nvptx_options "--cuda-feature=+ptx72")
+  elseif(${gpu_arch} STREQUAL "sm_90")
+    list(APPEND nvptx_options "--cuda-feature=+ptx72")
+  else()
+    message(FATAL_ERROR "Unknown Nvidia GPU architecture '${gpu_arch}'")
+  endif()
+
+  if(LIBC_CUDA_ROOT)
+    list(APPEND nvptx_options "--cuda-path=${LIBC_CUDA_ROOT}")
+  endif()
+  set(${output_var} ${nvptx_options} PARENT_SCOPE)
+endfunction()
+
+#TODO: Fold this into a function to get test framework compile options (which
+# need to be separate from the main test compile options because otherwise they
+# error)
+set(LIBC_HERMETIC_TEST_COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_DEFAULT}
+    -fpie -ffreestanding -fno-exceptions -fno-rtti)
+# The GPU build requires overriding the default CMake triple and architecture.
+if(LIBC_GPU_TARGET_ARCHITECTURE_IS_AMDGPU)
+  list(APPEND LIBC_HERMETIC_TEST_COMPILE_OPTIONS
+       -nogpulib -mcpu=${LIBC_GPU_TARGET_ARCHITECTURE} -flto
+       --target=${LIBC_GPU_TARGET_TRIPLE}
+       -mcode-object-version=${LIBC_GPU_CODE_OBJECT_VERSION})
+elseif(LIBC_GPU_TARGET_ARCHITECTURE_IS_NVPTX)
+  get_nvptx_compile_options(nvptx_options ${LIBC_GPU_TARGET_ARCHITECTURE})
+  list(APPEND LIBC_HERMETIC_TEST_COMPILE_OPTIONS
+       -nogpulib ${nvptx_options} -fno-use-cxa-atexit --target=${LIBC_GPU_TARGET_TRIPLE})
+endif()
diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index 54c7e1eaed0bfd..308ba7d0d5dd74 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -1,154 +1,5 @@
 set(OBJECT_LIBRARY_TARGET_TYPE "OBJECT_LIBRARY")
 
-function(_get_compile_options_from_flags output_var)
-  set(compile_options "")
-
-  if(LIBC_TARGET_ARCHITECTURE_IS_RISCV64 OR(LIBC_CPU_FEATURES MATCHES "FMA"))
-    check_flag(ADD_FMA_FLAG ${FMA_OPT_FLAG} ${flags})
-  endif()
-  check_flag(ADD_SSE4_2_FLAG ${ROUND_OPT_FLAG} ${flags})
-  check_flag(ADD_EXPLICIT_SIMD_OPT_FLAG ${EXPLICIT_SIMD_OPT_FLAG} ${flags})
-  
-  if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
-    if(ADD_FMA_FLAG)
-      if(LIBC_TARGET_ARCHITECTURE_IS_X86)
-        list(APPEND compile_options "-mavx2")
-        list(APPEND compile_options "-mfma")
-      elseif(LIBC_TARGET_ARCHITECTURE_IS_RISCV64)
-        list(APPEND compile_options "-D__LIBC_RISCV_USE_FMA")
-      endif()
-    endif()
-    if(ADD_SSE4_2_FLAG)
-      list(APPEND compile_options "-msse4.2")
-    endif()
-    if(ADD_EXPLICIT_SIMD_OPT_FLAG)
-      list(APPEND compile_options "-D__LIBC_EXPLICIT_SIMD_OPT")
-    endif()
-  elseif(MSVC)
-    if(ADD_FMA_FLAG)
-      list(APPEND compile_options "/arch:AVX2")
-    endif()
-    if(ADD_EXPLICIT_SIMD_OPT_FLAG)
-      list(APPEND compile_options "/D__LIBC_EXPLICIT_SIMD_OPT")
-    endif()
-  endif()
-
-  set(${output_var} ${compile_options} PARENT_SCOPE)
-endfunction(_get_compile_options_from_flags)
-
-function(_get_common_compile_options output_var flags)
-  _get_compile_options_from_flags(compile_flags ${flags})
-
-  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${compile_flags})
-
-  if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
-    list(APPEND compile_options "-fpie")
-
-    if(LLVM_LIBC_FULL_BUILD)
-      # Only add -ffreestanding flag in full build mode.
-      list(APPEND compile_options "-ffreestanding")
-    endif()
-
-    if(LIBC_COMPILER_HAS_FIXED_POINT)
-      list(APPEND compile_options "-ffixed-point")
-    endif()
-
-    list(APPEND compile_options "-fno-builtin")
-    list(APPEND compile_options "-fno-exceptions")
-    list(APPEND compile_options "-fno-lax-vector-conversions")
-    list(APPEND compile_options "-fno-unwind-tables")
-    list(APPEND compile_options "-fno-asynchronous-unwind-tables")
-    list(APPEND compile_options "-fno-rtti")
-    if (LIBC_CC_SUPPORTS_PATTERN_INIT)
-      list(APPEND compile_options "-ftrivial-auto-var-init=pattern")
-    endif()
-    list(APPEND compile_options "-Wall")
-    list(APPEND compile_options "-Wextra")
-    # -DLIBC_WNO_ERROR=ON if you can't build cleanly with -Werror.
-    if(NOT LIBC_WNO_ERROR)
-      list(APPEND compile_options "-Werror")
-    endif()
-    list(APPEND compile_options "-Wconversion")
-    list(APPEND compile_options "-Wno-sign-conversion")
-    list(APPEND compile_options "-Wimplicit-fallthrough")
-    list(APPEND compile_options "-Wwrite-strings")
-    list(APPEND compile_options "-Wextra-semi")
-    if(NOT CMAKE_COMPILER_IS_GNUCXX)
-      list(APPEND compile_options "-Wnewline-eof")
-      list(APPEND compile_options "-Wnonportable-system-include-path")
-      list(APPEND compile_options "-Wstrict-prototypes")
-      list(APPEND compile_options "-Wthread-safety")
-      list(APPEND compile_options "-Wglobal-constructors")
-    endif()
-  elseif(MSVC)
-    list(APPEND compile_options "/EHs-c-")
-    list(APPEND compile_options "/GR-")
-  endif()
-  if (LIBC_TARGET_ARCHITECTURE_IS_GPU)
-    list(APPEND compile_options "-nogpulib")
-    list(APPEND compile_options "-fvisibility=hidden")
-    list(APPEND compile_options "-fconvergent-functions")
-
-    # Manually disable all standard include paths and include the resource
-    # directory to prevent system headers from being included.
-    list(APPEND compile_options "-isystem${COMPILER_RESOURCE_DIR}/include")
-    list(APPEND compile_options "-nostdinc")
-  endif()
-  set(${output_var} ${compile_options} PARENT_SCOPE)
-endfunction()
-
-# Obtains NVPTX specific arguments for compilation.
-# The PTX feature is primarily based on the CUDA toolchain version. We want to
-# be able to target NVPTX without an existing CUDA installation, so we need to
-# set this manually. This simply sets the PTX feature to the minimum required
-# for the features we wish to use on that target. The minimum PTX features used
-# here roughly corresponds to the CUDA 9.0 release.
-# Adjust as needed for desired PTX features.
-function(get_nvptx_compile_options output_var gpu_arch)
-  set(nvptx_options "")
-  list(APPEND nvptx_options "-march=${gpu_arch}")
-  list(APPEND nvptx_options "-Wno-unknown-cuda-version")
-  list(APPEND nvptx_options "SHELL:-mllvm -nvptx-emit-init-fini-kernel=false")
-  if(${gpu_arch} STREQUAL "sm_35")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_37")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_50")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_52")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_53")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_60")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_61")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_62")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_70")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_72")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_75")
-    list(APPEND nvptx_options "--cuda-feature=+ptx63")
-  elseif(${gpu_arch} STREQUAL "sm_80")
-    list(APPEND nvptx_options "--cuda-feature=+ptx72")
-  elseif(${gpu_arch} STREQUAL "sm_86")
-    list(APPEND nvptx_options "--cuda-feature=+ptx72")
-  elseif(${gpu_arch} STREQUAL "sm_89")
-    list(APPEND nvptx_options "--cuda-feature=+ptx72")
-  elseif(${gpu_arch} STREQUAL "sm_90")
-    list(APPEND nvptx_options "--cuda-feature=+ptx72")
-  else()
-    message(FATAL_ERROR "Unknown Nvidia GPU architecture '${gpu_arch}'")
-  endif()
-
-  if(LIBC_CUDA_ROOT)
-    list(APPEND nvptx_options "--cuda-path=${LIBC_CUDA_ROOT}")
-  endif()
-  set(${output_var} ${nvptx_options} PARENT_SCOPE)
-endfunction()
-
 # Build the object target for a single GPU arch.
 # Usage:
 #     _build_gpu_object_for_single_arch(
diff --git a/libc/cmake/modules/LLVMLibCRules.cmake b/libc/cmake/modules/LLVMLibCRules.cmake
index 020938d0d64de5..76f3892eeb32e7 100644
--- a/libc/cmake/modules/LLVMLibCRules.cmake
+++ b/libc/cmake/modules/LLVMLibCRules.cmake
@@ -1,3 +1,4 @@
+include(LLVMLibCCompileOptionRules)
 include(LLVMLibCTargetNameUtils)
 include(LLVMLibCFlagRules)
 include(LLVMLibCObjectRules)
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 5b96c5e9f8c801..f5952ab789ed76 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -130,6 +130,9 @@ function(create_libc_unittest fq_target_name)
                            libc.test.UnitTest.ErrnoSetterMatcher)
   list(REMOVE_DUPLICATES fq_deps_list)
 
+  _get_common_test_compile_options(compile_options "${LIBC_UNITTEST_FLAGS}")
+  list(APPEND compile_options ${LIBC_UNITTEST_COMPILE_OPTIONS})
+
   if(SHOW_INTERMEDIATE_OBJECTS)
     message(STATUS "Adding unit test ${fq_target_name}")
     if(${SHOW_INTERMEDIATE_OBJECTS} STREQUAL "DEPS")
@@ -181,22 +184,8 @@ function(create_libc_unittest fq_target_name)
   )
   target_include_directories(${fq_build_target_name} SYSTEM PRIVATE ${LIBC_INCLUDE_DIR})
   target_include_directories(${fq_build_target_name} PRIVATE ${LIBC_SOURCE_DIR})
-  target_compile_options(
-    ${fq_build_target_name}
-    PRIVATE -fpie ${LIBC_COMPILE_OPTIONS_DEFAULT}
-  )
-  if(LLVM_LIBC_FULL_BUILD)
-    target_compile_options(
-      ${fq_build_target_name}
-      PRIVATE -ffreestanding -fno-exceptions -fno-rtti -fno-unwind-tables -fno-asynchronous-unwind-tables
-    )
-  endif()
-  if(LIBC_UNITTEST_COMPILE_OPTIONS)
-    target_compile_options(
-      ${fq_build_target_name}
-      PRIVATE ${LIBC_UNITTEST_COMPILE_OPTIONS}
-    )
-  endif()
+  target_compile_options(${fq_build_target_name} PRIVATE ${compile_options})
+
   if(NOT LIBC_UNITTEST_CXX_STANDARD)
     set(LIBC_UNITTEST_CXX_STANDARD ${CMAKE_CXX_STANDARD})
   endif()
@@ -530,20 +519,6 @@ function(add_integration_test test_name)
   add_dependencies(${INTEGRATION_TEST_SUITE} ${fq_target_name})
 endfunction(add_integration_test)
 
-set(LIBC_HERMETIC_TEST_COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_DEFAULT}
-    -fpie -ffreestanding -fno-exceptions -fno-rtti)
-# The GPU build requires overriding the default CMake triple and architecture.
-if(LIBC_GPU_TARGET_ARCHITECTURE_IS_AMDGPU)
-  list(APPEND LIBC_HERMETIC_TEST_COMPILE_OPTIONS
-       -nogpulib -mcpu=${LIBC_GPU_TARGET_ARCHITECTURE} -flto
-       --target=${LIBC_GPU_TARGET_TRIPLE}
-       -mcode-object-version=${LIBC_GPU_CODE_OBJECT_VERSION})
-elseif(LIBC_GPU_TARGET_ARCHITECTURE_IS_NVPTX)
-  get_nvptx_compile_options(nvptx_options ${LIBC_GPU_TARGET_ARCHITECTURE})
-  list(APPEND LIBC_HERMETIC_TEST_COMPILE_OPTIONS
-       -nogpulib ${nvptx_options} -fno-use-cxa-atexit --target=${LIBC_GPU_TARGET_TRIPLE})
-endif()
-
 # Rule to add a hermetic test. A hermetic test is one whose executable is fully
 # statically linked and consists of pieces drawn only from LLVM's libc. Nothing,
 # including the startup objects, come from the system libc.
diff --git a/libc/test/UnitTest/CMakeLists.txt b/libc/test/UnitTest/CMakeLists.txt
index e3099e45154765..737a5db2107f18 100644
--- a/libc/test/UnitTest/CMakeLists.txt
+++ b/libc/test/UnitTest/CMakeLists.txt
@@ -27,13 +27,14 @@ function(add_unittest_framework_library name)
       ${TEST_LIB_HDRS}
     )
     target_include_directories(${lib} PUBLIC ${LIBC_SOURCE_DIR})
-    target_compile_options(${lib} PRIVATE -fno-exceptions -fno-rtti)
+    list(APPEND compile_options -fno-exceptions -fno-rtti)
     if(TARGET libc.src.time.clock)
       target_compile_definitions(${lib} PRIVATE TARGET_SUPPORTS_CLOCK)
     endif()
     if(LIBC_COMPILER_HAS_FIXED_POINT)
-      target_compile_options(${lib} PUBLIC -ffixed-point)
+      list(APPEND compile_options -ffixed-point)
     endif()
+    target_compile_options(${lib} PUBLIC ${compile_options})
   endforeach()
   target_include_directories(${name}.hermetic PRIVATE ${LIBC_BUILD_DIR}/include)
   target_compile_options(${name}.hermetic

@michaelrj-google michaelrj-google merged commit d0c2c0a into llvm:main Feb 16, 2024
4 of 5 checks passed
@jhuber6
Copy link
Contributor

jhuber6 commented Feb 16, 2024 •
edited

What is going on with all the commented out flags? This is going to break stuff if those aren't present. I would've preferred to hold off these rewrites until #81921 lands.

I'm really interested in moving that forward because it's going to rot very quickly with all the other rewrites happening. It makes the GPU target operate under the (mostly) same rules so it will make everything else easier once it lands.

@michaelrj-google
Copy link
Contributor Author

The commented out flags won't break anything, those are specifically in the newly added test compile options section. Those flags weren't being added at all before. The tests should still be passing (unless I've really screwed up)

I wanted to land this patch quickly since it should hopefully unblock @lntue 's own cmake rewrite. Hopefully it won't cause too many issues for your patch (which I'm in the middle of reviewing, I will hopefully be done soon), I tried to keep this change as small as reasonable.

@michaelrj-google michaelrj-google deleted the libcTestFlags branch February 16, 2024 00:17
@michaelrj-google michaelrj-google mentioned this pull request Feb 16, 2024
Merged
@nickdesaulniers
Copy link
Member

Consider filing a bug then linking to it in the comments. In the bug you can describe to others what the plan is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lntue lntue lntue approved these changes

Assignees
No one assigned
Labels
libc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@michaelrj-google @llvmbot @jhuber6 @nickdesaulniers @lntue