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

Fix a bug when using HIP atomic with Kokkos::Complex #4159

Merged
merged 1 commit into from
Jul 14, 2021

Conversation

Rombur
Copy link
Member

@Rombur Rombur commented Jul 13, 2021

__ballot returns an unsigned long long int in HIP and not an unsigned int like in CUDA. With this PR, all the tests are enabled for HIP.

__ballot returns an unsigned long long int in HIP and not an unsigned int like in CUDA
Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

Apart from the question of rather using uint64_t to follow the documentation more closely, this looks good to me.
In the end, we should of course fix this also in desul.

@Rombur
Copy link
Member Author

Rombur commented Jul 13, 2021

Retest this please

@Rombur Rombur added this to Awaiting Feedback in Developer: Bruno Turcksin Jul 14, 2021
@crtrott
Copy link
Member

crtrott commented Jul 14, 2021

Regarding Desul, our version is way out of sync with the main desul repo right now since there are tons of fixes in our version which were only uncovered when integrating fully with Kokkos. I intend to do that backport to Desul when we got everything working, and then merge again into Kokkos with whatever changes code review there caused.

@crtrott crtrott merged commit 7398486 into kokkos:develop Jul 14, 2021
@crtrott crtrott added this to In progress in Kokkos Release 3.5 via automation Jul 14, 2021
@crtrott crtrott moved this from In progress to Done in Kokkos Release 3.5 Jul 14, 2021
@brian-kelley
Copy link
Contributor

Thanks for fixing this, all the KokkosKernels tests are now passing with real and complex 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants