fix(executorch): support KV-cache aliased I/O in the TensorRT delegate - #4445
fix(executorch): support KV-cache aliased I/O in the TensorRT delegate#4445Conarnar wants to merge 2 commits into
Conversation
52d316b to
c5ab1e4
Compare
|
Thanks for adding aliased-I/O metadata to the ExecuTorch blob. This fixes a real missing capability, and the serialization changes look reasonable. I found a blocking issue that applies even when only one method is exported. The patch removes every aliased input from the delegate arguments and replaces it with a private, zero-initialized buffer. If the caller supplies the cache tensor, its existing contents are ignored and the caller cannot observe the update. For example, the expected behavior is: The new behavior is: Checking only Runtime-owned storage also needs a defined lifetime. The current buffer is initialized once, has no reset operation, and is shared by every call using that loaded method. Two conversations, or concurrent requests, would therefore use the same cache. Could caller-owned aliases stay explicit delegate inputs, with the output bound to the same pointer? If runtime-owned storage is genuinely needed, please add explicit ownership and a stable state identity, plus reset or session-selection behavior. On testing, it would help to pin the runtime behavior directly rather than through serialization round-trips: caller-visible mutation, repeated calls on one loaded method, a fresh load starting from a known state, and sequence isolation. For the multi-method case: multi-method Torch-TensorRT ExecuTorch export is still in flight (#4440), so a natural next step is to build a two-method prefill/decode test on top of it and assert that decode observes cache state written by prefill. I want to flag the expected outcome up front: with the current design I believe that test fails, because each method loads as its own delegate with its own private cache, so there is no shared object for the two methods to write through. That is really why I think the cache has to be owned above the delegate and bound into both methods; the shared-cache test is the acceptance criterion for that ownership change rather than something stacking alone will make pass. For mixed TensorRT and CUDA execution there are two independent requirements worth testing separately: (1) both methods bind the same KV-cache storage, and (2) dependent GPU work from the two delegates is ordered. Ordering needs a shared caller stream (there is separate in-flight work for that, #4421), so a mixed test should run on top of it, but note the shared stream only provides (2). Property (1), shared storage across the TensorRT and CUDA delegates, cannot come from a delegate-private buffer, so it also depends on moving cache ownership above the delegate. |
|
Following up with something concrete I should have led with, plus a correction to my own comment. The existing runtime already defines the expected behaviorThe non-ExecuTorch C++ runtime binds an aliased output to the caller's pointer ctx->setTensorAddress(name.c_str(), in_it->second.data_ptr());and So this is not only a question of which design is nicer. It is that the ExecuTorch Correction: the alias kind is never consultedI said checking for (int in_idx : handle->output_aliased_input_idx) {
if (in_idx >= 0) {
handle->input_is_self_owned[in_idx] = true; // no kind check
}
}That matters for On the constraint you hitYour comment in the header explains the real obstacle, and I do not think I gave it My concern is where it gets solved. Working around it inside the delegate means the Two smaller things
The serialization work (carrying Composition noteRe-checking my earlier point about multi-method, in #4440 each method gets its own |
…ng for hybrid graphs
torch_tensorrt.save(retrace=False) uses the legacy dynamo exporter, which inlines the
partitioned _run_on_gpu (non-TensorRT) submodules back into the graph before building an
ExportedProgram. For a hybrid graph interleaving TensorRT engines with a CUDA/pytorch
delegated op, inline_torch_modules wired each submodule's inputs by MATCHING placeholder
names to graph nodes (get_duplicate_nodes). Name matching binds an input to a same-named
but unrelated node on a collision (e.g. a submodule input placeholder name-matching a
different engine's getitem), which:
- rewires a consumer to the wrong producer and orphans the real one; the orphan is then
pruned by dead-code elimination, leaving a delegate short an output at runtime (an
aliased engine reports "expected N args, got N-1"); and
- for a submodule mixing graph-input and computed-intermediate inputs, leaks the
computed intermediates as spurious graph placeholders (misclassified USER_INPUTs).
Wire submodule inputs POSITIONALLY from the call_module args (gm_node.args, which is
authoritative) instead of by name: let graph_copy create a fresh placeholder for each
submodule input, then rewire each to submodule_inputs[i] by position and erase it. Drop
get_duplicate_nodes (now unused).
Also fix two torch-version-compat gaps this path hits on recent torch:
- lift(): pass an explicit persistent= flag on BUFFER InputSpecs (required since 2.3).
- create_trt_exp_program(): an inlined GraphModule may carry a plain fx.CodeGen (no
pytree_info); fall back to specs rebuilt from the example inputs + graph outputs.
With these, retrace=False export of a hybrid TensorRT+CUDA program is bit-identical to
retrace=True (validated on a 2-layer int4 MoE decode: per-step argmax + logits match).
Tests: tests/py/dynamo/models/test_exporter_inlining.py -- positional input wiring under a
name collision, and multi-output preservation (GPU-free fx unit tests).
c5ab1e4 to
40e0486
Compare
What changed vs the earlier (delegate-owned) versionThe earlier revision made the delegate own the KV cache: because ExecuTorch This revision makes the cache caller-owned, above the delegate. Export / lowering (new):
Runtime (changed):
|
Adds end-to-end caller-owned KV-cache support to the ExecuTorch TensorRT
delegate: the KV buffers are owned by the caller above the delegate and threaded
in as mutable-buffer delegate args, instead of being self-allocated inside a
(stateless) TensorRT engine.
Runtime + serialization (delegate):
- serialize each engine's aliased (KV-cache / in-place) I/O into the delegate blob
(serialization.py, backend.py, TensorRTBlobHeader.{h,cpp});
- at runtime bind each aliased TRT output binding to its aliased input's
caller-provided pointer (in-place) and reflect the result into the delegate
output EValue -- a no-op when the memory planner already aliased the two
(TensorRTBackend.{h,cpp}).
Export/lowering (torch_tensorrt):
- expose each engine's aliased outputs as graph-level BUFFER_MUTATIONs so
ExecuTorch keeps the KV buffers as caller-owned mutable buffers: at transform
time for the legacy exporter (retrace=False), and via a post-export pass
(_declare_aliased_kv_mutations_on_ep) for torch.export (retrace=True), which
otherwise truncates the aliased outputs at the fx boundary;
- keep delegate-mutated buffers above the delegate in TensorRTPartitioner
(tag_constant_data would otherwise freeze them as constants).
Tests cover serialization round-trip, the exposure-flag dispatch across both
retrace modes, the buffer-mutation declaration, and the partitioner un-tagging.
40e0486 to
2312f42
Compare
Description
Adds end-to-end caller-owned KV-cache support to the ExecuTorch TensorRT
delegate. The KV buffers are owned by the caller above the delegate and threaded
through as mutable-buffer delegate args (both the input and the engine's aliased
output), so a TensorRT engine updates them in place and the cache persists across
decode steps — matching the contract the non-ExecuTorch TensorRT runtime already
exposes.
Runtime + serialization (delegate)
(
serialization.py,backend.py,TensorRTBlobHeader.{h,cpp}).caller-provided pointer (in-place) and reflect the result into the delegate
output EValue — a no-op when the memory planner already aliased the two
(zero-copy) (
TensorRTBackend.{h,cpp}).USERaliases are shape-validated at init;kv_cache_updatealiases areshape-enforced by TensorRT's
IKVCacheUpdateLayer.Export / lowering (torch_tensorrt)
BUFFER_MUTATIONs soExecuTorch keeps the KV buffers as caller-owned mutable buffers instead of
freezing them: at transform time for the legacy exporter (
retrace=False), andvia a post-export pass (
_declare_aliased_kv_mutations_on_ep) fortorch.export(retrace=True), which otherwise drops the aliased outputs atthe fx boundary.
TensorRTPartitioner(
tag_constant_datawould otherwise freeze them as constants).Dependency
This PR is stacked on #4446 and must land after it — #4446 fixes the legacy
(
retrace=False) submodule inlining that the composable ExecuTorch export pathdepends on.
Follow-up tests (gated on other PRs)
A cross-delegate prefill/decode acceptance test — decode consuming the KV
cache that prefill wrote through a separate per-method delegate — will be added
once #4440 (per-method
TensorRTPartitioner→ separate delegate instances)and #4454 (shared caller CUDA stream, for ordering the dependent GPU work
between the two) land. That configuration is what exercises cross-delegate cache
sharing, which single-delegate tests cannot cover.
Testing
aliased_ioserialization round-trip; blob-header parse(present / empty / missing-key); exposure-flag dispatch across both retrace
modes; the
BUFFER_MUTATIONdeclaration; partitioner keeps onlymutation-target buffers above the delegate.
retrace=Falseandretrace=True— the cache persists in place across steps (step 0 outputreproduces at step 1, then diverges as it accumulates). A non-KV model exports
and runs unchanged (output byte-identical to eager).