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

OpenACC: add atomics support #6446

Merged
merged 42 commits into from
Nov 9, 2023
Merged

OpenACC: add atomics support #6446

merged 42 commits into from
Nov 9, 2023

Conversation

dalg24
Copy link
Member

@dalg24 dalg24 commented Sep 14, 2023

Supersedes #5955

seyonglee and others added 30 commits July 20, 2023 12:29
- Remove const_cast()
- Change Kokkos::abort() with printf()
- Add FIXME_OPENACC comment.
(OpenACC C/C++ does not support atomic max/min/mod operations)
Disable TestOpenACC_BitManipulationBuiltins for OpenACC due to errors.
Co-authored-by: Damien L-G <dalg24+github@gmail.com>
Co-authored-by: Damien L-G <dalg24+github@gmail.com>
Re-enable atomic max/min tests for OpenACC.
@dalg24
Copy link
Member Author

dalg24 commented Oct 12, 2023

Retest this please

…fail

due to compiler bugs, which are reported to NVIDIA.
- Change the values of start and end variables in
TestAtomicOperations_double.hpp and TestAtomicOperations_float.hpp so
that atomic-division tests calculate trivial divisions. (In the original
tests, NVHPC compiler failed since device results are slightly different
from the host results due to precision mismatch.)
host and device atomic operations using a relative error.
@dalg24
Copy link
Member Author

dalg24 commented Nov 3, 2023

It looks good to me. Let's re trigger the CI.
Retest this please.

@dalg24 dalg24 changed the title Nothing to see move along OpenACC: add atomics support Nov 3, 2023
@dalg24 dalg24 marked this pull request as ready for review November 6, 2023 05:45
@dalg24
Copy link
Member Author

dalg24 commented Nov 6, 2023

This is ready.
It will conflict with #6534 but these are trivial to resolve.

core/unit_test/TestAtomicOperations.hpp Show resolved Hide resolved
core/unit_test/TestAtomicOperations.hpp Show resolved Hide resolved
Comment on lines +507 to +515
#if defined(KOKKOS_ENABLE_OPENACC) && defined(KOKKOS_COMPILER_NVHPC)
// NVHPC may use different internal precisions for the device and host
// atomic operations. Therefore, relative errors are used to compare the
// host results and device results.
case 5:
return update != 0 ? atomic_op_test_rel<DivAtomicTest, T, ExecSpace>(
old_val, update)
: true;
#else
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that we have very similar "problems" for 32-bit support.

@dalg24
Copy link
Member Author

dalg24 commented Nov 8, 2023

(I applied Bruno's suggestion (proposed in desul/desul#118) and merged develop to make sure there is no surprise after the SYCL updates)

@dalg24
Copy link
Member Author

dalg24 commented Nov 8, 2023

Retest this please

@dalg24 dalg24 merged commit 97a90d5 into kokkos:develop Nov 9, 2023
27 of 28 checks passed
@dalg24 dalg24 deleted the openacc_atomics branch November 9, 2023 23:15
@dalg24 dalg24 mentioned this pull request Nov 9, 2023
@Rombur
Copy link
Member

Rombur commented Nov 13, 2023

This PR outputs an insane number of messages
1: DESUL error in device_atomic_compare_exchange(): Not supported atomic operation in the OpenACC backend
see https://cloud.cees.ornl.gov/jenkins-ci/job/Kokkos/14840/consoleFull

I think it's filling up the server with the log.

@dalg24
Copy link
Member Author

dalg24 commented Nov 13, 2023

Are you able to fix?

@Rombur
Copy link
Member

Rombur commented Nov 13, 2023

Still waiting to get access to the machine to see if that's the issue.

@seyonglee
Copy link
Contributor

Are you able to fix?

Those error messages are printed because some of the internal Kokkos host codes call atomic_compare_exchange() API with non-arithmetic data types. One way to fix this issue would be to silence this message when atomic_compare_exchange() API with a non-arithmetic type is called on the host.

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

Successfully merging this pull request may close these issues.

None yet

4 participants