Skip to content

feat(executorch): copy-back for non-KV mutable buffers in the TensorRT delegate - #4459

Open
Conarnar wants to merge 13 commits into
pytorch:mainfrom
Conarnar:fix/executorch-copyback-mutable-buffers
Open

feat(executorch): copy-back for non-KV mutable buffers in the TensorRT delegate#4459
Conarnar wants to merge 13 commits into
pytorch:mainfrom
Conarnar:fix/executorch-copyback-mutable-buffers

Conversation

@Conarnar

@Conarnar Conarnar commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Why this PR is needed

Caller-owned KV cache support (#4445) lets a mutable buffer live above the delegate and be updated in place by the engine. It handles this by lifting each mutated buffer to a delegate input and erasing the trailing copy_, relying on TensorRT's IKVCacheUpdateLayer aliasing to write the new value back to the caller-owned storage (zero-copy).

That assumption only holds for KV-cache writes. The slice_scatter and index_copy converters have a fast path that emits an IKVCacheUpdateLayer whose output is aliased in-place to the cache input. Any other in-place mutable buffer has no such aliasing — for example the conv_state / recurrent_state ring-buffers of a Gated DeltaNet (GDN) layer. For those, erasing the copy_ dropped the write-back entirely: the update became dead code and was eliminated, so the engine received fewer args than expected at runtime and the buffer never updated (silently wrong output).

The fix

Distinguish the two kinds of mutation in lift_mutated_buffers:

  • KV writes (slice_scatter / index_copy) keep the existing zero-copy aliasing path — the copy_ is erased and the write-back is handled by the engine's IKVCacheUpdateLayer.
  • Any other ("copy-back") mutation has its new value re-attached as an ordinary graph output and recorded as a BUFFER_MUTATION of its caller-owned buffer, so ExecuTorch copies it back after the delegate runs.

This uses the standard mutable-buffer representation rather than the engine-enforced aliased-I/O path (which is reserved for zero-copy KV aliasing).

Classification reuses the converters' own eligibility predicates rather than the op target alone, so a slice_scatter / index_copy the converter cannot turn into an IKVCacheUpdateLayer falls to copy-back instead of being dropped. assert_predicted_kv_aliased then checks at compile time that every write predicted as KV really does appear in an engine's aliased_io, turning a mis-prediction into a loud error rather than a silently lost write-back.

How it works

The classification happens once, in lift_mutated_buffers, and is threaded to both save paths:

  1. dynamo/lowering/_buffer_lifting.py — for each lifted buffer, a KV write is left to aliasing; a non-KV write has its new value appended as a trailing graph output (so it survives DCE now that the copy_ is gone) and its buffer name recorded in gm.meta["_copyback_mutation_buffers"].
  2. dynamo/_compiler.py — forwards that list onto the compiled module's meta so it reaches the exporters.
  3. dynamo/_exporter.py
    • create_trt_exp_program (retrace=False): tags the trailing outputs with their buffer target and moves all mutation outputs ahead of the user outputs (verifier requirement).
    • _declare_aliased_kv_mutations_on_ep (retrace=True): reclassifies the last len(copyback_buffers) user outputs from USER_OUTPUT to BUFFER_MUTATION and rebuilds the top-level out_spec so to_edge's unflatten sees the right leaf count.

Where copy-back is declared

retrace exporter exported_program executorch aot_inductor
True either declared declared not declared
False legacy (default) declared at transform time declared at transform time declared at transform time
False use_legacy_exporter=False warns warns warns

Under retrace=True the declaration pass now runs on the exported_program branch as well as executorch, so both formats carry a correct BUFFER_MUTATION. aot_inductor stays undeclared, matching #4445's KV scoping.

retrace=False declares copy-back only through the legacy exporter, at transform time. The pass cannot pick it up afterwards: its reclassification is positional, so running it on a program the legacy exporter already declared would retarget outputs that are no longer the trailing ones. retrace=False with use_legacy_exporter=False therefore cannot declare copy-back at all, and now emits a warning rather than silently saving a signature that omits the update.

The change is a no-op for any model without a non-KV in-place mutable buffer — including all standard models and all KV-cache models. A KV-only export is byte-identical to before.

Behaviour change worth flagging

Declaring copy-back twice now raises. The positional reclassification cannot detect an already-declared buffer the way the KV path's already_exposed set does: after a first declaration the mutations sit at the front and the trailing outputs are the user's, so a second pass retargets those — dropping a user output and leaving the buffer with two mutation specs. That combination is reachable today via retrace=True + use_legacy_exporter=True + dynamic shapes, where the legacy exporter declares at transform time and the save path would declare again. It previously produced a corrupted signature silently; it now fails with a clear error.

Testing

Unit tests added in this PR (CPU-only, gated on executorch.exir):

  • tests/py/dynamo/lowering/test_buffer_lifting.py — classification: KV slice_scatter / index_copy writes stay aliased with no copy-back; a non-KV mutation is recorded and its new value re-attached as the trailing output (verified numerically equal to the updated buffer); index_put falls to copy-back; a mixed KV + non-KV graph records only the non-KV buffer; an ineligible index_copy (2-D cache) and an ineligible slice_scatter (wrong dim) both fall to copy-back rather than being dropped. Plus the assert_predicted_kv_aliased backstop: passes when the prediction is aliased, raises when it is not, aggregates aliased_io across multiple engines, and no-ops with no prediction.
  • tests/py/dynamo/executorch/test_kv_cache_export.py — both exporter passes reclassify a trailing copy-back output to BUFFER_MUTATION ahead of the user outputs; a write-only copy-back buffer still reaches lift as a get_attr (drives the real lift/ExportedProgram constructor so a regression reproduces the verifier error rather than passing vacuously); the get_attr re-add does not duplicate an existing one or invent a dangling one; nested (dotted) buffer names are remapped to their flattened form; declaring copy-back twice raises; and save() warns exactly on the retrace=False + non-legacy combination.

The copy-back path was also exercised end-to-end during development — a KV-only model as a no-regression sentinel, and a hybrid TensorRT + CUDA decode model with a convolution-state copy-back buffer (both retrace modes) — but those runs are not part of this PR's automated tests.

Stacking

Stacked on #4445 (caller-owned KV-cache), which is stacked on #4446 (retrace=False legacy-exporter fix). Should land after both.

Follow-ups / interaction with in-flight PRs

@meta-cla meta-cla Bot added the cla signed label Aug 4, 2026
@github-actions github-actions Bot added component: tests Issues re: Tests component: lowering Issues re: The lowering / preprocessing passes component: core Issues re: The core compiler component: api [Python] Issues re: Python API component: api [C++] Issues re: C++ API component: runtime component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels Aug 4, 2026
@cehongwang
cehongwang requested a review from shoumikhin August 4, 2026 23:21
@cehongwang

Copy link
Copy Markdown
Collaborator

#4459 widens the #4445 ordering blocker into a three-way reversal. lift_mutated_buffers appends copy-back values to the graph output:

out_args = list(output_node.args[0])
out_args.extend(nv for nv, _ in copyback)
output_node.args = (tuple(out_args),)

and the interpreter appends aliased KV outputs after that. So the engine binding order becomes [user…, copyback…, kv_aliased…]. But _declare_aliased_kv_mutations_on_ep sets the graph output to kv_getitems + copyback_getitems + out_args, giving delegate args [kv_aliased…, copyback…, user…] — the three groups in reverse. Since preprocess still passes output names through in engine order, a model with all three kinds gets a full permutation mismatch rather than the two-way swap in #4445.

# index_put KV write correctly falls through to copy-back rather than being
# dropped in the false expectation of aliasing. (A dedicated index_put ->
# IKVCacheUpdateLayer converter would let it use zero-copy aliasing instead.)
_KV_WRITE_TARGETS = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This classification is op-level, but aliasing isn't an op-level property — so this can still drop a write-back in exactly the way the PR sets out to fix.

The comment above says the set must "stay in sync with the ops the converters actually turn into an IKVCacheUpdateLayer." The problem is that no set of ops can stay in sync with that, because whether an IKVCacheUpdateLayer gets emitted depends on shapes and network position, not just the target. In slice_scatter.py, _kv_eligible requires:

  • a static s_max
  • a 4-D [b, d, s_max, h] cache
  • dim == 2
  • a non-dynamic batch dim

and on top of that emit_kv_cache_update_layer bails when the cache isn't a direct network input (input_binding_name returns None) or when add_kv_cache_update returns None. index_copy.py has the same eligible/index_copy_fallback split.

When any of those fail, the converter emits a plain scatter with no aliasing — but _is_kv_cache_write already returned True, so the copy_ is erased with no copy-back appended and the write-back is silently lost. A 3-D cache or dim != 2 is enough to hit it.

Before this PR the erase was unconditional, so this failure mode existed for every mutated buffer; this PR narrows it to these two ops but keeps the same "trust the op" assumption for them.

Could the classification be derived from what was actually emitted rather than predicted? The engine's aliased_io map is the ground truth, and _declare_aliased_kv_mutations_on_ep already reads it. Deriving copy-back as "mutated buffer not present in aliased_io" post-conversion would make the two sides agree by construction.

If you'd rather keep the pre-conversion classification to avoid restructuring, the minimum would be a post-conversion assertion: every buffer classified as KV must appear in the engine's aliased_io, otherwise error (or fall back to copy-back) rather than silently dropping the write.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the time aliased_io is generated, the mutation write-backs have already been removed by DCE. Deriving copy-back from it would require re-attaching the write-back for ALL buffers and removing the ones present in aliased_io afterward.

Applied your suggestion to the classification and added the post-conversion assertion.

self.assertEqual(len(lifted), 2)
self.assertEqual(new_gm.meta["_copyback_mutation_buffers"], ["state"])


Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The KV cases here use slice_scatter / index_copy shapes that are KV-eligible, so they only cover the happy path of the classification.

Could you add a case where the op is slice_scatter but the converter's fast path would not fire — e.g. a 3-D cache, or dim != 2, or a dynamic s_max? Today that lands in the KV bucket and loses its write-back. Whatever behavior you settle on (copy-back, or a hard error), a test pinning it would keep the two sides from drifting as _kv_eligible evolves.

@Conarnar

Conarnar commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

#4459 widens the #4445 ordering blocker into a three-way reversal. lift_mutated_buffers appends copy-back values to the graph output:

out_args = list(output_node.args[0])
out_args.extend(nv for nv, _ in copyback)
output_node.args = (tuple(out_args),)

and the interpreter appends aliased KV outputs after that. So the engine binding order becomes [user…, copyback…, kv_aliased…]. But _declare_aliased_kv_mutations_on_ep sets the graph output to kv_getitems + copyback_getitems + out_args, giving delegate args [kv_aliased…, copyback…, user…] — the three groups in reverse. Since preprocess still passes output names through in engine order, a model with all three kinds gets a full permutation mismatch rather than the two-way swap in #4445.

Same with #4445, I was not able to reproduce this.

@Conarnar
Conarnar force-pushed the fix/executorch-copyback-mutable-buffers branch from 6a88214 to a8b0d61 Compare August 6, 2026 20:53
@Conarnar
Conarnar requested a review from cehongwang August 6, 2026 21:47
@cehongwang

Copy link
Copy Markdown
Collaborator

I am not 100% sure, but it seems like, in the decode step the KV cache is written in place by the engine, then copied again by ExecuTorch here: https://github.com/pytorch/executorch/blob/7811c69f41db6cc84abfd0b4a32f2e49393958a5/exir/passes/insert_write_back_for_buffers_pass.py#L163 because it is also a BUFFER_MUTATION output node. Please correct me if I am wrong

@cehongwang

Copy link
Copy Markdown
Collaborator

This fixes the bug for executorch but leaves Torch-TensorRT runtime wrong. @narendasan Should we use the same mechanism to support in-place operation for Torch-TensorRT runtime by appending a copy node at the end of graph and run it in python as graph break? If we want to do that, we should move that part from export.py to compiler or interpreter

@Conarnar

Conarnar commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

I am not 100% sure, but it seems like, in the decode step the KV cache is written in place by the engine, then copied again by ExecuTorch here: https://github.com/pytorch/executorch/blob/7811c69f41db6cc84abfd0b4a32f2e49393958a5/exir/passes/insert_write_back_for_buffers_pass.py#L163 because it is also a BUFFER_MUTATION output node. Please correct me if I am wrong

You are correct, there is a redundant copy added by ET. Unfortunately, removing it would cause ExecuTorch to stop tracking and persisting the mutation. Fixing this properly would require delegate-boundary aliasing support on the ExecuTorch side.

@shoumikhin

Copy link
Copy Markdown
Contributor

A KV cache owned by a submodule is skipped, so the copy-back path never runs.

I tried this PR on a decoder whose per-layer caches live inside the attention modules, which is how a transformer normally stores them, and the export fails. Two separate problems, both about fully qualified buffer names. The second one is arguably yours to fix since this PR adds the consumer; the first is older but this PR is where it starts to matter.

Problem 1: nested buffers are never lifted

lift_mutated_buffers resolves the get_attr target with hasattr/getattr:

buffer_name = get_attr_node.target
if not hasattr(gm, buffer_name):
    logger.warning(
        "lift_mutated_buffers: get_attr target %s not found on gm; skipping",
        buffer_name,
    )
    continue
buffer_tensor = getattr(gm, buffer_name)

A get_attr target is fully qualified, so a cache owned by a submodule arrives as layers.0.self_attn.kv_cache.k_cache. getattr does not walk a dotted path, so this reports every nested buffer as missing even when it is right there:

hasattr(gm, "layer.state")     # False
gm.get_buffer("layer.state")   # the tensor

Every mutation is skipped with a warning, nothing is lifted, and the cache is later frozen as constant data. On my model that was 16 warnings and a silently dead cache.

get_buffer resolves through submodules:

try:
    buffer_tensor = gm.get_buffer(buffer_name)
except AttributeError:
    ...

Problem 2: the copy-back list keeps names that no longer exist

Once problem 1 is fixed, the mutations are found and classified, and then the export fails in the verifier:

SpecViolationError: Buffer output getitem_1 does not point to a buffer that exists.
mutated targets  : 'layers.0.self_attn.kv_cache.k_cache'
buffers available: 'lifted_buf_layers_0_self_attn_kv_cache_k_cache'

inline_lifted_buffers_into_gm has to flatten a dotted name, because register_buffer rejects ".":

if "." in buf_name:
    attr_name = "lifted_buf_" + buf_name.replace(".", "_")
...
buf_to_attr[buf_name] = attr_name

But buf_to_attr is a local variable, and gm.meta["_copyback_mutation_buffers"] still holds the original dotted names. The BUFFER_MUTATION OutputSpec is then built from a name that was renamed out from under it.

A minimal reproducer, no TensorRT needed:

class Attn(torch.nn.Module):
    def __init__(self):
        super().__init__()
        self.register_buffer("state", torch.zeros(4, 8))
    def forward(self, idx, val):
        updated = self.state.index_put([idx], val)
        self.state.copy_(updated)
        return updated.sum(-1)

class Model(torch.nn.Module):          # cache one level down
    def __init__(self):
        super().__init__()
        self.layer = Attn()
    def forward(self, idx, val):
        return self.layer(idx, val)

ep = torch.export.export(Model().eval(), (torch.tensor([0]), torch.zeros(1, 8)), strict=True)
gm, lifted = lift_mutated_buffers(ep.module())
gm = inline_lifted_buffers_into_gm(gm, lifted)

print(gm.meta["_copyback_mutation_buffers"])   # ['layer.state']
print(list(dict(gm.named_buffers())))          # ['lifted_buf_layer_state']

Note the ordering: on this PR as it stands, that snippet stops at problem 1 and prints

lift_mutated_buffers: get_attr target layer.state not found on gm; skipping
copyback targets  : []
registered buffers: ['layer.state']

With problem 1 fixed it gets further and prints the mismatch above. So problem 1 hides problem 2, and fixing only the lookup turns a silently frozen cache into a hard verifier error.

Suggested fix

Remap the copy-back list through the same mapping that renamed the buffers, right after buf_to_attr is built:

gm.meta["_lifted_buffer_attr_names"] = dict(buf_to_attr)
copyback = gm.meta.get("_copyback_mutation_buffers")
if copyback:
    gm.meta["_copyback_mutation_buffers"] = [
        buf_to_attr.get(name, name) for name in copyback
    ]

Recording the mapping in meta as well seems worth it, so a future consumer of buffer names does not have to reconstruct the flattening rule.

The KV aliasing path already avoids this, because it reads the name off the get_attr node (_exporter.py, buf_target = getattr(buffer_node, "target", None)), which is the flat registered name by then. Only the copy-back list carries the pre-rename name.

Why CI does not catch it

Every test in test_buffer_lifting.py registers the buffer on the top-level module:

self.register_buffer("cache", torch.zeros(1, 4, 16, 8))

A flat name has no dots, so buf_name.replace(".", "_") is a no-op and buf_to_attr is the identity. Both problems are invisible. One test with the cache one level down would have caught both, and that is probably the more valuable half of the change, since the fix alone invites the same regression later.

Verified

With both fixes applied I get a working multi-method program: prefill and decode share one caller-owned cache, the runtime reports 16 aliased output(s) bound in-place to caller-owned inputs, an autoregressive loop in C++ produces varying tokens, and prefill logits match eager within 3e-05 (bfloat16), with the same argmax.

Worth noting the failure mode differs per problem, and only one is loud. Problem 2 is caught by the verifier with a clear message. Problem 1 only logs a warning and then produces a program whose cache silently does not persist, which reads as a model quality issue rather than an export bug. If a discovered mutation target cannot be resolved, raising instead of warning would turn that into an immediate, obvious failure.

Environment: torch 2.14.0.dev, TensorRT 11.1.0.106, H100, this PR on top of #4446 and #4445.

@shoumikhin

Copy link
Copy Markdown
Contributor

Two problems when a buffer write is excluded from TensorRT, which together block a two-delegate program.

I hit these while exporting a model that splits across the TensorRT and CUDA delegates. Both are in lift_mutated_buffers, and the second one only becomes visible after the first is fixed.

Problem 1: the aliasing prediction ignores torch_executed_ops

_kv_write_will_alias decides whether a write will become engine-level aliasing by looking at the op. It never sees the ops the caller excluded from TensorRT, so an excluded write is still predicted to alias, its copy_ is dropped, and then the cross-check in compile() fails:

RuntimeError: lift_mutated_buffers classified these buffer writes as KV-cache
(engine-aliased) and dropped their copy_, but the compiled engine did not alias
them (absent from aliased_io): ['buf_layers_0_self_attn_kv_cache_k_cache', ...
16 buffers ...]. Their write-back would be silently dropped.

An op that never reaches a converter cannot emit an IKVCacheUpdateLayer, so it cannot produce aliasing. To reproduce, compile any model with a cache write and exclude that write:

torch_tensorrt.dynamo.compile(
    exported, inputs=inputs, min_block_size=1,
    torch_executed_ops={"torch.ops.aten.index_copy.default"},
)

Credit where due: that guard is what turned this into a clear error naming every affected buffer, instead of a cache that silently stops updating. It did its job.

A fix that worked for me is to pass settings into lift_mutated_buffers and have the predictor honor the exclusion list, matched the same way the partitioner does it so the two cannot disagree:

from torch_tensorrt.dynamo.conversion._ConverterRegistry import ConverterRegistry

excluded = set(settings.torch_executed_ops)
if (ConverterRegistry.qualified_name_or_str(target) in excluded
        or target in excluded):
    return False

Problem 2: an excluded write should not be lifted at all

With the prediction corrected, the export gets further and then fails inside ExecuTorch:

RuntimeError: Tried to erase Node getitem_145 but it still had 1 users
in the graph: {executorch_call_delegate_1: None}

Lifting the buffer turns the mutation into a delegate output. When the write itself runs on the other backend, that output feeds the second delegate, and ExecuTorch cannot express a buffer mutation consumed across a delegate boundary, so _unsafe_adjust_original_program fails while deleting it.

If TensorRT is not converting the write, it should leave the buffer alone entirely and let the copy_ stand for whichever backend claims it:

for copy_node, get_attr_node in mutation_pairs:
    buffer_name = get_attr_node.target
    new_value = copy_node.args[1] if len(copy_node.args) > 1 else None
    if _write_is_excluded_from_tensorrt(new_value, settings):
        continue

Why this matters together

With both applied, one program carries both delegates and runs correctly:

prefill      delegates=['CudaBackend', 'TensorRTBackend']
decode       delegates=['CudaBackend', 'TensorRTBackend']
generated tokens vary across steps, and logits match eager to 9.6e-05

Without them, excluding an op from TensorRT either drops a cache write or fails to lower at all, so a model that needs part of its graph on another backend cannot be exported.

Happy to send these as a follow-up PR once this one lands, or you are welcome to take the snippets directly. They are independent of the copy-back work itself, so they could also go in separately if you would rather keep this PR focused.

@Conarnar

Copy link
Copy Markdown
Contributor Author

@shoumikhin Thanks for taking a look and the thorough writeup. I have taken your suggestion regarding KV caching in submodules and applied the fix to problem 2 here. Problem 1 is a pre-existing bug on main and can be landed separately (#4472).

Regarding excluded buffer writes from TRT, I believe those are out of scope for this PR and can remain in yours (#4470).

…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).
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).

The retrace=True pass runs for exported_program as well as executorch. The
truncation happens at the fx boundary for every output format, so declaring only
on the executorch path left an exported_program saved with the mutation absent
from its signature while the engine still updated the cache in place. It is
declared before _normalize_engine_constants_to_python, which rewrites the engine
constants the pass reads aliased_io from. retrace=False was already correct for
every format via create_trt_exp_program. aot_inductor stays undeclared and
warns: whether an aliased in-place mutation survives functionalization under
inductor is unverified.

Tests cover serialization round-trip, the exposure-flag dispatch across both
retrace modes, the buffer-mutation declaration, and the partitioner un-tagging.
…mutations

_declare_aliased_kv_mutations_on_ep rebuilds the ExportedProgram to attach the
new output specs, but reconstructed only root/graph/signature/state_dict/
range_constraints/module_call_graph/constants. example_inputs and verifiers are
not recoverable from the graph and reset to their defaults when omitted, so the
pass was not a drop-in replacement for the program it rewrites.

That was invisible while the pass ran only on the executorch path, since to_edge
does not read either. Declaring mutations for exported_program as well makes it
reachable: torch.export.save then persists a program whose example inputs are
silently gone. AOTI refuses such a program outright ("exported_program.
example_inputs is required to be set in order for AOTInductor compilation"), so
this also has to be fixed before that format can ever declare mutations.

Carry both through. The stub programs in test_kv_cache_export.py now model them,
and the capturing test asserts they reach the constructor.

Reported by cehongwang in review.
Whether an aliased KV output gets declared a BUFFER_MUTATION is decided in two
places: the exporter (the legacy one declares at transform time, via
create_trt_exp_program) and save()'s per-output-format branch. Nothing reconciles
them, so a program that arrives already declared is declared a second time. The
duplicate spec then fails the ExportedProgram verifier's output ordering check.

Seed already_exposed from the incoming signature's BUFFER_MUTATION targets rather
than an empty set, so the pass skips buffers that are already declared and returns
the program untouched when nothing new remains.

That covers every combination that reaches it, including exported_program with
use_legacy_exporter=True, which the preceding commit made reachable.

The added test drives the pass on an already-declared program with
ExportedProgram monkeypatched to raise, so a regression fails on "rebuilt the
program" rather than on some later verifier complaint. The no-op fixture grows an
output_specs field, which a real ExportGraphSignature always has.

Reported by shoumikhin in review.
Four comments describe a zero-copy path where the memory planner places the
delegate's output slot on the aliased input, so the reflect is skipped. That
never happens: the delegate input and its aliased output are live at the same
time, so the planner cannot co-locate them. Instrumenting the branch over a
30-layer export confirms it -- dst == bind_ptr was false for all 120 aliased
outputs across a prefill and a decode step.

Two of them go further and call the skipped case the common fast path, which
inverts what the code does: every aliased output reflects, and a model with
aliased outputs therefore always syncs before returning.

Say what actually happens in all four (TensorRTBackend.h, and the reflect list,
the reflect loop and the must_sync rationale in TensorRTBackend.cpp). The
dst != bind_ptr check itself stays -- it is unreachable today, but it is what
stops a self-copy if planning ever changes -- and its comment now says so
instead of advertising a fast path.

Also two comment fixes found in the same pass: "These two" in _exporter.py
referred forward to checks the reader had not reached yet, and a test comment
said "previously-dropped" where it meant the output torch.export truncates.

No behaviour change.

Reported by shoumikhin in review.
The aliased-output branch discarded the resize_tensor result and skipped the
resize altogether when the rank was out of range, while the sibling non-aliased
branch treats both as fatal.

et_alias_out.nbytes() is read on the next line and sizes both the reflect D2D
copy and, through the delegate output EValue, ExecuTorch's write-back copy_. A
dynamic aliased output that outgrows its planned size would therefore move the
stale planned byte count in both, silently truncating the cache update rather
than failing.

Mirror the sibling branch: reject an out-of-range rank and propagate a
resize_tensor error.

Reported by shoumikhin in review.
…th too

Whether an aliased KV output is declared a BUFFER_MUTATION depends on two
independent switches. save() picks the exporter (retrace, plus an optional
use_legacy_exporter override) and only the legacy exporter exposes the mutations,
at transform time; save()'s per-format branch declares them for everything else.
The retrace=False branch did neither, so retrace=False with
use_legacy_exporter=False produced a program that silently omits an update the
engine performs -- no declaration and no diagnostic.

Run the declaration pass on the exported_program and executorch branches there as
well. The preceding commit made the pass skip buffers that already carry a spec,
so this is correct for either exporter: the legacy one keeps declaring at
transform time and the pass returns its program untouched.

aot_inductor stays undeclared on both paths, as before.

The added test drives save() over both formats and both exporters and asserts the
pass runs exactly once; against the previous commit all four parametrizations
fail.

Reported by shoumikhin in review.
The runtime binds delegate output i to output_binding_names[i]. That holds
because the partition's outputs are getitem(engine_node, i) in index order, and
nothing verifies it: inputs are checked in _reorder_input_names_for_executorch,
outputs were not. arrange_graph_outputs moves buffer mutations ahead of user
outputs and is a no-op here only while the mutated buffers stay above the
delegate, so a regression there would swap the serialized names silently.

Validate the correspondence in preprocess. A single-output engine returned
unwrapped is accepted -- one binding has no order to get wrong -- and anything
else must be one getitem per binding, in index order.

_build_edge_program only ever emitted `output((engine_node,))`, including for its
three-output-binding case, which is not a shape that can occur: a three-tuple
cannot be consumed as one value. It now emits one getitem per output binding, so
the fixtures model what the backend actually receives.

Reported by shoumikhin in review.
…d_io

aliased_io changes what a blob means. A parser that predates it binds each
aliased output to its own allocation instead of the input it aliases, so it does
not fail -- it returns wrong results. The magic is the only field that parser
validates, so it is the only thing that can make the skew fail closed.

Emit TR02 when metadata.aliased_io is non-empty and keep TR01 otherwise, rather
than bumping unconditionally: a blob with no alias map means exactly what it
meant before, so it stays loadable by an older runtime. Both magics are accepted
on read, so new runtimes still load existing artifacts.

Verified against a real older build rather than a simulated one: a TR02 blob
loads and produces the expected KV-persistence result on a runtime built from
this branch, and a runtime built before the change rejects the same blob with
"failed to parse TensorRT blob".

Reported by shoumikhin in review.
…r verify

The caller-owned KV path had no CI coverage: kv_cache_decode_check ships in the
release tarball and is defined in the packaged CMake project, but nothing built
or ran it, so a regression in the aliased binding would only surface downstream.

Export a decode .pte alongside the static-shape one and pass it to the verify
script as an optional second argument. When present the script builds
kv_cache_decode_check from the unpacked tarball, runs it, and requires the
persistence assertion to pass; the same no-libtorch link check the example runner
gets is applied to it. Without the argument the script behaves as before.

Also assert the tarball ships kv_cache_decode_check.cpp, next to the existing
entries, so the packaging contract is checked rather than assumed.

Reported by shoumikhin in review.
…ering claim

Four `continue`s in _declare_aliased_kv_mutations_on_ep left an aliased output
undeclared without saying so, and the mismatch only surfaced later as a delegate
arity error at execute. Log at each, at the level the case warrants: warn when the
persisted alias map disagrees with the engine's bindings (unknown input name, or
an index past the delegate args) and when the aliased input is not a registered
buffer, since all three leave the engine with an output binding the delegate
cannot satisfy; debug when the buffer already carries a spec, which is the
expected idempotent skip. The non-aliased output path stays silent -- it is the
common case, not a fault.

_reorder_input_names_for_executorch's docstring also justified skipping the output
reordering by claiming a TensorRT partition has no mutation outputs. That has not
been true since aliased I/O landed. The order does survive lowering, but for a
different reason: _keep_mutated_buffers_above_delegate keeps mutated buffers out
of the delegate, so ExecuTorch records the mutation as a USER_OUTPUT and
arrange_graph_outputs computes the identity permutation. Say that, and note the
guarantee is conditional -- _validate_output_binding_order is what enforces it.

Reported by cehongwang in review.
…T delegate

A mutable buffer with no engine aliasing -- a convolution-state ring buffer, say
-- is mutated outside the engine, so the KV aliasing path does not cover it.
lift_mutated_buffers appends its new value as a trailing user output;
reclassifying that output as a BUFFER_MUTATION of its buffer makes ExecuTorch
copy it back after the delegate runs.

Thread the buffer FQNs from gm.meta['_copyback_mutation_buffers'] into
_declare_aliased_kv_mutations_on_ep on the retrace=True executorch path, and have
create_trt_exp_program do the equivalent at transform time on the legacy path.

The reclassification is positional: it takes the trailing len(copyback_buffers)
outputs. That cannot skip an already-declared buffer the way the KV path's
already_exposed set does, because after a first declaration the mutations sit at
the front and the trailing outputs are the user's -- a second pass would retarget
those, dropping a user output and leaving the buffer with two mutation specs.
Reject that combination instead of silently corrupting the signature.
…program

Copy-back was threaded only on the executorch branch, so saving as
exported_program produced a signature that omits an update the program performs.
Hoist _copyback_bufs to the retrace=True block and pass it on both format
branches.

retrace=False is left alone deliberately. Only the legacy exporter declares
copy-back there, at transform time, and the pass cannot pick it up afterwards:
its reclassification is positional, so running it on an already-declared program
would retarget the wrong outputs. That leaves retrace=False with
use_legacy_exporter=False unable to declare copy-back at all, which now warns
instead of silently dropping it, mirroring the aot_inductor warning above.
@Conarnar
Conarnar force-pushed the fix/executorch-copyback-mutable-buffers branch from 718d0c8 to 1d2ed27 Compare August 15, 2026 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla signed component: api [C++] Issues re: C++ API component: api [Python] Issues re: Python API component: core Issues re: The core compiler component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths component: lowering Issues re: The lowering / preprocessing passes component: runtime component: tests Issues re: Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants