Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions openmp/runtime/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ endif()

add_subdirectory(src)
add_subdirectory(test)
add_subdirectory(unittests)

# make these variables available for tools:
set(LIBOMP_LIBRARY_DIR ${LIBOMP_LIBRARY_DIR} PARENT_SCOPE)
Expand Down
45 changes: 41 additions & 4 deletions openmp/runtime/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -174,17 +174,28 @@ if(NOT WIN32)
endif()

# Add the OpenMP library
libomp_get_ldflags(LIBOMP_CONFIGURED_LDFLAGS)

# First, create an OBJECT library with all the runtime sources.
# This allows both the main library and unit tests to link against the same
# compiled objects.
add_library(omp_objects OBJECT ${LIBOMP_SOURCE_FILES})
set_property(TARGET omp_objects PROPERTY FOLDER "OpenMP/Libraries")
set_property(TARGET omp_objects PROPERTY POSITION_INDEPENDENT_CODE ON)
# Export the omp_objects target so unit tests can use it
set(LIBOMP_OBJECTS_TARGET omp_objects PARENT_SCOPE)

libomp_get_ldflags(LIBOMP_CONFIGURED_LDFLAGS)
libomp_get_libflags(LIBOMP_CONFIGURED_LIBFLAGS)
# Build libomp library. Add LLVMSupport dependency if building in-tree with libomptarget profiling enabled.

# Build libomp library. Add LLVMSupport dependency if building in-tree with
# libomptarget profiling enabled.
if(OPENMP_STANDALONE_BUILD OR (NOT OPENMP_ENABLE_LIBOMP_PROFILING))
add_library(omp ${LIBOMP_LIBRARY_KIND} ${LIBOMP_SOURCE_FILES})
add_library(omp ${LIBOMP_LIBRARY_KIND} $<TARGET_OBJECTS:omp_objects>)
Copy link
Member

Choose a reason for hiding this comment

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

[not a change request] According to the documentation, you can also

target_link_libraries(omp PUBLIC omp_objects)

I had mixed success with it; keep $<TARGET_OBJECTS:omp_objects> if it works.

set_property(TARGET omp PROPERTY FOLDER "OpenMP/Libraries")
# Linking command will include libraries in LIBOMP_CONFIGURED_LIBFLAGS
target_link_libraries(omp ${LIBOMP_CONFIGURED_LIBFLAGS} ${LIBOMP_DL_LIBS})
else()
add_llvm_library(omp ${LIBOMP_LIBRARY_KIND} ${LIBOMP_SOURCE_FILES} PARTIAL_SOURCES_INTENDED
add_llvm_library(omp ${LIBOMP_LIBRARY_KIND} $<TARGET_OBJECTS:omp_objects> PARTIAL_SOURCES_INTENDED
LINK_LIBS ${LIBOMP_CONFIGURED_LIBFLAGS} ${LIBOMP_DL_LIBS}
LINK_COMPONENTS Support
BUILDTREE_ONLY
Expand All @@ -200,6 +211,31 @@ if(${LIBOMP_USE_HWLOC})
)
endif()

# Build a testing version of libomp that exports all symbols for unit tests.
# This library uses the same compiled objects as libomp, but with all symbols
# exported to allow testing internal functions.
if(NOT WIN32 AND NOT STUBS_LIBRARY)
set(LIBOMP_TEST_LDFLAGS ${LIBOMP_CONFIGURED_LDFLAGS})
# Replace the libomp exports with the test exports exporting all symbols.
if(LIBOMP_HAVE_VERSION_SCRIPT_FLAG)
string(REPLACE "${LIBOMP_SRC_DIR}/exports_so.txt"
"${LIBOMP_SRC_DIR}/exports_test_so.txt"
LIBOMP_TEST_LDFLAGS "${LIBOMP_TEST_LDFLAGS}")
Copy link
Member

Choose a reason for hiding this comment

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

Instead of string-replacing which feels very hacky, could we make a parameterized version of libomp_get_libflags? E.g.

libomp_get_libflags(LIBOMP_TEST_LDFLAGS FOR_UNITTESTS)

Where FOR_UNITTESTS is a cmake_parse_arguments option in libomp_get_libflags.

I think we don't need exports_test_so.txt at all; just do not use the --version-script switch and all symbols are visible to the unittests (assuming defaut -fvisibility=default).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we don't need exports_test_so.txt at all; just do not use the --version-script switch

then we would need to string-remove that, tho?

Copy link
Member

Choose a reason for hiding this comment

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

The modified libomp_get_libflags would not even add --version-script when collecting LIBOMP_TEST_LIBFLAGS for omp_testing

IMHO string replacement of command line options is a hacky thing to do, better to generate the right options in the first place.

endif()

# Create the testing library from the same objects as libomp.
add_library(omp_testing SHARED $<TARGET_OBJECTS:omp_objects>)
Copy link
Member

Choose a reason for hiding this comment

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

Consider moving the test-related CMake code into the unittests/ directory. In other LLVM projects, the test/ and unittest/ are skipped unlest LLVM_INCLUDE_TESTS (or its project-specific variant such as CLANG_INCLUDE_TESTS) is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did that with the latest commit, but it involves a little bit of cmake duplication, e.g. regarding LIBOMP_LINKER_LANGUAGE. Maybe we should stick to the previous version? Because this situation here is a little bit special in that we need to guarantee that omp_testing and omp are built as identically as possible.

Copy link
Member

Choose a reason for hiding this comment

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

You can hoist the definition of LIBOMP_LINKER_LANGUAGE one level up, i.e. https://github.com/llvm/llvm-project/blob/main/openmp/runtime/CMakeLists.txt, so it is available in both, src/ and unittests/.

I usually add such things to the top-level CMakeLists.txt in the first place. I don't see why definitions such as LIBOMP_VERSION_MAJOR should not be available in the entire project.

set_property(TARGET omp_testing PROPERTY FOLDER "OpenMP/Libraries")
target_link_libraries(omp_testing ${LIBOMP_CONFIGURED_LIBFLAGS} ${LIBOMP_DL_LIBS})
set_target_properties(omp_testing PROPERTIES
PREFIX "" SUFFIX "" OUTPUT_NAME "libomp_testing${LIBOMP_LIBRARY_SUFFIX}"
LINK_FLAGS "${LIBOMP_TEST_LDFLAGS}"
LINKER_LANGUAGE ${LIBOMP_LINKER_LANGUAGE}
EXCLUDE_FROM_ALL TRUE # Don't build by default, only when tests need it
)
add_dependencies(omp_testing libomp-needed-headers)
endif()

if(OPENMP_MSVC_NAME_SCHEME)
if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
set(LIBOMP_PDB_NAME ${LIBOMP_DEFAULT_LIB_NAME}${MSVC_TOOLS_VERSION}d.${LIBOMP_ARCH})
Expand Down Expand Up @@ -296,6 +332,7 @@ set(LIBOMPTARGET_OPENMP_HOST_RTL_FOLDER "${LIBOMP_LIBRARY_DIR}" CACHE STRING
# objects depend on : .inc files
add_custom_target(libomp-needed-headers DEPENDS kmp_i18n_id.inc kmp_i18n_default.inc)
set_target_properties(libomp-needed-headers PROPERTIES FOLDER "OpenMP/Sourcegenning")
add_dependencies(omp_objects libomp-needed-headers)
add_dependencies(omp libomp-needed-headers)

# Windows specific build rules
Expand Down
42 changes: 42 additions & 0 deletions openmp/runtime/unittests/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
add_custom_target(OpenMPUnitTests)
set_target_properties(OpenMPUnitTests PROPERTIES FOLDER "OpenMP/Tests")

if(NOT TARGET llvm_gtest)
message(WARNING "OpenMP unittests disabled due to GTest being unavailable; "
"Try LLVM_INSTALL_GTEST=ON for the LLVM build")
return()
endif()
if(NOT TARGET omp_testing)
message(WARNING "OpenMP unittests disabled due to omp_testing library being unavailable")
return()
endif()

function(add_openmp_unittest test_dirname)
add_unittest(OpenMPUnitTests ${test_dirname} ${ARGN})

# Link against the testing library which exports all symbols.
target_link_libraries(${test_dirname} PRIVATE omp_testing)
add_dependencies(${test_dirname} omp_testing)

target_include_directories(${test_dirname} PRIVATE
${LIBOMP_INCLUDE_DIR}
${LIBOMP_SRC_DIR}
)
endfunction()

configure_lit_site_cfg(
${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
MAIN_CONFIG
${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
)

add_openmp_testsuite(
check-libomp-unit
"Running libomp unit tests"
${CMAKE_CURRENT_BINARY_DIR}
EXCLUDE_FROM_CHECK_ALL
DEPENDS OpenMPUnitTests omp
)

add_subdirectory(src)
9 changes: 9 additions & 0 deletions openmp/runtime/unittests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# libomp Unit Tests

Usage:
```
cd <your-llvm-build-directory>/runtimes/runtimes-bins
ninja check-libomp-unit
```

Note: unit tests are currently not supported on Windows
22 changes: 22 additions & 0 deletions openmp/runtime/unittests/lit.cfg.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# -*- Python -*-

# Configuration file for the 'lit' test runner.

import os
import subprocess

import lit.formats

# name: The name of this test suite.
config.name = "OpenMP-Unit"

# suffixes: A list of file extensions to treat as test files.
config.suffixes = []

# test_source_root: The root path where tests are located.
# test_exec_root: The root path where tests should be run.
config.test_exec_root = config.openmp_unittests_dir
config.test_source_root = config.test_exec_root

# testFormat: The test format to use to interpret tests.
config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, ".unittests")
9 changes: 9 additions & 0 deletions openmp/runtime/unittests/lit.site.cfg.py.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
@AUTO_GEN_COMMENT@

config.library_dir = "@LIBOMP_LIBRARY_DIR@"
config.openmp_unittests_dir = "@CMAKE_CURRENT_BINARY_DIR@"
config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")

# Let the main config do the real work.
lit_config.load_config(config, "@CMAKE_CURRENT_SOURCE_DIR@/lit.cfg.py")

3 changes: 3 additions & 0 deletions openmp/runtime/unittests/src/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
add_openmp_unittest(libomp.unittests
TestKmpStr.cpp
)
Loading
Loading