-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[memprof] Use COMPILER_RT_TEST_COMPILER #88074
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
[memprof] Use COMPILER_RT_TEST_COMPILER #88074
Conversation
Created using spr 1.3.6-beta.1
@llvm/pr-subscribers-pgo Author: Alexander Richardson (arichardson) ChangesUnlike the other compiler-rt unit tests MemProf was not using the This change should fix the buildbots that compile compiler-rt using Full diff: https://github.com/llvm/llvm-project/pull/88074.diff 1 Files Affected:
diff --git a/compiler-rt/lib/memprof/tests/CMakeLists.txt b/compiler-rt/lib/memprof/tests/CMakeLists.txt
index f812bd1f86ff8f..733e7dda72c611 100644
--- a/compiler-rt/lib/memprof/tests/CMakeLists.txt
+++ b/compiler-rt/lib/memprof/tests/CMakeLists.txt
@@ -41,33 +41,47 @@ if(NOT WIN32)
list(APPEND MEMPROF_UNITTEST_LINK_FLAGS -pthread)
endif()
+set(MEMPROF_UNITTEST_DEPS)
+if (TARGET cxx-headers OR HAVE_LIBCXX)
+ list(APPEND MEMPROF_UNITTEST_DEPS cxx-headers)
+endif()
+
set(MEMPROF_UNITTEST_LINK_LIBRARIES
${COMPILER_RT_UNWINDER_LINK_LIBS}
${SANITIZER_TEST_CXX_LIBRARIES})
-list(APPEND MEMPROF_UNITTEST_LINK_LIBRARIES "dl")
-
-if(COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST MEMPROF_SUPPORTED_ARCH)
- # MemProf unit tests are only run on the host machine.
- set(arch ${COMPILER_RT_DEFAULT_TARGET_ARCH})
+append_list_if(COMPILER_RT_HAS_LIBDL -ldl MEMPROF_UNITTEST_LINK_LIBRARIES)
- add_executable(MemProfUnitTests
- ${MEMPROF_UNITTESTS}
- ${COMPILER_RT_GTEST_SOURCE}
- ${COMPILER_RT_GMOCK_SOURCE}
- ${MEMPROF_SOURCES}
+# Adds memprof tests for each architecture.
+macro(add_memprof_tests_for_arch arch)
+ set(MEMPROF_TEST_RUNTIME_OBJECTS
$<TARGET_OBJECTS:RTSanitizerCommon.${arch}>
$<TARGET_OBJECTS:RTSanitizerCommonCoverage.${arch}>
$<TARGET_OBJECTS:RTSanitizerCommonLibc.${arch}>
$<TARGET_OBJECTS:RTSanitizerCommonSymbolizer.${arch}>
- $<TARGET_OBJECTS:RTSanitizerCommonSymbolizerInternal.${arch}>)
- set_target_compile_flags(MemProfUnitTests ${MEMPROF_UNITTEST_CFLAGS})
- set_target_link_flags(MemProfUnitTests ${MEMPROF_UNITTEST_LINK_FLAGS})
- target_link_libraries(MemProfUnitTests ${MEMPROF_UNITTEST_LINK_LIBRARIES})
-
- if (TARGET cxx-headers OR HAVE_LIBCXX)
- add_dependencies(MemProfUnitTests cxx-headers)
- endif()
+ $<TARGET_OBJECTS:RTSanitizerCommonSymbolizerInternal.${arch}>
+ )
+ set(MEMPROF_TEST_RUNTIME RTMemProfTest.${arch})
+ add_library(${MEMPROF_TEST_RUNTIME} STATIC ${MEMPROF_TEST_RUNTIME_OBJECTS})
+ set_target_properties(${MEMPROF_TEST_RUNTIME} PROPERTIES
+ ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+ FOLDER "Compiler-RT Runtime tests")
+ set(MEMPROF_TEST_OBJECTS)
+ generate_compiler_rt_tests(MEMPROF_TEST_OBJECTS
+ MemProfUnitTests "MemProf-${arch}-UnitTest" ${arch}
+ RUNTIME ${MEMPROF_TEST_RUNTIME}
+ DEPS ${MEMPROF_UNITTEST_DEPS}
+ SOURCES ${MEMPROF_UNITTESTS} ${MEMPROF_SOURCES} ${COMPILER_RT_GTEST_SOURCE}
+ COMPILE_DEPS ${MEMPROF_UNIT_TEST_HEADERS}
+ CFLAGS ${MEMPROF_UNITTEST_CFLAGS}
+ LINK_FLAGS ${MEMPROF_UNITTEST_LINK_FLAGS} ${MEMPROF_UNITTEST_LINK_LIBRARIES})
+endmacro()
- set_target_properties(MemProfUnitTests PROPERTIES
- RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
+# Interception unit tests testsuite.
+add_custom_target(MemProfUnitTests)
+set_target_properties(MemProfUnitTests PROPERTIES FOLDER "Compiler-RT Tests")
+if(COMPILER_RT_CAN_EXECUTE_TESTS AND COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST MEMPROF_SUPPORTED_ARCH)
+ # MemProf unit tests are only run on the host machine.
+ foreach(arch ${COMPILER_RT_DEFAULT_TARGET_ARCH})
+ add_memprof_tests_for_arch(${arch})
+ endforeach()
endif()
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but maybe wait with re-landing #88075
I'm no expert, but from description and what I understand this looks good to me. |
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Currently, the testsuite uses the default runtimes path to find the runtimes libraries which may or may not match the just-built runtimes. This change uses the `-resource-dir` flag for clang whenever `COMPILER_RT_TEST_STANDALONE_BUILD_LIBS` is set to ensure that we are actually testing the currently built libraries rather than the ones bundled with `${COMPILER_RT_TEST_COMPILER}`. The existing logic works fine when clang and compiler-rt share the same build directory ``-DLLVM_ENABLE_PROJECTS=clang;compiler-rt`, but when building compiler-rt separately we need to tell the compiler used for the tests where it can find the just-built libraries. This reduces the fixes check-all failures to one in my configuration: ``` cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja -DCMAKE_C_COMPILER=$HOME/output/upstream-llvm/bin/clang -DCMAKE_CXX_COMPILER=$HOME/output/upstream-llvm/bin/clang++ -DCOMPILER_RT_INCLUDE_TESTS=ON -DLLVM_EXTERNAL_LIT=$HOME/build/upstream-llvm-project-build/bin/llvm-lit -DLLVM_CMAKE_DIR=$HOME/output/upstream-llvm -DCOMPILER_RT_DEBUG=OFF -S $HOME/src/upstream-llvm-project/compiler-rt -B $HOME/src/upstream-llvm-project/compiler-rt/cmake-build-all-sanitizers ``` This relands the previous PR with fixes for Windows. Depends on #88074 to be merged first for GCC buildbots. Pull Request: #88075
Currently, the testsuite uses the default runtimes path to find the runtimes libraries which may or may not match the just-built runtimes. This change uses the `-resource-dir` flag for clang whenever `COMPILER_RT_TEST_STANDALONE_BUILD_LIBS` is set to ensure that we are actually testing the currently built libraries rather than the ones bundled with `${COMPILER_RT_TEST_COMPILER}`. The existing logic works fine when clang and compiler-rt share the same build directory ``-DLLVM_ENABLE_PROJECTS=clang;compiler-rt`, but when building compiler-rt separately we need to tell the compiler used for the tests where it can find the just-built libraries. This reduces the fixes check-all failures to one in my configuration: ``` cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja -DCMAKE_C_COMPILER=$HOME/output/upstream-llvm/bin/clang -DCMAKE_CXX_COMPILER=$HOME/output/upstream-llvm/bin/clang++ -DCOMPILER_RT_INCLUDE_TESTS=ON -DLLVM_EXTERNAL_LIT=$HOME/build/upstream-llvm-project-build/bin/llvm-lit -DLLVM_CMAKE_DIR=$HOME/output/upstream-llvm -DCOMPILER_RT_DEBUG=OFF -S $HOME/src/upstream-llvm-project/compiler-rt -B $HOME/src/upstream-llvm-project/compiler-rt/cmake-build-all-sanitizers ``` This relands the previous PR with fixes for Windows. Depends on llvm#88074 to be merged first for GCC buildbots. Pull Request: llvm#88075
This reverts commit 8b2ba6a. The uild errors (see below) were likely due to the same issue PR #88074 fixed. Addressed by following that PR. https://lab.llvm.org/buildbot/#/builders/165/builds/52789 https://lab.llvm.org/buildbot/#/builders/91/builds/25273
Unlike the other compiler-rt unit tests MemProf was not using the
generate_compiler_rt_tests()
helper that ensures the test is compiledusing the test compiler (generally the Clang binary built earlier).
This was exposed by #83088
because it started adding Clang-specific flags to
COMPILER_RT_UNITTEST_CFLAGS if the compiler ID matched "Clang".
This change should fix the buildbots that compile compiler-rt using
a GCC compiler with LLVM_ENABLE_PROJECTS=compiler-rt.