-
Notifications
You must be signed in to change notification settings - Fork 420
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
Add support for rocThrust in sort when using HIP #6793
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
# ROCm 5.6 and earlier set AMDGPU_TARGETS and GPU_TARGETS to all the supported | ||
# architectures. Therefore, we end up compiling Kokkos for all the supported | ||
# architecture. Starting with ROCm 5.7 AMDGPU_TARGETS and GPU_TARGETS are empty. | ||
# It is the user's job to set the variables. Since we are injecting the | ||
# architecture flag ourselves, we can let the variables empty. To replicate the | ||
# behavior of ROCm 5.7 and later for earlier version of ROCm we set | ||
# AMDGPU_TARGETS and GPU_TARGETS to empty and set the values in the cache. If | ||
# the values are not cached, FIND_PACKAGE(rocthrust) will overwrite them. | ||
SET(AMDGPU_TARGETS "" CACHE STRING "AMD GPU targets to compile for") | ||
SET(GPU_TARGETS "" CACHE STRING "GPU targets to compile for") | ||
FIND_PACKAGE(rocthrust REQUIRED) | ||
dalg24 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
KOKKOS_CREATE_IMPORTED_TPL(ROCTHRUST INTERFACE LINK_LIBRARIES roc::rocthrust) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still have the arborx/ArborX#652 issue where the exported rocThrust package potentially adds flags to compile for all GPU architecture it knows about? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At least for the version shipped with ROCm 5.7, it works fine. I see only one architecture. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to find out for sure because IIRC compile time blows up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked at another PR it looks like the compile time is indeed 2x more for both 5.2 and 5.6 so we need to handle this and set the |
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.
You might want to add that setting to
HIP::print_configuration()
@masterleinad same comment about
KOKKOS_ENABLE_ONEDPL
andSYCL
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.
Let me do that after #6758 is merged.