-
Notifications
You must be signed in to change notification settings - Fork 19
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
Implement rocSOLVER dsyevd for MAGMA build #597
Conversation
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
(I don't have the necessary GPU hardware though to test this.)
CMakeLists.txt
Outdated
@@ -253,6 +253,35 @@ if (CUDA_FOUND) | |||
include_directories(${CUDA_INCLUDE_DIRS}) | |||
endif() | |||
|
|||
list(APPEND CMAKE_PREFIX_PATH $ENV{ROCM_PATH}) |
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 it be done in the build script instead of here?
CMakeLists.txt
Outdated
endif() | ||
|
||
if(BML_ROCSOLVER) | ||
find_package(rocblas) |
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.
Should REQUIRED be added as an argument here? Build should fail if the roc solver is requested but rocblas is not found, right?
CMakeLists.txt
Outdated
@@ -454,6 +492,9 @@ if(MAGMA_FOUND) | |||
if(BML_CUSOLVER) | |||
list(APPEND LINK_LIBRARIES ${CUDA_cusolver_LIBRARY}) | |||
endif() | |||
if(BML_ROCSOLVER) | |||
list(APPEND LINK_LIBRARIES ${ROCM_rocsolver_LIBRARY}) |
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.
rocsolver_LIBRARIES is used earlier. Is it the same as ROCM_ro?csolver_LIBRARIES
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 just pushed a revision. Are these comments addressed?
@mewall Did you verify that none of your additions conflict with the magma build on NVIDIA? |
o Pattern after cuSOLVER implementation o Build is controlled using BML_ROCSOLVER - Like BML_CUSOLVER
I built on summit using BML_CUSOLVER. There doesn't appear to be any conflict with the NVIDIA MAGMA build. However, some other tests are failing on summit. I'll create an issue.
…________________________________
From: Jean-Luc Fattebert ***@***.***>
Sent: Monday, March 7, 2022 9:05:46 AM
To: lanl/bml
Cc: Wall, Michael E; Mention
Subject: [EXTERNAL] Re: [lanl/bml] Implement rocSOLVER dsyevd for MAGMA build (PR #597)
@mewall<https://github.com/mewall> Did you verify that none of your additions conflict with the magma build on NVIDIA?
—
Reply to this email directly, view it on GitHub<#597 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AA67VELGQDSC6NUSAB2K4YTU6YSNVANCNFSM5QDVWXTQ>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
o Pattern after cuSOLVER implementation
o Build is controlled using BML_ROCSOLVER
- Like BML_CUSOLVER