Skip to content
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 HIP TRSM #122

Merged
merged 1 commit into from
Sep 6, 2024
Merged

Add HIP TRSM #122

merged 1 commit into from
Sep 6, 2024

Conversation

abouteiller
Copy link
Contributor

This is based over #121 as this is needed for it to work

I have used the info handles in a different way than we did for potrf, I think its better than having to set taskpool internals in the wrapper

@abouteiller abouteiller added the enhancement New feature or request label Aug 14, 2024
@abouteiller abouteiller self-assigned this Aug 14, 2024
@abouteiller abouteiller requested a review from a team as a code owner August 14, 2024 13:49
src/dplasmaaux_cuda.h Outdated Show resolved Hide resolved
src/ztrsm_LLN.jdf Outdated Show resolved Hide resolved
@abouteiller abouteiller force-pushed the feature/hip-trsm branch 2 times, most recently from 701267c to f06ece2 Compare September 5, 2024 19:57
@QingleiCao
Copy link

Some CUDA and HIP parameters are separate. Do we have a plan to make CUDA and HIP supported at the same time in the future?

BTW, what I'm doing in HiCMA is adding a header file to redefine CUDA variables (at least for most of them) so that minimal change is needed.

@abouteiller
Copy link
Contributor Author

Some CUDA and HIP parameters are separate. Do we have a plan to make CUDA and HIP supported at the same time in the future?

Yes eventually that would be ideal. Right now it is not possible, but the code in dplasma is written in the way that would make it possible when parsec supports it.

BTW, what I'm doing in HiCMA is adding a header file to redefine CUDA variables (at least for most of them) so that minimal change is needed.

We decided against doing a .h trickery because that almost works, but not always (rocsolver and cusolver do not always take the same parameters, in some cases you need a workspaces, but not in the other, etc.), and having a common code is therefore problematic and more error prone than helpful.

Copy link
Contributor

@devreal devreal left a comment

Choose a reason for hiding this comment

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

LGTM

@abouteiller
Copy link
Contributor Author

CI error is #115 and can be ignored

@abouteiller abouteiller merged commit 599a680 into ICLDisco:master Sep 6, 2024
2 of 3 checks passed
@abouteiller abouteiller deleted the feature/hip-trsm branch October 24, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants