[blas][rocblas] Take the HIP stream from the interop handle - #765
Open
zjin-lcf wants to merge 1 commit into
Open
[blas][rocblas] Take the HIP stream from the interop handle#765zjin-lcf wants to merge 1 commit into
zjin-lcf wants to merge 1 commit into
Conversation
get_stream asks the queue for a native stream. A SYCL queue owns a pool of HIP streams, so the returned stream is not necessarily the one the enqueued native command was scheduled on, and only that stream is covered by the SYCL event gating the command. Take the stream from the interop handle instead, which is what the cuBLAS backend and the AdaptiveCpp build of this backend already do. Co-authored-by: Cursor <cursoragent@cursor.com>
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
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.
Summary
RocblasScopedContextHandler::get_streamcurrently asks the queue for a native stream:return sycl::get_native<sycl::backend::ext_oneapi_hip>(queue);A SYCL queue owns a pool of HIP streams, so the stream this returns is not necessarily the one the enqueued native command was scheduled on — and only the scheduled stream is covered by the SYCL event that gates the command's completion. Work submitted to a different stream would not be ordered by that event.
The interop handle knows the stream the command was actually scheduled on, so this takes it from there. That matches what the cuBLAS backend already does:
and what the AdaptiveCpp build of this same backend does:
so this makes the DPC++ path consistent with both.
Caveat
This is a consistency fix found while investigating #486, not a fix for an observed failure. I could not measure any behavioural difference from it on the hardware available (MI210 and MI300A) — the mismatch appears to be latent in practice, presumably because the pool commonly hands back the same stream. I am raising it separately from the #486 fix for that reason.
Test plan
developwith the rocBLAS backend (gfx90a, ROCm 7.1.1)Made with Cursor