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

Several test cases where async is followed by synchronous execution without intervening wait #39

Open
tschwinge opened this issue Mar 21, 2023 · 6 comments · Fixed by #42

Comments

@tschwinge
Copy link

For example:

#pragma acc exit data delete(a[0:n], b[0:n]) copyout(c[0:n])

That exit data doesn't have an async clause, and neither a wait clause to wait for the preceding async(1) code, and also there is no intervening wait directive or acc_wait. Thus -- per my understanding -- that exit data may start executing while the the preceding async(1) code is still in progress, and thus may unmap data that's still in use.

As I've seen such a pattern not only here but also in several other test cases, I wonder if my understanding is flawed, or if all these should be fixed (in some way still to be determined)?

@ajarmusch
Copy link
Collaborator

Thank you for bringing this issue to my attention. Based on your description, it seems that the current code may lead to incorrect behavior if the exit data starts executing while the preceding async(1) code is still in progress. This could result in data being unmapped prematurely while it's still in use. We will fix this test and start looking into other async tests that need addressing.

When a PR is being made I will link this issue and close it at the time of the merge. Thank you.

@tschwinge
Copy link
Author

If we think that'd be useful, I could also bring this up with the OpenACC Technical Committee for clarification?

@ajarmusch ajarmusch linked a pull request Jun 7, 2023 that will close this issue
@ajarmusch
Copy link
Collaborator

Thank you! Were you able to bring this up with he Committee? We want to make sure our implementation is correct. If you get the chance please take a look at the related PR. Thank you for your patience!!

chrismun pushed a commit to chrismun/OpenACCV-V that referenced this issue Jun 7, 2023
chrismun pushed a commit to chrismun/OpenACCV-V that referenced this issue Jun 7, 2023
…g redundant directives. Also addressed acc_copyin_async faiing test. May be lingering issues related to OpenACCUserGroup#39 still.
chrismun pushed a commit to chrismun/OpenACCV-V that referenced this issue Jun 11, 2023
…t delete directive. also addressed failing acc copyin async test. still may be more issues related to 39, wouldn't close yet.
chrismun added a commit that referenced this issue Jun 16, 2023
addressed issue #39, still in progress..
@chrismun chrismun reopened this Jun 17, 2023
@chrismun
Copy link
Collaborator

chrismun commented Jul 3, 2023

Hi, I added a wait clause after the data construct, but I wasn't confident, and upon revisiting this, I think that the test was ok from the beginning. I think that the data construct runs synchronously and only the operations such as data transfers associated with the async clauses are attempted asynchronously. The data construct has a present clause, which makes sure the async(1) copyin of c is done. I might remove the wait clause I added before the copyout.

@tschwinge
Copy link
Author

OK, so we really need to run this through the OpenACC Technical Committee for clarification; I'll put it onto the agenda. Thanks for your patience.

@jefflarkin
Copy link

jefflarkin commented Jul 13, 2023

Consensus from the technical call on 7/13/23 is that the wait is necessary.

There was a discussion of whether because the data region isn't on queue 1 a wait is also required before the data region.

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

Successfully merging a pull request may close this issue.

4 participants