Skip to content

[HIP] Adds GTest Filter for Kokkos #226

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

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

jplehr
Copy link
Contributor

@jplehr jplehr commented Apr 1, 2025

This filters-out two tests that are currently failing.

This filters-out two tests that are currently failing.
@jplehr jplehr requested review from yxsamliu, ronlieb and jhuber6 April 1, 2025 09:30
Copy link

@mhalk mhalk left a comment

Choose a reason for hiding this comment

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

LGTM.
(Assuming you checked for regressions and actual effect. ;))

@@ -141,7 +141,7 @@ macro(create_hip_tests)
)
add_custom_target(build-kokkos DEPENDS TestKokkosHIP)
ExternalProject_Get_Property(TestKokkosHIP BINARY_DIR)
add_custom_target(test-kokkos COMMAND "ctest" WORKING_DIRECTORY "${BINARY_DIR}" DEPENDS build-kokkos)
add_custom_target(test-kokkos ${CMAKE_COMMAND} -E env GTEST_FILTER=-hip.atomics:hip.bit_manip_bit_ceil "ctest" WORKING_DIRECTORY "${BINARY_DIR}" DEPENDS build-kokkos)
Copy link

Choose a reason for hiding this comment

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

Is COMMAND obsolete here?
Would have expected to see: COMMAND ${CMAKE_COMMAND} -E [...]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The honest answer is that I don't know. This is how I found it for the "Command Line Tool Mode" (https://cmake.org/cmake/help/latest/manual/cmake.1.html#run-a-command-line-tool), which appears to be the way how to do it when you want to run a command and change the environment.

@jplehr jplehr merged commit 9734912 into llvm:main Apr 1, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants