-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[Offload] Add check-offload-unit for liboffload unittests #137312
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
Conversation
@llvm/pr-subscribers-offload Author: Callum Fare (callumfare) ChangesAdds a The target is not part of Also remove the Full diff: https://github.com/llvm/llvm-project/pull/137312.diff 10 Files Affected:
diff --git a/offload/test/CMakeLists.txt b/offload/test/CMakeLists.txt
index 4768d9ccf223b..6e103ccc720c8 100644
--- a/offload/test/CMakeLists.txt
+++ b/offload/test/CMakeLists.txt
@@ -63,3 +63,37 @@ add_offload_testsuite(check-offload
EXCLUDE_FROM_CHECK_ALL
DEPENDS llvm-offload-device-info omptarget ${OMP_DEPEND} ${LIBOMPTARGET_TESTED_PLUGINS}
ARGS ${LIBOMPTARGET_LIT_ARG_LIST})
+
+# Add liboffload unit tests based on available devices rather than configured targets
+macro(add_offload_unittest_suite target_name)
+ set(OFFLOAD_PLATFORM ${target_name})
+ string(TOLOWER "${OFFLOAD_PLATFORM}" SUITE_NAME)
+
+ configure_lit_site_cfg(
+ ${CMAKE_CURRENT_SOURCE_DIR}/unit/lit.site.cfg.in
+ ${CMAKE_CURRENT_BINARY_DIR}/unit/${SUITE_NAME}/lit.site.cfg
+ MAIN_CONFIG
+ ${CMAKE_CURRENT_SOURCE_DIR}/unit/lit.cfg.py)
+
+ add_lit_testsuite(check-offload-unit-${SUITE_NAME} "Running offload unittest suite for ${SUITE_NAME}"
+ ${CMAKE_CURRENT_BINARY_DIR}/unit/${SUITE_NAME}
+ EXCLUDE_FROM_CHECK_ALL
+ DEPENDS OffloadUnitTests)
+
+ list(APPEND OFFLOAD_UNIT_TEST_SUITES ${CMAKE_CURRENT_BINARY_DIR}/unit/${SUITE_NAME})
+endmacro()
+
+set (OFFLOAD_UNIT_TEST_SUITES "")
+
+if (LIBOMPTARGET_FOUND_NVIDIA_GPU)
+ add_offload_unittest_suite("CUDA")
+endif()
+
+if (LIBOMPTARGET_FOUND_AMDGPU_GPU)
+ add_offload_unittest_suite("AMDGPU")
+endif()
+
+add_lit_testsuite(check-offload-unit "Running offload unittest suites"
+ ${OFFLOAD_UNIT_TEST_SUITES}
+ EXCLUDE_FROM_CHECK_ALL
+ DEPENDS LLVMOffload OffloadUnitTests)
diff --git a/offload/test/lit.cfg b/offload/test/lit.cfg
index f7ed287e7aece..0725b56f0f05d 100644
--- a/offload/test/lit.cfg
+++ b/offload/test/lit.cfg
@@ -69,7 +69,7 @@ config.name = 'libomptarget :: ' + config.libomptarget_current_target
config.suffixes = ['.c', '.cpp', '.cc', '.f90', '.cu', '.td']
# excludes: A list of directories to exclude from the testuites.
-config.excludes = ['Inputs']
+config.excludes = ['Inputs', 'unit']
# test_source_root: The root path where tests are located.
config.test_source_root = os.path.dirname(__file__)
diff --git a/offload/test/unit/lit.cfg.py b/offload/test/unit/lit.cfg.py
new file mode 100644
index 0000000000000..12800f3469948
--- /dev/null
+++ b/offload/test/unit/lit.cfg.py
@@ -0,0 +1,24 @@
+# -*- Python -*-
+
+# Configuration file for the 'lit' test runner.
+
+import os
+import subprocess
+
+import lit.formats
+
+# name: The name of this test suite.
+config.name = "Offload-Unit-{}".format(config.offload_platform)
+
+# suffixes: A list of file extensions to treat as test files.
+config.suffixes = []
+
+config.environment = { "OFFLOAD_UNITTEST_PLATFORM" : config.offload_platform}
+
+# 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 = os.path.join(config.library_dir, "unittests")
+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")
diff --git a/offload/test/unit/lit.site.cfg.in b/offload/test/unit/lit.site.cfg.in
new file mode 100644
index 0000000000000..aa1484058b576
--- /dev/null
+++ b/offload/test/unit/lit.site.cfg.in
@@ -0,0 +1,9 @@
+@AUTO_GEN_COMMENT@
+
+config.library_dir = "@LIBOMPTARGET_LIBRARY_DIR@"
+config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
+config.offload_platform = "@OFFLOAD_PLATFORM@"
+
+# Let the main config do the real work.
+lit_config.load_config(config, "@CMAKE_CURRENT_SOURCE_DIR@/unit/lit.cfg.py")
+
diff --git a/offload/unittests/CMakeLists.txt b/offload/unittests/CMakeLists.txt
index 25ac4b2fa3675..f9cb56ae0c024 100644
--- a/offload/unittests/CMakeLists.txt
+++ b/offload/unittests/CMakeLists.txt
@@ -1,9 +1,8 @@
-add_custom_target(LibomptUnitTests)
-set_target_properties(LibomptUnitTests PROPERTIES FOLDER "Tests/UnitTests")
+add_custom_target(OffloadUnitTests)
+set_target_properties(OffloadUnitTests PROPERTIES FOLDER "Tests/UnitTests")
-function(add_libompt_unittest test_dirname)
- add_unittest(LibomptUnitTests ${test_dirname} ${ARGN})
+function(add_offload_unittest test_dirname)
+ add_unittest(OffloadUnitTests ${test_dirname} ${ARGN})
endfunction()
-# add_subdirectory(Plugins)
add_subdirectory(OffloadAPI)
diff --git a/offload/unittests/OffloadAPI/CMakeLists.txt b/offload/unittests/OffloadAPI/CMakeLists.txt
index c4d628a5a87f8..fb480e0443583 100644
--- a/offload/unittests/OffloadAPI/CMakeLists.txt
+++ b/offload/unittests/OffloadAPI/CMakeLists.txt
@@ -4,7 +4,7 @@ set(PLUGINS_TEST_INCLUDE ${LIBOMPTARGET_INCLUDE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}
add_subdirectory(device_code)
message(${OFFLOAD_TEST_DEVICE_CODE_PATH})
-add_libompt_unittest("offload.unittests"
+add_offload_unittest("offload.unittests"
${CMAKE_CURRENT_SOURCE_DIR}/common/Environment.cpp
${CMAKE_CURRENT_SOURCE_DIR}/platform/olGetPlatformInfo.cpp
${CMAKE_CURRENT_SOURCE_DIR}/platform/olGetPlatformInfoSize.cpp
@@ -22,7 +22,7 @@ add_libompt_unittest("offload.unittests"
${CMAKE_CURRENT_SOURCE_DIR}/kernel/olGetKernel.cpp
${CMAKE_CURRENT_SOURCE_DIR}/kernel/olLaunchKernel.cpp
)
-add_dependencies("offload.unittests" ${PLUGINS_TEST_COMMON} LibomptUnitTestsDeviceBins)
+add_dependencies("offload.unittests" ${PLUGINS_TEST_COMMON} OffloadUnitTestsDeviceBins)
target_compile_definitions("offload.unittests" PRIVATE DEVICE_CODE_PATH="${OFFLOAD_TEST_DEVICE_CODE_PATH}")
target_link_libraries("offload.unittests" PRIVATE ${PLUGINS_TEST_COMMON})
target_include_directories("offload.unittests" PRIVATE ${PLUGINS_TEST_INCLUDE})
diff --git a/offload/unittests/OffloadAPI/common/Environment.cpp b/offload/unittests/OffloadAPI/common/Environment.cpp
index 88cf33e45f3d5..7c2e86c0ec931 100644
--- a/offload/unittests/OffloadAPI/common/Environment.cpp
+++ b/offload/unittests/OffloadAPI/common/Environment.cpp
@@ -65,6 +65,13 @@ ol_device_handle_t TestEnvironment::getDevice() {
static ol_device_handle_t Device = nullptr;
if (!Device) {
+ if (const char *EnvStr = getenv("OFFLOAD_UNITTEST_PLATFORM")) {
+ if (SelectedPlatform != "")
+ errs() << "Warning: --platform argument ignored as "
+ "OFFLOAD_UNITTEST_PLATFORM env var overrides it.\n";
+ SelectedPlatform = EnvStr;
+ }
+
if (SelectedPlatform != "") {
olIterateDevices(
[](ol_device_handle_t D, void *Data) {
diff --git a/offload/unittests/OffloadAPI/device_code/CMakeLists.txt b/offload/unittests/OffloadAPI/device_code/CMakeLists.txt
index ded555b3a3cf4..5814943e4aaa9 100644
--- a/offload/unittests/OffloadAPI/device_code/CMakeLists.txt
+++ b/offload/unittests/OffloadAPI/device_code/CMakeLists.txt
@@ -62,6 +62,6 @@ endif()
add_offload_test_device_code(foo.c foo)
add_offload_test_device_code(bar.c bar)
-add_custom_target(LibomptUnitTestsDeviceBins DEPENDS ${BIN_PATHS})
+add_custom_target(OffloadUnitTestsDeviceBins DEPENDS ${BIN_PATHS})
set(OFFLOAD_TEST_DEVICE_CODE_PATH ${CMAKE_CURRENT_BINARY_DIR} PARENT_SCOPE)
diff --git a/offload/unittests/Plugins/CMakeLists.txt b/offload/unittests/Plugins/CMakeLists.txt
deleted file mode 100644
index 06e5288ad6f6b..0000000000000
--- a/offload/unittests/Plugins/CMakeLists.txt
+++ /dev/null
@@ -1,11 +0,0 @@
-set(PLUGINS_TEST_COMMON omptarget)
-set(PLUGINS_TEST_SOURCES NextgenPluginsTest.cpp)
-set(PLUGINS_TEST_INCLUDE ${LIBOMPTARGET_INCLUDE_DIR})
-
-foreach(PLUGIN IN LISTS LIBOMPTARGET_TESTED_PLUGINS)
- message(STATUS "Building plugin unit tests for ${PLUGIN}")
- add_libompt_unittest("${PLUGIN}.unittests" ${PLUGINS_TEST_SOURCES})
- add_dependencies("${PLUGIN}.unittests" ${PLUGINS_TEST_COMMON} ${PLUGIN})
- target_link_libraries("${PLUGIN}.unittests" PRIVATE ${PLUGINS_TEST_COMMON} ${PLUGIN})
- target_include_directories("${PLUGIN}.unittests" PRIVATE ${PLUGINS_TEST_INCLUDE})
-endforeach()
diff --git a/offload/unittests/Plugins/NextgenPluginsTest.cpp b/offload/unittests/Plugins/NextgenPluginsTest.cpp
deleted file mode 100644
index 479b3f614aed2..0000000000000
--- a/offload/unittests/Plugins/NextgenPluginsTest.cpp
+++ /dev/null
@@ -1,167 +0,0 @@
-//===------- unittests/Plugins/NextgenPluginsTest.cpp - Plugin tests ------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "omptarget.h"
-#include "gtest/gtest.h"
-
-#include <unordered_set>
-
-const int DEVICE_ID = 0;
-std::unordered_set<int> setup_map;
-
-int init_test_device(int ID) {
- if (setup_map.find(ID) != setup_map.end()) {
- return OFFLOAD_SUCCESS;
- }
- if (__tgt_rtl_init_plugin() == OFFLOAD_FAIL ||
- __tgt_rtl_init_device(ID) == OFFLOAD_FAIL) {
- return OFFLOAD_FAIL;
- }
- setup_map.insert(ID);
- return OFFLOAD_SUCCESS;
-}
-
-// Test plugin initialization
-TEST(NextgenPluginsTest, PluginInit) {
- EXPECT_EQ(OFFLOAD_SUCCESS, init_test_device(DEVICE_ID));
-}
-
-// Test GPU allocation and R/W
-TEST(NextgenPluginsTest, PluginAlloc) {
- int32_t test_value = 23;
- int32_t host_value = -1;
- int64_t var_size = sizeof(int32_t);
-
- // Init plugin and device
- EXPECT_EQ(OFFLOAD_SUCCESS, init_test_device(DEVICE_ID));
-
- // Allocate memory
- void *device_ptr =
- __tgt_rtl_data_alloc(DEVICE_ID, var_size, nullptr, TARGET_ALLOC_DEFAULT);
-
- // Check that the result is not null
- EXPECT_NE(device_ptr, nullptr);
-
- // Submit data to device
- EXPECT_EQ(OFFLOAD_SUCCESS, __tgt_rtl_data_submit(DEVICE_ID, device_ptr,
- &test_value, var_size));
-
- // Read data from device
- EXPECT_EQ(OFFLOAD_SUCCESS, __tgt_rtl_data_retrieve(DEVICE_ID, &host_value,
- device_ptr, var_size));
-
- // Compare values
- EXPECT_EQ(host_value, test_value);
-
- // Cleanup data
- EXPECT_EQ(OFFLOAD_SUCCESS,
- __tgt_rtl_data_delete(DEVICE_ID, device_ptr, TARGET_ALLOC_DEFAULT));
-}
-
-// Test async GPU allocation and R/W
-TEST(NextgenPluginsTest, PluginAsyncAlloc) {
- int32_t test_value = 47;
- int32_t host_value = -1;
- int64_t var_size = sizeof(int32_t);
- __tgt_async_info *info;
-
- // Init plugin and device
- EXPECT_EQ(OFFLOAD_SUCCESS, init_test_device(DEVICE_ID));
-
- // Check if device supports async
- // Platforms like x86_64 don't support it
- if (__tgt_rtl_init_async_info(DEVICE_ID, &info) == OFFLOAD_SUCCESS) {
- // Allocate memory
- void *device_ptr = __tgt_rtl_data_alloc(DEVICE_ID, var_size, nullptr,
- TARGET_ALLOC_DEFAULT);
-
- // Check that the result is not null
- EXPECT_NE(device_ptr, nullptr);
-
- // Submit data to device asynchronously
- EXPECT_EQ(OFFLOAD_SUCCESS,
- __tgt_rtl_data_submit_async(DEVICE_ID, device_ptr, &test_value,
- var_size, info));
-
- // Wait for async request to process
- EXPECT_EQ(OFFLOAD_SUCCESS, __tgt_rtl_synchronize(DEVICE_ID, info));
-
- // Read data from device
- EXPECT_EQ(OFFLOAD_SUCCESS,
- __tgt_rtl_data_retrieve_async(DEVICE_ID, &host_value, device_ptr,
- var_size, info));
-
- // Wait for async request to process
- EXPECT_EQ(OFFLOAD_SUCCESS, __tgt_rtl_synchronize(DEVICE_ID, info));
-
- // Compare values
- EXPECT_EQ(host_value, test_value);
-
- // Cleanup data
- EXPECT_EQ(OFFLOAD_SUCCESS, __tgt_rtl_data_delete(DEVICE_ID, device_ptr,
- TARGET_ALLOC_DEFAULT));
- }
-}
-
-// Test GPU data exchange
-TEST(NextgenPluginsTest, PluginDataSwap) {
- int32_t test_value = 23;
- int32_t host_value = -1;
- int64_t var_size = sizeof(int32_t);
-
- // Look for compatible device
- int DEVICE_TWO = -1;
- for (int i = 1; i < __tgt_rtl_number_of_devices(); i++) {
- if (__tgt_rtl_is_data_exchangable(DEVICE_ID, i)) {
- DEVICE_TWO = i;
- break;
- }
- }
-
- // Only run test if we have multiple GPUs to test
- // GPUs must be compatible for test to work
- if (DEVICE_TWO >= 1) {
- // Init both GPUs
- EXPECT_EQ(OFFLOAD_SUCCESS, init_test_device(DEVICE_ID));
- EXPECT_EQ(OFFLOAD_SUCCESS, init_test_device(DEVICE_TWO));
-
- // Allocate memory on both GPUs
- // DEVICE_ID will be the source
- // DEVICE_TWO will be the destination
- void *source_ptr = __tgt_rtl_data_alloc(DEVICE_ID, var_size, nullptr,
- TARGET_ALLOC_DEFAULT);
- void *dest_ptr = __tgt_rtl_data_alloc(DEVICE_TWO, var_size, nullptr,
- TARGET_ALLOC_DEFAULT);
-
- // Check for success in allocation
- EXPECT_NE(source_ptr, nullptr);
- EXPECT_NE(dest_ptr, nullptr);
-
- // Write data to source
- EXPECT_EQ(OFFLOAD_SUCCESS, __tgt_rtl_data_submit(DEVICE_ID, source_ptr,
- &test_value, var_size));
-
- // Transfer data between devices
- EXPECT_EQ(OFFLOAD_SUCCESS,
- __tgt_rtl_data_exchange(DEVICE_ID, source_ptr, DEVICE_TWO,
- dest_ptr, var_size));
-
- // Read from destination device (DEVICE_TWO) memory
- EXPECT_EQ(OFFLOAD_SUCCESS, __tgt_rtl_data_retrieve(DEVICE_TWO, &host_value,
- dest_ptr, var_size));
-
- // Ensure match
- EXPECT_EQ(host_value, test_value);
-
- // Cleanup
- EXPECT_EQ(OFFLOAD_SUCCESS, __tgt_rtl_data_delete(DEVICE_ID, source_ptr,
- TARGET_ALLOC_DEFAULT));
- EXPECT_EQ(OFFLOAD_SUCCESS, __tgt_rtl_data_delete(DEVICE_TWO, dest_ptr,
- TARGET_ALLOC_DEFAULT));
- }
-}
|
✅ With the latest revision this PR passed the Python code formatter. |
15458e7
to
ea0ea57
Compare
offload/test/CMakeLists.txt
Outdated
|
||
set (OFFLOAD_UNIT_TEST_SUITES "") | ||
|
||
if (LIBOMPTARGET_FOUND_NVIDIA_GPU) |
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.
I feel like the unit tests should be mostly generic. For the lit
tests in OpenMP we have features that mark the tests as unsupported if it requires a specific GPU. For this, it'd probably be ideal to just return success if it's not an AMD GPU for example, that's still something to test.
@@ -65,6 +65,13 @@ ol_device_handle_t TestEnvironment::getDevice() { | |||
static ol_device_handle_t Device = nullptr; | |||
|
|||
if (!Device) { | |||
if (const char *EnvStr = getenv("OFFLOAD_UNITTEST_PLATFORM")) { |
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.
Shouldn't we just be able to loop over all the discovered platforms? Most of this stuff is intended to be generic after all.
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.
It's definitely possible, it would require all tests being parameterized on the device, which wouldn't be a massive change.
One reason I added the explicit option was to avoid it auto selecting the host plugin, which is basically unsupported at this point. But I could make sure that the test environment filters it out of the device list, or even just filter it out at the liboffload implementation level since it's not really usable for anything just now.
Another reason was to allow the check-offload-unit
tests to be broken down into a test suite config for each available platform (check-offload-unit-cuda
etc), in the same way check-offload
/check-libomptarget
has sub-targets for every configuration. I feel like it's a little bit easier to parse what's actually being tested rather than one test suite that runs on a variable number of configurations.
I don't have strong feelings about it though so I'm happy to make the change if you want.
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.
Lol, forgot about the host plugin and I doubt anyone cares about it. Honestly, the liboffload
API probably shouldn't even present it to the user until someone cares enough to update it.
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.
Nice
Is the PR message still accurate? |
@jhuber6 Thanks for the review! Yeah it's still accurate, I updated it after the last change. It's still possible to restrict tests to a specific platform if you run the test binary manually with that flag, but the check-offload-unit target runs it on everything. |
@jplehr Once this is a bit more mature we should look into adding this to the bot. |
Adds a `check-offload-unit` target for running the liboffload unit test suite. This unit test binary runs the tests for every available device. This can optionally filtered to devices from a single platform, but the check target runs on everything. The target is not part of `check-offload` and does not get propagated to the top level build. I'm not sure if either of these things are desirable, but I'm happy to look into it if we want. Also remove the `offload/unittests/Plugins` test as it's dead code and doesn't build.
Adds a `check-offload-unit` target for running the liboffload unit test suite. This unit test binary runs the tests for every available device. This can optionally filtered to devices from a single platform, but the check target runs on everything. The target is not part of `check-offload` and does not get propagated to the top level build. I'm not sure if either of these things are desirable, but I'm happy to look into it if we want. Also remove the `offload/unittests/Plugins` test as it's dead code and doesn't build.
Adds a `check-offload-unit` target for running the liboffload unit test suite. This unit test binary runs the tests for every available device. This can optionally filtered to devices from a single platform, but the check target runs on everything. The target is not part of `check-offload` and does not get propagated to the top level build. I'm not sure if either of these things are desirable, but I'm happy to look into it if we want. Also remove the `offload/unittests/Plugins` test as it's dead code and doesn't build.
Adds a `check-offload-unit` target for running the liboffload unit test suite. This unit test binary runs the tests for every available device. This can optionally filtered to devices from a single platform, but the check target runs on everything. The target is not part of `check-offload` and does not get propagated to the top level build. I'm not sure if either of these things are desirable, but I'm happy to look into it if we want. Also remove the `offload/unittests/Plugins` test as it's dead code and doesn't build.
Adds a `check-offload-unit` target for running the liboffload unit test suite. This unit test binary runs the tests for every available device. This can optionally filtered to devices from a single platform, but the check target runs on everything. The target is not part of `check-offload` and does not get propagated to the top level build. I'm not sure if either of these things are desirable, but I'm happy to look into it if we want. Also remove the `offload/unittests/Plugins` test as it's dead code and doesn't build.
Adds a
check-offload-unit
target for running the liboffload unit test suite. This unit test binary runs the tests for every available device. This can optionally filtered to devices from a single platform, but the check target runs on everything.The target is not part of
check-offload
and does not get propagated to the top level build. I'm not sure if either of these things are desirable, but I'm happy to look into it if we want.Also remove the
offload/unittests/Plugins
test as it's dead code and doesn't build.