[leon7609]

PR #3: deepseek4: stabilize MTP decode path

Branch suggestion: fix/deepseek4-mtp-decode-stabilize
Target: ggml-org/llama.cpp master — most changes are in shared files (src/llama-context.cpp, common/speculative.cpp) that exist upstream. Some sub-changes are arch-gated to LLM_ARCH_DEEPSEEK4_MTP_SUPPORT so they're inert unless DSv4 MTP support is loaded — that gating is what makes this submittable independently of DSv4 support landing.
Commit: f01ad092d (43 lines across 2 files)

Description

Follow-up to the MTP sidecar work originally in nisparks's DSv4 MTP integration (4a310073c snapshot). That snapshot ran model load + sidecar attach + speculative context init successfully but crashed on the first inference request with three independent bugs in the MTP-enabled decode path. This PR fixes all three.

Bug 1: XOR assert in llama_context::decode() is too strict for MTP sidecar batches

The existing precondition

GGML_ASSERT((!batch_inp.token && batch_inp.embd) || (batch_inp.token && !batch_inp.embd))

requires a batch to provide only tokens or only embeddings, never both. DeepSeek V4 Flash's MTP sidecar graph legitimately needs both — conditioning tokens at one position plus the hidden state row from the target's last decode. The recursive llama_decode(ctx_mtp, ...) call in handle_mtp_for_ubatch() hits this assert on the first decode.

Fix: relax the XOR to permit token && embd only when the loaded arch is LLM_ARCH_DEEPSEEK4_MTP_SUPPORT. Other models retain the original constraint.

Bug 2: common_speculative_state_mtp writes batch.token[0] without allocating it

llama_batch_init(..., embd=n_embd, ...) allocates only the embedding buffer; the speculative state then writes to batch.token[0], dereferencing a NULL or unallocated pointer.

Fix: explicitly allocate batch.token alongside batch.embd.

Bug 3: hidden-row offset reads use wrong tensor stride

handle_mtp_for_ubatch() and the MTP-draft hidden-copy path computed row offsets as row * n_embd * sizeof(float), which assumes a tightly packed F32 tensor with stride n_embd. Real ggml_tensor strides come from tensor->nb[1]; with non-default layouts the wrong offset reads out-of-bounds or stale data. After partial accept + checkpoint restore, last_n_accepted=1 can also exceed the target graph's actual visible row count.

Fix: read row offsets via tensor->nb[1]; clamp last_n_accepted to the actual graph row count; add type/shape/bounds guards that stop the current draft pass rather than crash the server if the source tensor is malformed.

Reproduction (before fix)

./llama-server \
  -m DeepSeek-V4-Flash-IQ2XXS-...-chat-v2.gguf \
  --spec-type mtp \
  --spec-draft-model DeepSeek-V4-Flash-MTP-Q4K-Q8_0-F32.gguf \
  --spec-draft-ngl all \
  --spec-draft-ctx-size 131072 \
  --spec-draft-n-max 8 --spec-draft-n-min 1 \
  -c 131072 -np 1 -ngl all -fa on \
  --no-warmup --cont-batching --jinja

# Container starts, /health returns 200.
# First chat completion request → GGML_ASSERT crash, exit 139:
#
#   GGML_ASSERT((!batch_inp.token && batch_inp.embd)
#               || (batch_inp.token && !batch_inp.embd)) failed
#     at src/llama-context.cpp:1566
#
#   call stack:
#     llama_decode
#     llama_context::handle_mtp_for_ubatch
#     llama_decode (recursive)
#     llama_context::decode ← assert here

After fix

Same command; first request completes with valid output. Speculative decoding emits draft tokens and target-side verification finishes cleanly. Probes 7/10 pass (same set as the no-MTP same build — the 3 failing probes are pre-existing nisparks-tree issues unrelated to this fix). MTP-enabled decode now reaches 11-12 tok/s in the naïve configuration — see fix/deepseek4-mtp-adaptive-fallback (separate PR) for the perf work needed to keep MTP from regressing throughput vs no-MTP.

Hardware tested

• NVIDIA RTX PRO 6000 Blackwell Workstation Edition (sm_120, 96 GB VRAM)
• CUDA 13.0, driver 595.71.05
• Build: same as companion PRs

Why upstream

This fixes an actual crash in any DSv4 MTP-enabled decode path. The XOR relaxation is arch-gated to LLM_ARCH_DEEPSEEK4_MTP_SUPPORT so it's a no-op for other models. The batch.token allocation and tensor-stride fixes are general correctness improvements on the speculative state and hidden-state read paths — also safe to land independently.

The relaxation precedent is important: future hybrid-architecture sidecars (any model that needs simultaneous conditioning tokens + hidden embeddings for a recursive decode) will need the same hook.

Related

• nisparks PR Wip/deepseek v4 support ggml-org/llama.cpp#22378 (closed) — original DSv4 support
• Snapshot commit 4a310073c "wip: mtp patch snapshot" — the WIP that this PR fixes
• Companion: fix/deepseek4-mtp-adaptive-fallback (perf work; this PR is correctness only)