[libc] Fix unit test compile flags propagation. #106128

lntue · 2024-08-26T20:13:53Z

With this change, I was able to build and test for aarch64 & riscv64 on x86-64 host as follow:

Pre-requisite:

cross build toolchain for aarch64

$ sudo apt install binutils-aarch64-linux-gnu gcc-aarch64-linux-gnu g++-aarch64-linux-gnu

cross build toolchain for riscv64

$ sudo apt install binutils-riscv64-linux-gnu gcc-riscv64-linux-gnu g++-riscv64-linux-gnu

qemu user:

$ sudo apt install qemu qemu-user qemu-user-static

CMake invocation:

$ cmake ../runtimes -GNinja -DLLVM_ENABLE_RUNTIMES=libc -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLIBC_TARGET_TRIPLE=<aarch64-linux-gnu/riscv64-linux-gnu> -DCMAKE_BUILD_TYPE=Release -DLIBC_TEST_COMPILE_OPTIONS_DEFAULT="-static"
$ ninja libc
$ ninja check-libc

jhuber6 · 2024-08-26T20:18:57Z

libc/cmake/modules/LLVMLibCCompileOptionRules.cmake

@@ -233,7 +233,7 @@ endfunction()

 function(_get_hermetic_test_compile_options output_var flags)
  _get_compile_options_from_flags(compile_flags ${flags})
-  list(APPEND compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${compile_flags}
+  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${compile_flags}


Isn't this just overwriting the flags from _get_compile_options_from_flags?

jhuber6 · 2024-08-26T20:20:05Z

libc/cmake/modules/LLVMLibCTestRules.cmake

@@ -138,7 +138,7 @@ function(create_libc_unittest fq_target_name)

  _get_common_test_compile_options(compile_options "${LIBC_UNITTEST_C_TEST}"
                                   "${LIBC_UNITTEST_FLAGS}")
-  list(APPEND compile_options ${LIBC_UNITTEST_COMPILE_OPTIONS})
+  list(APPEND compile_options ${LIBC_UNITTEST_COMPILE_OPTIONS} -static)


If this is needed for your use-case, it might be better to just provide it in a CMake cache file. We should have an option to pass in user-requested flags right?

Added LIBC_TEST_COMPILE_OPTIONS_DEFAULT instead.

libc/test/src/math/smoke/nanf128_test.cpp

github-actions · 2024-08-28T11:00:32Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvmbot · 2024-08-29T16:19:06Z

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

With this change, I was able to build and test for aarch64 & riscv64 on x86-64 host as follow:

Pre-requisite:

cross build toolchain for aarch64

$ sudo apt install binutils-aarch64-linux-gnu gcc-aarch64-linux-gnu g++-aarch64-linux-gnu

cross build toolchain for riscv64

$ sudo apt install binutils-riscv64-linux-gnu gcc-riscv64-linux-gnu g++-riscv64-linux-gnu

qemu user:

$ sudo apt install qemu qemu-user qemu-user-static

CMake invocation:

$ cmake ../runtimes -GNinja -DLLVM_ENABLE_RUNTIMES=libc -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLIBC_TARGET_TRIPLE=&lt;aarch64-linux-gnu/riscv64-linux-gnu&gt; -DCMAKE_BUILD_TYPE=Release
$ ninja libc
$ ninja check-libc

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

8 Files Affected:

(modified) libc/CMakeLists.txt (+26-6)
(modified) libc/cmake/modules/LLVMLibCArchitectures.cmake (+2-1)
(modified) libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake (+4)
(modified) libc/cmake/modules/LLVMLibCCheckMPFR.cmake (+1)
(modified) libc/cmake/modules/LLVMLibCCompileOptionRules.cmake (+10-8)
(modified) libc/cmake/modules/LLVMLibCTestRules.cmake (+9-6)
(modified) libc/test/UnitTest/CMakeLists.txt (+6-7)
(modified) libc/utils/MPFRWrapper/CMakeLists.txt (+2-1)

diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index dd45d6cc8cb6ab..c393d23a3b33d4 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -112,6 +112,7 @@ add_compile_definitions(LIBC_NAMESPACE=${LIBC_NAMESPACE})
 
 # Flags to pass down to the compiler while building the libc functions.
 set(LIBC_COMPILE_OPTIONS_DEFAULT "" CACHE STRING "Architecture to tell clang to optimize for (e.g. -march=... or -mcpu=...)")
+set(LIBC_TEST_COMPILE_OPTIONS_DEFAULT "" CACHE STRING "Common compile options for all the tests.")
 
 list(APPEND LIBC_COMPILE_OPTIONS_DEFAULT ${LIBC_COMMON_TUNE_OPTIONS})
 
@@ -131,12 +132,31 @@ if(COMMAND_RETURN_CODE EQUAL 0)
   message(STATUS "Set COMPILER_RESOURCE_DIR to "
                  "${COMPILER_RESOURCE_DIR} using --print-resource-dir")
 else()
-  if (LIBC_TARGET_OS_IS_GPU)
-    message(FATAL_ERROR "COMPILER_RESOURCE_DIR must be set for GPU builds")
-  else()
-    set(COMPILER_RESOURCE_DIR OFF)
-    message(STATUS "COMPILER_RESOURCE_DIR not set
-                    --print-resource-dir not supported by host compiler")
+  # Try with GCC option: -print-search-dirs, which will output in the form:
+  #   install: <path>
+  #   programs: ........
+  # So we try to capture the <path> after "install: " in the first line of the
+  # output.
+  execute_process(
+    OUTPUT_STRIP_TRAILING_WHITESPACE
+    COMMAND ${CMAKE_CXX_COMPILER} -print-search-dirs
+    RESULT_VARIABLE COMMAND_RETURN_CODE
+    OUTPUT_VARIABLE COMPILER_RESOURCE_DIR
+  )
+  if(COMMAND_RETURN_CODE EQUAL 0)
+    string(REPLACE " " ";" COMPILER_RESOURCE_DIR ${COMPILER_RESOURCE_DIR})
+    string(REPLACE "\n" ";" COMPILER_RESOURCE_DIR "${COMPILER_RESOURCE_DIR}")
+    list(GET COMPILER_RESOURCE_DIR 1 COMPILER_RESOURCE_DIR)
+    message(STATUS "Set COMPILER_RESOURCE_DIR to "
+    "${COMPILER_RESOURCE_DIR} using --print-search-dirs")
+else()
+    if (LIBC_TARGET_OS_IS_GPU)
+      message(FATAL_ERROR "COMPILER_RESOURCE_DIR must be set for GPU builds")
+    else()
+      set(COMPILER_RESOURCE_DIR OFF)
+      message(STATUS "COMPILER_RESOURCE_DIR not set
+                      --print-resource-dir not supported by host compiler")
+    endif()
   endif()
 endif()
 
diff --git a/libc/cmake/modules/LLVMLibCArchitectures.cmake b/libc/cmake/modules/LLVMLibCArchitectures.cmake
index dacb4db75d3374..d922b4f21a8ac6 100644
--- a/libc/cmake/modules/LLVMLibCArchitectures.cmake
+++ b/libc/cmake/modules/LLVMLibCArchitectures.cmake
@@ -207,4 +207,5 @@ if(explicit_target_triple AND
 endif()
 
 message(STATUS
-        "Building libc for ${LIBC_TARGET_ARCHITECTURE} on ${LIBC_TARGET_OS}")
+        "Building libc for ${LIBC_TARGET_ARCHITECTURE} on ${LIBC_TARGET_OS} with
+        LIBC_COMPILE_OPTIONS_DEFAULT: ${LIBC_COMPILE_OPTIONS_DEFAULT}")
diff --git a/libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake b/libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake
index be569f6f9cabfb..3b6b6f56fc80cf 100644
--- a/libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake
+++ b/libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake
@@ -13,6 +13,10 @@ elseif(${LIBC_TARGET_ARCHITECTURE_IS_AARCH64})
   set(LIBC_COMPILE_OPTIONS_NATIVE -mcpu=native)
 endif()
 
+if(LIBC_CROSSBUILD)
+  set(LIBC_COMPILE_OPTIONS_NATIVE ${LIBC_COMPILE_OPTIONS_DEFAULT})
+endif()
+
 # Making sure ALL_CPU_FEATURES is sorted.
 list(SORT ALL_CPU_FEATURES)
 
diff --git a/libc/cmake/modules/LLVMLibCCheckMPFR.cmake b/libc/cmake/modules/LLVMLibCCheckMPFR.cmake
index a27c2dc0c030bc..c0f0d0e34b2816 100644
--- a/libc/cmake/modules/LLVMLibCCheckMPFR.cmake
+++ b/libc/cmake/modules/LLVMLibCCheckMPFR.cmake
@@ -12,6 +12,7 @@ else()
     ${CMAKE_CURRENT_BINARY_DIR}
     SOURCES
     ${LIBC_SOURCE_DIR}/utils/MPFRWrapper/check_mpfr.cpp
+    COMPILE_DEFINITIONS ${LIBC_COMPILE_OPTIONS_DEFAULT}
     LINK_LIBRARIES
       -lmpfr -lgmp -latomic
   )
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index e3dfe1a1529691..45dfe3e63302bf 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -188,7 +188,10 @@ endfunction()
 function(_get_common_test_compile_options output_var c_test flags)
   _get_compile_options_from_flags(compile_flags ${flags})
 
-  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${compile_flags})
+  set(compile_options
+      ${LIBC_COMPILE_OPTIONS_DEFAULT}
+      ${LIBC_TEST_COMPILE_OPTIONS_DEFAULT}
+      ${compile_flags})
 
   if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
     list(APPEND compile_options "-fpie")
@@ -232,9 +235,12 @@ function(_get_common_test_compile_options output_var c_test flags)
 endfunction()
 
 function(_get_hermetic_test_compile_options output_var flags)
-  _get_compile_options_from_flags(compile_flags ${flags})
-  list(APPEND compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${compile_flags}
-       ${flags} -fpie -ffreestanding -fno-exceptions -fno-rtti)
+  _get_common_test_compile_options(compile_options "" "${flags}")
+
+  list(APPEND compile_options "-fpie")
+  list(APPEND compile_options "-ffreestanding")
+  list(APPEND compile_options "-fno-exceptions")
+  list(APPEND compile_options "-fno-rtti")
 
   # The GPU build requires overriding the default CMake triple and architecture.
   if(LIBC_TARGET_ARCHITECTURE_IS_AMDGPU)
@@ -248,9 +254,5 @@ function(_get_hermetic_test_compile_options output_var flags)
          -nogpulib -march=${LIBC_GPU_TARGET_ARCHITECTURE} -fno-use-cxa-atexit)
   endif()
 
-  if(LLVM_LIBC_FULL_BUILD)
-    list(APPEND compile_options "-DLIBC_FULL_BUILD")
-  endif()
-  
   set(${output_var} ${compile_options} PARENT_SCOPE)
 endfunction()
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index d8097855d16377..f899efc3cc5ec8 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -138,7 +138,7 @@ function(create_libc_unittest fq_target_name)
 
   _get_common_test_compile_options(compile_options "${LIBC_UNITTEST_C_TEST}"
                                    "${LIBC_UNITTEST_FLAGS}")
-  list(APPEND compile_options ${LIBC_UNITTEST_COMPILE_OPTIONS})
+  list(APPEND compile_options ${LIBC_UNITTEST_COMPILE_OPTIONS} -static)
 
   if(SHOW_INTERMEDIATE_OBJECTS)
     message(STATUS "Adding unit test ${fq_target_name}")
@@ -192,6 +192,7 @@ 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 ${compile_options})
+  target_link_options(${fq_build_target_name} PRIVATE ${compile_options})
 
   if(NOT LIBC_UNITTEST_CXX_STANDARD)
     set(LIBC_UNITTEST_CXX_STANDARD ${CMAKE_CXX_STANDARD})
@@ -465,8 +466,9 @@ function(add_integration_test test_name)
   target_include_directories(${fq_build_target_name} SYSTEM PRIVATE ${LIBC_INCLUDE_DIR})
   target_include_directories(${fq_build_target_name} PRIVATE ${LIBC_SOURCE_DIR})
 
-  _get_hermetic_test_compile_options(compile_options "${INTEGRATION_TEST_COMPILE_OPTIONS}")
-  target_compile_options(${fq_build_target_name} PRIVATE ${compile_options})
+  _get_hermetic_test_compile_options(compile_options "")
+  target_compile_options(${fq_build_target_name} PRIVATE
+                         ${compile_options} ${INTEGRATION_TEST_COMPILE_OPTIONS})
 
   if(LIBC_TARGET_ARCHITECTURE_IS_AMDGPU)
     target_link_options(${fq_build_target_name} PRIVATE
@@ -638,11 +640,12 @@ function(add_libc_hermetic test_name)
       #OUTPUT_NAME ${fq_target_name}
   )
 
-  _get_hermetic_test_compile_options(compile_options "${HERMETIC_TEST_COMPILE_OPTIONS}")
   target_include_directories(${fq_build_target_name} SYSTEM PRIVATE ${LIBC_INCLUDE_DIR})
   target_include_directories(${fq_build_target_name} PRIVATE ${LIBC_SOURCE_DIR})
-  _get_hermetic_test_compile_options(compile_options "${HERMETIC_TEST_COMPILE_OPTIONS}")
-  target_compile_options(${fq_build_target_name} PRIVATE ${compile_options})
+  _get_hermetic_test_compile_options(compile_options "")
+  target_compile_options(${fq_build_target_name} PRIVATE
+                         ${compile_options}
+                         ${HERMETIC_TEST_COMPILE_OPTIONS})
 
   set(link_libraries "")
   foreach(lib IN LISTS HERMETIC_TEST_LINK_LIBRARIES)
diff --git a/libc/test/UnitTest/CMakeLists.txt b/libc/test/UnitTest/CMakeLists.txt
index 6427b861580777..a10d6581f668d6 100644
--- a/libc/test/UnitTest/CMakeLists.txt
+++ b/libc/test/UnitTest/CMakeLists.txt
@@ -20,18 +20,17 @@ function(add_unittest_framework_library name)
       ${TEST_LIB_HDRS}
     )
     target_include_directories(${lib} PUBLIC ${LIBC_SOURCE_DIR})
-    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)
-      list(APPEND compile_options -ffixed-point)
-    endif()
-    target_compile_options(${lib} PUBLIC ${compile_options})
   endforeach()
-  _get_hermetic_test_compile_options(compile_options -nostdinc++)
+
+  _get_common_test_compile_options(compile_options "" "")
+  target_compile_options(${name}.unit PRIVATE ${compile_options})
+
+  _get_hermetic_test_compile_options(compile_options "")
   target_include_directories(${name}.hermetic PRIVATE ${LIBC_BUILD_DIR}/include)
-  target_compile_options(${name}.hermetic PRIVATE ${compile_options})
+  target_compile_options(${name}.hermetic PRIVATE ${compile_options} -nostdinc++)
 
   if(TEST_LIB_DEPENDS)
     foreach(dep IN ITEMS ${TEST_LIB_DEPENDS})
diff --git a/libc/utils/MPFRWrapper/CMakeLists.txt b/libc/utils/MPFRWrapper/CMakeLists.txt
index be0415d0956fa4..6feaf131da39e3 100644
--- a/libc/utils/MPFRWrapper/CMakeLists.txt
+++ b/libc/utils/MPFRWrapper/CMakeLists.txt
@@ -4,7 +4,8 @@ if(LIBC_TESTS_CAN_USE_MPFR)
     MPFRUtils.h
     mpfr_inc.h
   )
-  target_compile_options(libcMPFRWrapper PRIVATE -O3)
+  _get_common_test_compile_options(compile_options "" "")
+  target_compile_options(libcMPFRWrapper PRIVATE -O3 ${compile_options})
   add_dependencies(
     libcMPFRWrapper
     libc.src.__support.CPP.array

jhuber6

Overall fine, works for me when applied.

jhuber6 · 2024-08-29T19:18:06Z

libc/cmake/modules/LLVMLibCTestRules.cmake

+  _get_hermetic_test_compile_options(compile_options "")
+  target_compile_options(${fq_build_target_name} PRIVATE
+                         ${compile_options} ${INTEGRATION_TEST_COMPILE_OPTIONS})


Why move this?

The input argument for _get_hermetic_test_compile_options is actually for FLAGS, which will be expanded to compile options. The purpose is that if you want to append extra compile options for your target, you should do that outside of the _get_..._compile_options functions.

jhuber6

Worked on the GPU at least.

jhuber6 · 2024-09-06T05:49:34Z

libc/test/UnitTest/CMakeLists.txt

+  if(LLVM_LIBC_FULL_BUILD)
+    # TODO: Build test framework with LIBC_FULL_BUILD in full build mode after
+    # making LibcFPExceptionHelpers and LibcDeathTestExecutors hermetic.
+    set(LLVM_LIBC_FULL_BUILD "")


Maybe we could make this an option to the function instead?

This is only temporary. I plan to remove it in the next 1-2 weeks.

lntue requested review from michaelrj-google, overmighty, jhuber6, mikhailramalho and SchrodingerZhu August 26, 2024 20:14

jhuber6 reviewed Aug 26, 2024

View reviewed changes

lntue force-pushed the crossbuild branch from 55f16df to d6230b1 Compare August 29, 2024 16:18

lntue marked this pull request as ready for review August 29, 2024 16:18

llvmbot added the libc label Aug 29, 2024

jhuber6 approved these changes Aug 29, 2024

View reviewed changes

lntue and others added 6 commits September 6, 2024 05:04

[libc] Fix unit test compile flags propagation.

57e4e52

save current progress.

1c47a72

Revert test changes.

b93fceb

Address comments.

aa10b5f

Remove -static from unit tests.

c769f57

Temporarily fix full build.

5859bc8

lntue force-pushed the crossbuild branch from e0737b2 to 5859bc8 Compare September 6, 2024 05:08

Fix include dir for libMPFRWrapper.

37c182a

jhuber6 approved these changes Sep 6, 2024

View reviewed changes

lntue merged commit 80cf21d into llvm:main Sep 6, 2024
7 checks passed

lntue deleted the crossbuild branch September 6, 2024 15:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc] Fix unit test compile flags propagation. #106128

[libc] Fix unit test compile flags propagation. #106128

Uh oh!

lntue commented Aug 26, 2024 •

edited

Loading

Uh oh!

jhuber6 Aug 26, 2024

Uh oh!

lntue Aug 29, 2024

Uh oh!

jhuber6 Aug 26, 2024

Uh oh!

lntue Aug 29, 2024

Uh oh!

lntue Aug 29, 2024

Uh oh!

Uh oh!

github-actions bot commented Aug 28, 2024 •

edited

Loading

Uh oh!

llvmbot commented Aug 29, 2024

Uh oh!

jhuber6 left a comment

Uh oh!

jhuber6 Aug 29, 2024

Uh oh!

lntue Aug 29, 2024

Uh oh!

jhuber6 left a comment

Uh oh!

jhuber6 Sep 6, 2024

Uh oh!

lntue Sep 6, 2024

Uh oh!

Uh oh!

Uh oh!

[libc] Fix unit test compile flags propagation. #106128

[libc] Fix unit test compile flags propagation. #106128

Uh oh!

Conversation

lntue commented Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 29, 2024

Uh oh!

jhuber6 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhuber6 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lntue commented Aug 26, 2024 •

edited

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading