Skip to content

[Offload] Run unit tests as a part of check-offload #156675

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 3, 2025
Merged

[Offload] Run unit tests as a part of check-offload #156675

merged 1 commit into from
Sep 3, 2025
+1 −1

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Sep 3, 2025

Summary:
Add a dependnecy on the unit tests on the main check-offload test suite.
This matches what the other projects do, pass llvm-lit to the
directory to only run the lit tests, use the check-offload-unit for
only the unit tests.

@jhuber6
[Offload] Run unit tests as a part of check-offload
ff93ed3 
Summary:
Add a dependnecy on the unit tests on the main check-offload test suite.
This matches what the other projects do, pass `llvm-lit` to the
directory to only run the lit tests, use the `check-offload-unit` for
only the unit tests.
@llvmbot llvmbot added the offload label Sep 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 3, 2025

@llvm/pr-subscribers-offload

Author: Joseph Huber (jhuber6)

Changes

Summary:
Add a dependnecy on the unit tests on the main check-offload test suite.
This matches what the other projects do, pass llvm-lit to the
directory to only run the lit tests, use the check-offload-unit for
only the unit tests.

Full diff: https://github.com/llvm/llvm-project/pull/156675.diff

1 Files Affected:

  • (modified) offload/test/CMakeLists.txt (+1-1)
diff --git a/offload/test/CMakeLists.txt b/offload/test/CMakeLists.txt
index 711621de9075d..c317394ada66f 100644
--- a/offload/test/CMakeLists.txt
+++ b/offload/test/CMakeLists.txt
@@ -61,7 +61,7 @@ add_offload_testsuite(check-offload
   "Running libomptarget tests"
   ${LIBOMPTARGET_LIT_TESTSUITES}
   EXCLUDE_FROM_CHECK_ALL
-  DEPENDS llvm-offload-device-info omptarget ${OMP_DEPEND} ${LIBOMPTARGET_TESTED_PLUGINS}
+  DEPENDS llvm-offload-device-info omptarget ${OMP_DEPEND} ${LIBOMPTARGET_TESTED_PLUGINS} check-offload-unit
   ARGS ${LIBOMPTARGET_LIT_ARG_LIST})
 
 # Add liboffload unit tests - the test binary will run on all available devices

jplehr
jplehr approved these changes Sep 3, 2025
View reviewed changes
Copy link
Contributor

@jplehr jplehr left a comment

Choose a reason for hiding this comment

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

lgtm
I think this is the right thing to do.

@jhuber6 jhuber6 merged commit 99f61f3 into llvm:main Sep 3, 2025
11 checks passed
@jplehr
Copy link
Contributor

jplehr commented Sep 4, 2025

It appears that this breaks the ability to do LIT_FILTER=offload/some-test ninja check-offload with a test coming from the lit tests.

Leaving this here as an observation and something we likely want to fix at some point.

@jhuber6
Copy link
Contributor Author

jhuber6 commented Sep 4, 2025

It appears that this breaks the ability to do LIT_FILTER=offload/some-test ninja check-offload with a test coming from the lit tests.

Leaving this here as an observation and something we likely want to fix at some point.

Hm, maybe the other targets do something slightly different to register their unit tests. Right now we just do add_test.

@mgorny
Copy link
Member

mgorny commented Sep 5, 2025

This change broke standalone builds. I think it is because check-offload-unit was broken in the first place, but now we can't run tests at all:

[57/59] cd /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test && /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/temp/python3.13/bin/python3 /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/llvm-lit -vv -j 96 /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test/unit
/usr/bin/python3.13: can't open file '/var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/llvm-lit': [Errno 2] No such file or directory
FAILED: [code=2] test/CMakeFiles/check-offload-unit /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test/CMakeFiles/check-offload-unit 
cd /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test && /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/temp/python3.13/bin/python3 /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/llvm-lit -vv -j 96 /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test/unit
ninja: build stopped: subcommand failed.

Other tests are using add_offload_testsuite() that seems to configure lit correctly. This one seems to assume that llvm-lit is configured somehow, but offload doesn't do anything like that.

@jhuber6
Copy link
Contributor Author

jhuber6 commented Sep 5, 2025 •
edited
Loading

This change broke standalone builds. I think it is because check-offload-unit was broken in the first place, but now we can't run tests at all:

[57/59] cd /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test && /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/temp/python3.13/bin/python3 /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/llvm-lit -vv -j 96 /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test/unit
/usr/bin/python3.13: can't open file '/var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/llvm-lit': [Errno 2] No such file or directory
FAILED: [code=2] test/CMakeFiles/check-offload-unit /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test/CMakeFiles/check-offload-unit 
cd /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test && /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/temp/python3.13/bin/python3 /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/llvm-lit -vv -j 96 /var/tmp/portage/llvm-runtimes/offload-22.0.0.9999/work/offload_build/test/unit
ninja: build stopped: subcommand failed.

Other tests are using add_offload_testsuite() that seems to configure lit correctly. This one seems to assume that llvm-lit is configured somehow, but offload doesn't do anything like that.

Yeah, I think the offload unittests are using add_test from core CMake which probably isn't compatible. We can revert this for now. @callumfare are you able to look into this?

@jhuber6 jhuber6 deleted the offload_test branch September 5, 2025 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jplehr jplehr jplehr approved these changes

@callumfare callumfare Awaiting requested review from callumfare

@ronlieb ronlieb Awaiting requested review from ronlieb

@RossBrunton RossBrunton Awaiting requested review from RossBrunton

Assignees
No one assigned
Labels
offload
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jhuber6 @llvmbot @jplehr @mgorny