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)As of commit 95b8741 with merge base c27baa8 ( 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. |
meta-cla
Bot
added
the
CLA Signed
|
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
added
the
release notes: runtime
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, observed on torch-tensorrt at the time of writing, 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. That makes the ordering the graph expresses the ordering the device sees, for a handoff in either direction and between two CUDA delegates, and it needs no event handshake because both backends end up on the same stream. The stream is a sentinel the runtime owns rather than something a handle creates, so releasing a handle frees only the holder. 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. 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. Callers who want that overlap can install a caller stream per program, which still takes precedence over everything here. Test plan: Added backends/cuda/runtime/test/test_cuda_delegate_stream.cpp. It checks that two handles resolve to the same per-thread stream, which is the ordering guarantee, and that releasing a handle leaves that stream usable, which is what the sentinel requires. It needs no TensorRT, so it runs wherever the other C++ tests for this backend run. Both tests pass. backends/cuda/tests/test_coalesced_determinism.py exports a program split across both backends, checks the split happened, 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, which includes CI, so it is a local check rather than a gate. 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, twenty-nine tests. 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 next to the other stream names, or the ROCm build of this backend would not compile. Not measured: 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 30, 2026 19:27
315df2c to
95b8741
Compare
shoumikhin
marked this pull request as ready for review
August 30, 2026 21:08
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 concurrency, and it meant nothing ordered one delegate's work against the next. 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 were 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.
Using the per-thread stream for every handle makes the ordering the graph expresses the ordering the device sees, for a handoff in either direction and between two CUDA delegates. It is what the TensorRT delegate already uses, so no event handshake is needed to bridge the two. The stream is a sentinel rather than something this code owns, so the holder is released without destroying it.
Callers that want a stream of their own still have both existing mechanisms: the
use_shared_cuda_streamoption, and a caller stream, which continues to take precedence.What it is, and what it is not
compute-sanitizerreports no invalid access and no uninitialized read, so this is not memory corruption. It is an ordering violation, and the program is correct underCUDA_LAUNCH_BLOCKING=1, which changes only ordering. A fullcudaStreamSynchronizeat the end ofexecute()also fixes it but costs 1.65x on a split program, because it stalls the calling thread once per delegate. Sharing the stream costs nothing per execute.Test plan
Added
backends/cuda/tests/test_coalesced_determinism.py. It exports a program split across both backends, checks the split really happened, runs it a hundred times on one loaded program, and requires every result to equal the first exactly. Equality rather than a tolerance, because a delegate reading early returns a different answer, not a nearby one.That test passes with this change and fails without it, at the second of a hundred runs.
Measured on Linux aarch64, sm_110, on a program of twenty-five delegates:
The three C++ test binaries for this backend pass, twenty-nine tests. The Python suite for this backend has eighteen failures on this machine both with and without this change, so it introduces no regressions. Those failures are an unrelated gap: the delegate cannot lower a matmul or a convolution on this architecture, because it restricts autotuning to Triton and Triton has no template for it there.
CUDA graph paths
Those two exits from
execute()are gated on an option only reachable from C++, so a small harness drove them directly, linking both delegates with--no-as-neededso each registers. With capture and replay enabled, forty runs of the same coalesced program agreed with each other and with the same program run without them. So the paths execute and are deterministic under this change.That harness does not validate numerics, only agreement, so treat it as evidence the paths still work rather than evidence they are numerically right. Someone with the export-side setup for those paths should confirm the values.
Not covered
x86, where the reordering this fixes does not reproduce. The change should be neutral there, but I have not measured it.