Repository navigation
Fix ACPP Stream Semantics - #65
Merged
Merged
Conversation
davschneller
commented
Sep 28, 2026
davschneller
commented
Contributor
- fix queue waits
- fix ACPP host functions
AdaptiveCpp has no host_task, so streamHostFunction enqueued the function as a custom operation. AdaptiveCpp evaluates a custom operation when it is submitted, not once the operations before it on the queue have completed (its doc/enqueue-custom-operation.md says so explicitly): the function ran on the host right away, while e.g. a copy it depends on could still be running. With AdaptiveCpp, streamHostFunction now waits for the queue and then runs the function, as SeisSol already does on HIP. AI-generated. Model: Claude Opus 5.5
isStreamWorkDone asked AdaptiveCpp for the wait list of the queue and reported the queue as done if that was empty. For an in-order queue that AdaptiveCpp does not emulate, which is what Device creates, get_wait_list submits a new barrier and returns it, so the list is never empty: a caller polling isStreamWorkDone (as SeisSol does for the host MPI transfer mode) waits forever, and each poll adds another operation to the queue. It now uses the queue-empty query of SYCL_KHR_QUEUE_EMPTY_QUERY, which AdaptiveCpp provides, and synchronizes if neither that nor the oneAPI extension is available. AI-generated. Model: Claude Opus 5.5
A host function enqueued on a stream must see the result of the copies enqueued before it, and a stream that has been synchronized must report its work as done within a bounded number of polls. With AdaptiveCpp, both failed before the two previous fixes: a custom operation ran at submission (or, in the library-only OpenMP mode, could not be launched at all), and isStreamWorkDone never returned true. Both tests pass with CUDA and with AdaptiveCpp (omp.library-only). AI-generated. Model: Claude Opus 5.5
DeviceCircularQueueBuffer::deleteQueue deleted a queue made by newQueue, but left its pointer in externalQueues. The next syncAllQueuesWithHost (e.g. syncDevice, or the synchronization at teardown) then waited on the deleted queue: with AdaptiveCpp that crashed, with oneAPI it raised a SYCL error. SeisSol destroys streams in the ghost clusters, and the new stream tests destroy theirs before later tests synchronize the device, which is how the SYCL test jobs failed. The queue is now removed from externalQueues before it is deleted, and a test synchronizes the device after destroying a stream. AI-generated. Model: Claude Opus 5.5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.