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

Enable ROCm target based on existing CUDA/Thrust implementation. #1914

Merged
merged 29 commits into from
Sep 4, 2023

Conversation

sfantao
Copy link
Contributor

@sfantao sfantao commented Aug 24, 2023

Summary

Enable ROCm target based on existing CUDA/Thrust implementation.

Changelog: New Feature

Details and comments

Add the AER_THURST_ROCM to signal the a ROCm target is desired. Build system changes included to select specific GPU targets and leverage CMake support for HIP. The porting approach uses macro replacement to match CUDA runtime calls with their HIP counterparts. There is an assumption being made that the wavefronts/warps have all threads active, expanding that assumption to the ROCm implementation that uses a wider wave-front.

@doichanj doichanj self-requested a review August 25, 2023 01:24
@doichanj doichanj added the Changelog: New Feature Include in the Added section of the changelog label Aug 25, 2023
@sfantao
Copy link
Contributor Author

sfantao commented Aug 30, 2023

I believe the docs parsing issues have been fixed in the last update. @doichanj, let me know your thoughts.

@doichanj doichanj added the enhancement New feature or request label Sep 1, 2023
@doichanj doichanj added this to the Aer 0.13.0 milestone Sep 1, 2023
Copy link
Collaborator

@doichanj doichanj Sep 1, 2023

Choose a reason for hiding this comment

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

Please remove prelude section from release note.
Could you add similar explanation how to build from source for AMD GPU in CONTRIBUTING.md ?

Add ROCm build instructions.
Copy link
Collaborator

@doichanj doichanj left a comment

Choose a reason for hiding this comment

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

I was so impressed that we can enable AMD GPU by small changes. I think it will be easy to maintain codes in the future

@sfantao
Copy link
Contributor Author

sfantao commented Sep 1, 2023

@doichanj, yes, it was designed to be as little disruptive as possible for ports of already existing CUDA implementations.

I added the Contribution.md changes in the last commit. Let me know if you'd like to include more there.

@doichanj doichanj merged commit d568c6a into Qiskit:main Sep 4, 2023
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the Added section of the changelog enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants