-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[offload] [test] Skip amdgcn/nvptx tests if detected arch is not built #119006
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
Skip amdgcn and/or nvptx tests, if the detected GPU architecture is not present among GPUs offload was built for. Without this change, the tests are run if any GPU is detected -- which could lead to cryptic test failures, such as the ones reported in llvm#118824.
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-offload Author: Michał Górny (mgorny) ChangesSkip amdgcn and/or nvptx tests, if the detected GPU architecture is not present among GPUs offload was built for. Without this change, the tests are run if any GPU is detected -- which could lead to cryptic test failures, such as the ones reported in #118824. Full diff: https://github.com/llvm/llvm-project/pull/119006.diff 2 Files Affected:
diff --git a/offload/DeviceRTL/CMakeLists.txt b/offload/DeviceRTL/CMakeLists.txt
index 32a7510be980d8..94746b35ea7e0d 100644
--- a/offload/DeviceRTL/CMakeLists.txt
+++ b/offload/DeviceRTL/CMakeLists.txt
@@ -69,6 +69,9 @@ elseif(LIBOMPTARGET_DEVICE_ARCHITECTURES STREQUAL "auto" OR
"${LIBOMPTARGET_NVPTX_DETECTED_ARCH_LIST};${LIBOMPTARGET_AMDGPU_DETECTED_ARCH_LIST}")
endif()
list(REMOVE_DUPLICATES LIBOMPTARGET_DEVICE_ARCHITECTURES)
+# for tests
+set(LIBOMPTARGET_EXPANDED_DEVICE_ARCHITECTURES ${LIBOMPTARGET_DEVICE_ARCHITECTURES}
+ PARENT_SCOPE)
set(include_files
${include_directory}/Allocator.h
diff --git a/offload/test/CMakeLists.txt b/offload/test/CMakeLists.txt
index 8a827e0a625eff..9ab213acd23be2 100644
--- a/offload/test/CMakeLists.txt
+++ b/offload/test/CMakeLists.txt
@@ -37,6 +37,20 @@ string(REGEX MATCHALL "([^\ ]+\ |[^\ ]+$)" SYSTEM_TARGETS "${LIBOMPTARGET_SYSTEM
foreach(CURRENT_TARGET IN LISTS SYSTEM_TARGETS)
string(STRIP "${CURRENT_TARGET}" CURRENT_TARGET)
+ if(CURRENT_TARGET MATCHES "^amdgcn" AND
+ NOT "${LIBOMPTARGET_AMDGPU_DETECTED_ARCH_LIST}"
+ IN_LIST LIBOMPTARGET_EXPANDED_DEVICE_ARCHITECTURES)
+ message(WARNING "Detected AMDGPU arch ${LIBOMPTARGET_AMDGPU_DETECTED_ARCH_LIST} "
+ "not in built arch list, ${CURRENT_TARGET} tests will be skipped")
+ continue()
+ elseif(CURRENT_TARGET MATCHES "^nvptx" AND
+ NOT "${LIBOMPTARGET_DEP_CUDA_ARCH}"
+ IN_LIST LIBOMPTARGET_EXPANDED_DEVICE_ARCHITECTURES)
+ message(WARNING "Detected NVPTX arch ${LIBOMPTARGET_DEP_CUDA_ARCH} "
+ "not in built arch list, ${CURRENT_TARGET} tests will be skipped")
+ continue()
+ endif()
+
add_offload_testsuite(check-libomptarget-${CURRENT_TARGET}
"Running libomptarget tests"
${CMAKE_CURRENT_BINARY_DIR}/${CURRENT_TARGET}
|
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.
A potentially better alternative would be to define a CMake variable when a target is enabled, and then check if the variable exists and is true when enabling tests.
You mean in |
Hmm, except that |
Don't think they depend on eachother, they used to a long time ago. |
Does this version look better? Or should I move the condition straight into |
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
Skip amdgcn and/or nvptx tests, if the detected GPU architecture is not present among GPUs offload was built for. Without this change, the tests are run if any GPU is detected -- which could lead to cryptic test failures, such as the ones reported in #118824.