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

Implement desired fence behavior for view initialization #4823

Merged

Conversation

masterleinad
Copy link
Contributor

Fixes #4697 (I already documented the behavior in the Wiki).
Note that ZeroMemset branch currently doesn't call any Kokkos fence already no matter if we are providing an execution space or not. The Kokkos::View initialization already only fenced the given (or default) execution space and this pull request only removes the fence if an execution space is explicitly specified.

@masterleinad masterleinad force-pushed the check_initialization_view_constructor branch from 2045769 to 3b9bddf Compare February 25, 2022 19:31
@masterleinad masterleinad marked this pull request as ready for review February 25, 2022 20:21
@PhilMiller
Copy link
Contributor

Where is the updated documentation in the wiki? I don't see any mention of 'fence' in the View API ref

@PhilMiller
Copy link
Contributor

The only mention of 'fence' I see in the programming guide View page is regarding deallocation of view-of-views

@masterleinad
Copy link
Contributor Author

Where is the updated documentation in the wiki? I don't see any mention of 'fence' in the View API ref

https://github.com/kokkos/ProgrammingGuide/wiki/Kokkos%3A%3AView

@PhilMiller
Copy link
Contributor

Code looks good. I'll want to review the updated documentation (waiting for repo access), and then I'm happy to approve.

@PhilMiller PhilMiller linked an issue Mar 2, 2022 that may be closed by this pull request
@masterleinad
Copy link
Contributor Author

Retest this please.

@PhilMiller
Copy link
Contributor

Please pull in the expanded testing that got added in #4826.

Copy link
Contributor

@PhilMiller PhilMiller left a comment

Choose a reason for hiding this comment

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

Ok, the code is all good, and I believe the pending documentation update matches it.

@dalg24 dalg24 merged commit 374586e into kokkos:develop Mar 23, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Apr 26, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Apr 26, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request May 6, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request May 6, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request May 10, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request May 19, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request May 19, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request May 31, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request May 31, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Jun 22, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Jun 22, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Aug 5, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Aug 5, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Aug 12, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Aug 12, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Aug 30, 2022
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Aug 30, 2022
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.

Document synchronization semantics of View initialization
4 participants