Skip to content

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

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

Merged
merged 7 commits into from
Sep 6, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
57e4e52
[libc] Fix unit test compile flags propagation.
lntue Aug 26, 2024
1c47a72
save current progress.
lntue Aug 28, 2024
b93fceb
Revert test changes.
lntue Aug 29, 2024
aa10b5f
Address comments.
lntue Aug 29, 2024
c769f57
Remove -static from unit tests.
lntue Aug 29, 2024
5859bc8
Temporarily fix full build.
lntue Sep 6, 2024
37c182a
Fix include dir for libMPFRWrapper.
lntue Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions libc/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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})

Expand All @@ -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()

Expand Down
3 changes: 2 additions & 1 deletion libc/cmake/modules/LLVMLibCArchitectures.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
4 changes: 4 additions & 0 deletions libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 3 additions & 1 deletion libc/cmake/modules/LLVMLibCCheckMPFR.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ else()
LIBC_TESTS_CAN_USE_MPFR
${CMAKE_CURRENT_BINARY_DIR}
SOURCES
${LIBC_SOURCE_DIR}/utils/MPFRWrapper/check_mpfr.cpp
${LIBC_SOURCE_DIR}/utils/MPFRWrapper/check_mpfr.cpp
COMPILE_DEFINITIONS
${LIBC_COMPILE_OPTIONS_DEFAULT}
LINK_LIBRARIES
-lmpfr -lgmp -latomic
)
Expand Down
18 changes: 10 additions & 8 deletions libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -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()
13 changes: 8 additions & 5 deletions libc/cmake/modules/LLVMLibCTestRules.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down Expand Up @@ -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})
Comment on lines +469 to +471
Copy link
Contributor

Choose a reason for hiding this comment

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

Why move this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


if(LIBC_TARGET_ARCHITECTURE_IS_AMDGPU)
target_link_options(${fq_build_target_name} PRIVATE
Expand Down Expand Up @@ -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)
Expand Down
26 changes: 17 additions & 9 deletions libc/test/UnitTest/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,27 @@ function(add_unittest_framework_library name)
${TEST_LIB_SRCS}
${TEST_LIB_HDRS}
)
target_include_directories(${lib} PUBLIC ${LIBC_SOURCE_DIR})
list(APPEND compile_options -fno-exceptions -fno-rtti)
target_include_directories(${lib} PRIVATE ${LIBC_SOURCE_DIR})
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++)
target_include_directories(${name}.hermetic PRIVATE ${LIBC_BUILD_DIR}/include)
target_compile_options(${name}.hermetic PRIVATE ${compile_options})

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 "")
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

_get_common_test_compile_options(compile_options "" "")
target_compile_options(${name}.unit PRIVATE ${compile_options})
set(LLVM_LIBC_FULL_BUILD ON)
else()
_get_common_test_compile_options(compile_options "" "")
target_compile_options(${name}.unit PRIVATE ${compile_options})
endif()

_get_hermetic_test_compile_options(compile_options "")
target_include_directories(${name}.hermetic PRIVATE ${LIBC_INCLUDE_DIR})
target_compile_options(${name}.hermetic PRIVATE ${compile_options} -nostdinc++)

if(TEST_LIB_DEPENDS)
foreach(dep IN ITEMS ${TEST_LIB_DEPENDS})
Expand Down
2 changes: 1 addition & 1 deletion libc/test/integration/startup/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function(add_startup_test target_name)
PRIVATE
${LIBC_SOURCE_DIR}
${LIBC_BUILD_DIR}
${LIBC_BUILD_DIR}/include
${LIBC_INCLUDE_DIR}
)

if(ADD_STARTUP_TEST_DEPENDS)
Expand Down
4 changes: 3 additions & 1 deletion libc/utils/MPFRWrapper/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -19,6 +20,7 @@ if(LIBC_TESTS_CAN_USE_MPFR)
target_include_directories(libcMPFRWrapper PUBLIC ${LLVM_LIBC_MPFR_INSTALL_PATH}/include)
target_link_directories(libcMPFRWrapper PUBLIC ${LLVM_LIBC_MPFR_INSTALL_PATH}/lib)
endif()
target_include_directories(libcMPFRWrapper PUBLIC ${LIBC_SOURCE_DIR})
target_link_libraries(libcMPFRWrapper PUBLIC LibcFPTestHelpers.unit LibcTest.unit mpfr gmp)
elseif(NOT LIBC_TARGET_OS_IS_GPU AND NOT LLVM_LIBC_FULL_BUILD)
message(WARNING "Math tests using MPFR will be skipped.")
Expand Down
Loading