Auto-detect fixed kv-cache shape in DefaultKeyValueCache - #2166
Merged
kunal-vaishnavi merged 4 commits intoMay 21, 2026
Merged
Conversation
Some compiled backends (e.g. AMD RyzenAI) emit decoder models where the past_key/past_value ONNX inputs declare a static positive integer in the seq_len dimension instead of a symbolic dim. Such models require the kv-cache to be allocated to that exact size and reused as a shared past/present buffer; max_length cannot drive the size because ORT rejects any tensor whose shape doesn't match the model's static dim. Inspect past_key shapes per layer in DefaultKeyValueCache's constructor. When all layers agree on a positive seq_len, treat the model as fixed-shape: force past_present_share_buffer_=true, allocate the cache to the detected size, reject beam search, and warn if search.max_length exceeds the cache capacity. Behavior is unchanged for models with symbolic kv-cache dims (detection does not fire). Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
akholodnamdcom
force-pushed
the
auto-detect-fixed-kv-cache-shape
branch
from
May 18, 2026 22:31
5175754 to
bc1c850
Compare
akholodnamdcom
marked this pull request as ready for review
May 18, 2026 22:36
Contributor
Author
|
@baijumeswani, Does this one look like what we agreed on? Thanks! |
4 tasks
Address review feedback: the auto-detection only recognises models where every past_key layer declares the same fixed seq_len. Add a comment noting the restriction and outlining how to extend layer_shapes_ to support per-layer static seq_lens when a model in the wild needs it. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Address review feedback: pull the auto-detection and its consequence handling (beam-search reject, force share-buffer, info log, warn-once on max_length mismatch) out of DefaultKeyValueCache's constructor into a file-local helper DetectAndConfigureFixedKvShape in an anonymous namespace. The constructor now calls the helper once and uses its return value (the detected static seq_len, or 0) in the share-buffer branch. No behavior change: pure refactor verified against both the fixed-shape patched model (warning fires once, generation succeeds against the 128-slot cache) and the dynamic-shape Llama (detection short-circuits, no warning, normal run). Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Address review feedback: - Add the standard 'g_log.enabled && g_log.warning' guard around the warning Log call, matching the pattern already used at the past_present_share_buffer warning a few lines above. The bare Log() asserts on g_log.enabled in debug builds. - Drop std::once_flag/std::call_once (and the now-unused <mutex> include). Per-process dedup is wrong for multi-model hosts (e.g. Foundry Local), where every distinct model loaded into the process should be able to emit its own warning. Callers that re-run generation against the same model should use Generator::RewindTo() rather than constructing a new Generator (and model_benchmark exposes that as --reuse_generator), so dedup machinery for repeated constructions isn't justified. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
baijumeswani
approved these changes
May 20, 2026
kunal-vaishnavi
approved these changes
May 21, 2026
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
Some compiled backends (e.g. AMD RyzenAI, and likely future NPU/accelerator targets) emit decoder models where the
past_key_values.N.{key,value}ONNX inputs declare a static positive integer in theseq_lendimension instead of the usual symbolicpast_sequence_lengthparameter. Such models require the kv-cache tensor to be allocated to exactly that size and reused as a shared past/present buffer — ORT will reject any cache tensor whose shape doesn't match the model's static dim, and the runtime cannot pick a size fromsearch.max_lengthafter the fact.This PR makes OGA notice the static dim at cache construction and size the kv-cache accordingly, with no config schema change, no public API change, and no client cooperation required.
How it works
In
DefaultKeyValueCache's constructor, right after the existing per-layerhead_dimauto-detection (which already readsSessionInfo::GetInputShape), walk each layer'spast_key.Ninput and look at the second-to-last dim:fixed_kv_seq_len = that value.<= 0(symbolic, typically-1) or values disagree → leave detection off; behavior unchanged.When detection fires:
past_present_share_buffer_is forcedtrue(model demands it; the existing share-buffer code path already does the right thing for a pre-sized buffer reused as both past and present).fixed_kv_seq_len, notsearch.max_length. A new localcache_seq_len = fixed_kv_seq_len > 0 ? fixed_kv_seq_len : search.max_lengthreplaces the direct read ofsearch.max_lengthin the share-buffer branch, so dynamic-shape models are byte-equivalent to before.num_beams != 1) is rejected with a clearstd::runtime_error— beam picking reshuffles past tensors, which is incompatible with a model-mandated fixed shape.infolog records the detected size (gated byg_log.enabled, consistent with surrounding logs).search.max_length > fixed_kv_seq_len, awarningis emitted once per process (std::call_once) noting the cache is sized to the model's limit. We do not clampsearch.max_lengthbecausestate_.params_isstd::shared_ptr<const GeneratorParams>and is owned by the caller; clamping would require a public API change.Files changed
src/models/kv_cache.cpp—+67/-2. One detection block inDefaultKeyValueCache::DefaultKeyValueCache, oneshape_[2]source change in the share-buffer branch, and#include <mutex>forstd::call_once.No header changes. No public API changes. No config schema changes. No changes to
Combined/Windowed/LFM2/Cross/ModelManagedcache classes (out of scope for this PR — AMD RyzenAI decoder-only models route throughDefaultKeyValueCache).Reused infrastructure
SessionInfo::GetInputShape(name)—src/models/model.cpp:533-538. Same call already used atkv_cache.cpp:217forhead_dimauto-detection. Returns the ONNX-declared shape with<= 0for symbolic dims (confirmed by sibling checks insrc/models/multi_modal.cppandsrc/models/recurrent_state.cpp).Log("warning"|"info", ...)—src/logging.h:82. Same pattern as the existingLogcalls inkv_cache.cpp(lines 156, 201, 239).kv_cache.cpp:301-310(shape),:357-361(Add: inputs = presents),:366-367(Update no-op),:409-410(RewindTo no-op). All already correctly handle a pre-sized buffer.Verification
Tested end-to-end on Windows x64, RelWithDebInfo, CPU EP, with
Llama-3.2-1B-Instruct(Q4F16). Since no existing test model undertest/test_models/declares a static kv-cacheseq_len, I produced a patched copy of the Llama model whose past_key/value (and present.key/value) ONNX shapes were rewritten in place — only graph metadata, no weight reload — from symbolicpast_sequence_lengthto a staticdim_value = 128. With those static dims in place, ORT will reject any cache tensor that isn't exactly[1, 8, 128, 64]; bound-and-run success is therefore proof that the cache was sized correctly.-mlflagsearch.max_length[1,8,128,64])-1(use config)4096(JSON)[1,8,4096,64]→ bind failure.1024(runtime override)1024SetSearchOptionoverrides ofmax_lengthare ignored for cache sizing.-1→131072131072info, normal run. Regression preserved.-w 1 -r 3→ 4 cache constructions)-14096std::call_once).Verbatim warning emitted in test 1:
Backward compatibility
For every model that ships today with symbolic kv-cache dims (i.e. essentially all decoder models built via the OGA model builder or the Microsoft ORT pipelines),
seq_dim <= 0at every layer → detection short-circuits → cache construction is byte-identical tomain. No behavior change is exposed to existing users.