From f431aa9ec426a46e48928041098a4fdfb39d70bb Mon Sep 17 00:00:00 2001 From: Michael Halkenhaeuser Date: Wed, 17 Sep 2025 12:59:59 -0500 Subject: [PATCH] [OpenMP][omptest] Improve CMake and address review comments Avoid explicit ABI breaking check deactivation Replace whole-archive linking with dedicated build of GoogleTest lib --- openmp/tools/omptest/CMakeLists.txt | 62 +++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 16 deletions(-) diff --git a/openmp/tools/omptest/CMakeLists.txt b/openmp/tools/omptest/CMakeLists.txt index b313f223c354c..c9fd9643eb10e 100644 --- a/openmp/tools/omptest/CMakeLists.txt +++ b/openmp/tools/omptest/CMakeLists.txt @@ -51,7 +51,7 @@ add_library(omptest ) # Target: ompTest library -# On (implicit) request of GoogleTest, link against the one provided with LLVM. +# On (implicit) request of GoogleTest, embed the sources provided with LLVM. if ((NOT LIBOMPTEST_BUILD_STANDALONE) OR LIBOMPTEST_BUILD_UNITTESTS) # Check if standalone build was requested together with unittests if (LIBOMPTEST_BUILD_STANDALONE) @@ -65,26 +65,56 @@ if ((NOT LIBOMPTEST_BUILD_STANDALONE) OR LIBOMPTEST_BUILD_UNITTESTS) set(LIBOMPTEST_BUILD_STANDALONE OFF) endif() - # Add dependency llvm_gtest; emits error if unavailable. - add_dependencies(omptest llvm_gtest) + # Set and check GTest's source directory. + set(OMPTEST_GTEST_PATH ${LLVM_THIRD_PARTY_DIR}/unittest/googletest) + if(NOT EXISTS "${OMPTEST_GTEST_PATH}/src/gtest-all.cc") + message(FATAL_ERROR "Missing gtest-all.cc under ${OMPTEST_GTEST_PATH}. " + "Make sure LLVM_THIRD_PARTY_DIR is set properly.") + endif() + + # Build GTest as OBJECT library, so we can merge it into omptest. + add_library(omptest_gtest OBJECT "${OMPTEST_GTEST_PATH}/src/gtest-all.cc") - # Link llvm_gtest as whole-archive to expose required symbols - set(GTEST_LINK_CMD "-Wl,--whole-archive" llvm_gtest - "-Wl,--no-whole-archive" LLVMSupport) + # Ensure PIC for inclusion into a shared library on ELF platforms. + set_target_properties(omptest_gtest PROPERTIES POSITION_INDEPENDENT_CODE ON) - # Add GoogleTest-based header - target_sources(omptest PRIVATE ./include/OmptTesterGoogleTest.h) + # Set GTest include directories for omptest_gtest + target_include_directories(omptest_gtest PRIVATE + "${OMPTEST_GTEST_PATH}" + "${OMPTEST_GTEST_PATH}/include" + ) - # Add LLVM-provided GoogleTest include directories. - target_include_directories(omptest PRIVATE - ${LLVM_THIRD_PARTY_DIR}/unittest/googletest/include) + # Set GTest include directories for omptest + target_include_directories(omptest PUBLIC + $ + $ + ) - # TODO: Re-visit ABI breaking checks, disable for now. - target_compile_definitions(omptest PUBLIC - -DLLVM_DISABLE_ABI_BREAKING_CHECKS_ENFORCING) + # Avoid using LLVM-specific ostream helpers inside GoogleTest. + target_compile_definitions(omptest_gtest PRIVATE GTEST_NO_LLVM_SUPPORT=1) - # Link against gtest and gtest_main - target_link_libraries(omptest PRIVATE ${GTEST_LINK_CMD}) + # Add GoogleTest-based header and merge GTest into the shared lib. + target_sources(omptest PRIVATE + ./include/OmptTesterGoogleTest.h + $ + ) + + # Link against LLVMSupport and Threads (recommended for GTest). + find_package(Threads REQUIRED) + target_link_libraries(omptest PUBLIC LLVMSupport Threads::Threads) + + # Ensure that embedded GTest symbols are exported from libomptest.so even in + # builds that default to hidden. + set_target_properties(omptest PROPERTIES + CXX_VISIBILITY_PRESET default + VISIBILITY_INLINES_HIDDEN OFF + ) + + # Make sure correct include directories are used, e.g. by the unit tests. + # Otherwise, ABI-checks may fail. + if(DEFINED LLVM_INCLUDE_DIRS) + target_include_directories(omptest PUBLIC ${LLVM_INCLUDE_DIRS}) + endif() else() # Add 'standalone' compile definitions target_compile_definitions(omptest PRIVATE