Skip to content

[ROCm][RDNA][Perf] Fix MiniCPM-V TTFT regression from always-on ViT attention mask - #1398

Merged
stoivonen-amd merged 1 commit into
gfx11from
stephant/idefics2-skip-encoder-mask-when-unpadded
Oct 28, 2026
Merged

stoivonen-amd merged 1 commit into
gfx11from
stephant/idefics2-skip-encoder-mask-when-unpadded

Conversation

@stoivonen-amd

@stoivonen-amd stoivonen-amd commented Oct 23, 2026 •

Copy link
Copy Markdown

Summary

Fixes a ~8–9 ms TTFT regression for MiniCPM-V on gfx1151 (+8–15% on the 0.9B/4B models).

vllm-project#42785 removed the check in Idefics2VisionTransformer.forward that skipped the ViT attention mask when no patch is padded. This PR restores that behavior in the MiniCPM-V caller, matching HF NaViT SigLIP, which skips the mask in the same case. The shared Idefics2 encoder is unchanged.

Details

Cause. Any mask forces MMEncoderAttention off its fused backend (e.g. TRITON_ATTN) onto SDPA with padding. With the SigLIP-So400m encoder, an eager profile shows 27× SDPA + 27× constant_pad_nd per request instead of 27× triton_attn_wrapper before vllm#42785.

Fix. The eager get_vision_hidden_states of MiniCPM-V 2.5 / 2.6 / 4.0 / 4.5 now share _mcpmv_patch_attention_mask. It returns None when every slice has the same patch count, so the encoder skips the attention mask. tgt_sizes is kept on CPU, so the check needs no device sync. Padded batches still get the mask.

CUDA graphs. The encoder CUDA graph path (encoder_cudagraph_forward) is untouched and always passes the mask, so the captured graph is unchanged.

Test

gfx1151 (ROCm), BF16, --mm-encoder-attn-backend TRITON_ATTN, greedy decoding. Models: MiniCPM-V with the SigLIP-So400m encoder (internal 0.9B / 4B checkpoints, openbmb/MiniCPM-V-2_6). Measured by applying this diff to a recent nightly ROCm wheel (dd50dea38).

Performance (0.9B, one 560x315 image, 3 runs each, run back to back)

With vllm#42785 (nightly) This PR
Mean TTFT 72.4 ms (72.8 / 71.6 / 72.9) 64.1 ms (65.8 / 64.8 / 61.6)

In an earlier session on the same board, a wheel from before vllm#42785 measured 63.4 ms against 69.5 ms for the nightly, so this PR recovers the full regression. Decode is unchanged at ~224 tok/s.

Correctness (greedy tokens vs the nightly, MiniCPM-V-2.6 and 4B)

  • Identical with the encoder CUDA graph on, for all three inputs (560x315, 1024x800, two images). The mask is still built in 24 / 24 capture calls.
  • Identical in eager mode with padded inputs (1024x800, two images). The mask is still applied.
  • Slightly different in eager mode with the unpadded 560x315 input, where the mask is now skipped. The mask was all zeros, so the computation is mathematically the same; the difference is numerical noise from switching attention kernels. This path is also not bit-reproducible run to run: repeated MiniCPM-V-2.6 runs alternate between two outputs. On main, graph and eager runs already diverge by a similar amount.

Not run: test_vit_cudagraph.py -k minicpmv. It only asserts non-empty output, which the graph-mode token comparison above already covers more strictly.

Notes

  • Not a duplicate: no open or merged PR in vllm-project/vllm or ROCm/vllm restores this check.
  • AI assistance (Claude Code) was used for this change; I reviewed every changed line and ran the tests above.

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

🤖 Generated with Claude Code

@stoivonen-amd
stoivonen-amd force-pushed the stephant/idefics2-skip-encoder-mask-when-unpadded branch from b207642 to 1729809 Compare October 26, 2026 18:33
@stoivonen-amd stoivonen-amd changed the title [ROCm][MiniCPM-V] Skip the ViT attention mask when no patch is padded [ROCm][RDNA][Perf] Fix MiniCPM-V TTFT regression from always-on ViT attention mask Oct 26, 2026
@stoivonen-amd
stoivonen-amd marked this pull request as ready for review October 26, 2026 18:48
…ttention mask

Skip the ViT attention mask in the eager MiniCPM-V encoder path when no
slice is padded. vllm-project#42785 removed the equivalent check
from Idefics2VisionTransformer.forward, adding ~8-9 ms of TTFT to small
MiniCPM-V models on gfx1151. HF NaViT SigLIP skips the mask in the same
case.

Cause: since vllm-project#42785 (encoder CUDA graph for MiniCPM-V) a mask is built for
every request. Any mask takes MMEncoderAttention off its fused backend
(e.g. --mm-encoder-attn-backend TRITON_ATTN) and onto torch SDPA with a
padded additive mask (27 SDPA + 27 constant_pad_nd calls per request for
the So400m tower).

Fix: the eager get_vision_hidden_states of MiniCPM-V 2.5/2.6/4.0/4.5 now
share _mcpmv_patch_attention_mask, which returns None when every slice
has the same patch count. tgt_sizes is kept on CPU, so the check needs no
device sync, and the shared Idefics2 encoder is unchanged. The encoder
CUDA graph path still always passes the mask. Padded batches still get
the mask.

Impact on gfx1151: TTFT of the Internal MiniCPM 0.9B/4B resized VLM rows
regressed by ~8-9 ms (e.g. 0.9B BF16: 59.8 -> 69.0 ms, +15%), independent
of LLM size; decode is unchanged. With this change, 0.9B BF16 on the same
wheel goes from 72.4 to 64.1 ms (3 runs each).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Signed-off-by: Stephan Toivonen <Stephan.Toivonen@amd.com>
@stoivonen-amd
stoivonen-amd force-pushed the stephant/idefics2-skip-encoder-mask-when-unpadded branch from 1729809 to 673ce84 Compare October 27, 2026 15:57
@stoivonen-amd
stoivonen-amd merged commit 49c3a2e into gfx11 Oct 28, 2026
12 checks passed
@stoivonen-amd
stoivonen-amd deleted the stephant/idefics2-skip-encoder-mask-when-unpadded branch October 28, 2026 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants