Run CUDA delegates on the per-thread stream - #22318
Open
shoumikhin wants to merge 1 commit into
Open
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22318
Note: Links to docs will display an error until the docs builds have been completed. ❌ You can merge normally! (1 Unrelated Failure), 4 Unclassified FailuresAs of commit 80676cc with merge base c27baa8 ( UNCLASSIFIED FAILURES - DrCI could not classify the following jobs because the workflow did not run on the merge base. The failures may be pre-existing on trunk or introduced by this PR:
BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
shoumikhin
force-pushed
the
fix/cuda-delegate-per-thread-stream
branch
3 times, most recently
from
August 30, 2026 18:03
7a149ef to
315df2c
Compare
shoumikhin
force-pushed
the
fix/cuda-delegate-per-thread-stream
branch
from
August 30, 2026 19:27
315df2c to
95b8741
Compare
shoumikhin
marked this pull request as ready for review
August 30, 2026 21:08
shoumikhin
force-pushed
the
fix/cuda-delegate-per-thread-stream
branch
from
August 31, 2026 04:30
95b8741 to
701747b
Compare
shoumikhin
force-pushed
the
fix/cuda-delegate-per-thread-stream
branch
from
August 31, 2026 06:01
701747b to
7d2ce28
Compare
Each delegate handle created its own CUDA stream. Delegates in one program run one after another, so a stream apiece bought no ordering between them: a delegate could read an input while the delegate that produces it still had work queued on a different stream. A program that is entirely one backend does not notice, because the stream is the same throughout. A program split across the CUDA and TensorRT backends does. The TensorRT delegate runs on the per-thread stream when no caller stream is installed, so the two backends sat on different streams with nothing between them. Such a program returned a different answer on almost every call: executing one loaded program forty times produced nine distinct output sums, one of them correct. Every handle now takes the per-thread stream, which makes the ordering the graph expresses the ordering the device sees, in either direction and between two CUDA delegates, with no event handshake needed because both backends end up on the same stream. A caller stream still takes precedence per execute. Ordering consecutive methods against each other is what use_shared_cuda_stream was added to do, and the per-thread stream now does it for every method, so that option would only create a stream nothing uses. It is accepted and ignored, with a log line saying so, and both in-repo callers no longer set it. Because one stream is now shared, a capture that an error abandons would leave that stream capturing for every later delegate on the thread, which would silently have its kernels captured instead of run. A scope guard ends the capture on every exit from the capture step, so a failed run stays local to the handle that failed. Two behaviour changes worth knowing about, both consequences of one stream rather than many: Two threads calling into one loaded method are no longer ordered against each other by the handle's stream, because the per-thread stream is a different stream on each thread. Before, they shared the handle's one created stream and were ordered without anyone asking. A caller driving one method from a pool, and keeping data on the device between calls, now has to order those calls itself. Two independent programs submitted from one host thread with no synchronization between them now run one after the other rather than overlapping, since they share a stream. Measured at about 2x on two equal single block kernels. A caller stream per program restores the overlap. Test plan: backends/cuda/tests/test_coalesced_determinism.py exports a program split across both backends, checks the saved program really contains both backends by reading it back, runs it a hundred times on one loaded program, and requires every result to equal the first exactly. It passes with this change and fails without it at the second run. It skips where the TensorRT delegate is not installed, and now also where it is installed without its C++ runtime, since the export needs that and would otherwise fail rather than skip. Measured on Linux aarch64, sm_110, on a program of twenty-five delegates: before, thirty-nine of forty runs were wrong; after, seven thousand six hundred consecutive runs were correct across several processes. Programs of a single delegate on either backend were already correct and stayed so over five hundred runs each. Median latency for that split program went from about 850 to about 940 microseconds, which is the ordering that was previously skipped. A single delegate program is unchanged. The three existing C++ test binaries for this backend pass. The Python suite for this backend has eighteen failures on this machine both with and without this change, so it introduces no regressions; those are an unrelated gap in matmul and convolution lowering on this architecture. cudaStreamPerThread has no alias in the HIP compatibility header, so this adds one, or the ROCm build of this backend would not compile. Not covered: CI does not install the TensorRT delegate, so the test above skips there and this change has no automated coverage until it does. x86, where the reordering this fixes does not reproduce. And the CUDA graph capture and replay paths beyond confirming that a program using them runs and agrees with itself over forty runs.
shoumikhin
force-pushed
the
fix/cuda-delegate-per-thread-stream
branch
from
August 31, 2026 15:20
7d2ce28 to
80676cc
Compare
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.
Each delegate handle created its own CUDA stream. Delegates in one program run one
after another, so a stream apiece bought no ordering between them: a delegate could
read an input while the delegate that produces it still had work queued on a
different stream.
A program that is entirely one backend does not notice, because the stream is the
same throughout. A program split across the CUDA and TensorRT backends does. The
TensorRT delegate runs on the per-thread stream when no caller stream is installed,
so the two backends sat on different streams with nothing between them. Such a
program returned a different answer on almost every call: executing one loaded
program forty times produced nine distinct output sums, one of them correct.
Every handle now takes the per-thread stream, which makes the ordering the graph
expresses the ordering the device sees, in either direction and between two CUDA
delegates, with no event handshake needed because both backends end up on the same
stream. A caller stream still takes precedence per execute.
Ordering consecutive methods against each other is what use_shared_cuda_stream was
added to do, and the per-thread stream now does it for every method, so that option
would only create a stream nothing uses. It is accepted and ignored, with a log line
saying so, and both in-repo callers no longer set it.
Because one stream is now shared, a capture that an error abandons would leave that
stream capturing for every later delegate on the thread, which would silently have
its kernels captured instead of run. A scope guard ends the capture on every exit
from the capture step, so a failed run stays local to the handle that failed.
Two behaviour changes worth knowing about, both consequences of one stream rather
than many:
Two threads calling into one loaded method are no longer ordered against each other
by the handle's stream, because the per-thread stream is a different stream on each
thread. Before, they shared the handle's one created stream and were ordered without
anyone asking. A caller driving one method from a pool, and keeping data on the
device between calls, now has to order those calls itself.
Two independent programs submitted from one host thread with no synchronization
between them now run one after the other rather than overlapping, since they share a
stream. Measured at about 2x on two equal single block kernels. A caller stream per
program restores the overlap.
Test plan:
backends/cuda/tests/test_coalesced_determinism.py exports a program split across both
backends, checks the saved program really contains both backends by reading it back,
runs it a hundred times on one loaded program, and requires every result to equal the
first exactly.
It passes with this change and fails without it at the second run. It skips where the
TensorRT delegate is not installed, and now also where it is installed without its
C++ runtime, since the export needs that and would otherwise fail rather than skip.
Measured on Linux aarch64, sm_110, on a program of twenty-five delegates: before,
thirty-nine of forty runs were wrong; after, seven thousand six hundred consecutive
runs were correct across several processes. Programs of a single delegate on either
backend were already correct and stayed so over five hundred runs each. Median
latency for that split program went from about 850 to about 940 microseconds, which is
the ordering that was previously skipped. A single delegate program is unchanged.
The three existing C++ test binaries for this backend pass. The Python suite for this
backend has eighteen failures on this machine both with and without this change, so it
introduces no regressions; those are an unrelated gap in matmul and convolution
lowering on this architecture.
cudaStreamPerThread has no alias in the HIP compatibility header, so this adds one, or
the ROCm build of this backend would not compile.
Not covered: CI does not install the TensorRT delegate, so the test above skips there
and this change has no automated coverage until it does. x86, where the reordering
this fixes does not reproduce. And the CUDA graph capture and replay paths beyond
confirming that a program using them runs and agrees with itself over forty runs.