Skip to content

llama : split a tied output projection under split mode tensor - #65

Open
Piggidragon wants to merge 2 commits into
GenerelSchwerz:llama/devfrom
Piggidragon:mgpu/tied-output-split
Open

llama : split a tied output projection under split mode tensor#65
Piggidragon wants to merge 2 commits into
GenerelSchwerz:llama/devfrom
Piggidragon:mgpu/tied-output-split

Conversation

@Piggidragon

Copy link
Copy Markdown

Overview

A model with tied embeddings has no output.weight. Its output projection reuses the embedding
table under the table's own name, and that copy is what reaches a meta buffer under
--split-mode tensor. The split-state callback matches on the name, pattern_output_weight only
matched output\.weight, so the copy fell through to MIRRORED: every device held the whole table
and ran the whole projection instead of its share.

The table itself stays on the input device and never reaches the meta device, so matching
token_embd.weight here is unambiguous.

Split out of #57.

Testing

llama-bench -ngl 99 -ts 50/50 -r 3 -sm tensor, gemma-4-26B-A4B (tied embeddings), two GPUs,
RTX 4070 + RTX 3060, stock clocks:

pp512 tg128
before 1687.72 +/- 8.46 85.86 +/- 0.27
after 1698.01 +/- 7.67 92.64 +/- 0.02

+7.9% generation. The output projection runs once per token, so prefill is unaffected, as expected.

test-llama-archs -s 1 at 1, 2, 3 and 4 virtual CUDA devices: passes. Built with
-DLLAMA_FATAL_WARNINGS=ON.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES - implemented by an agent on my instruction, see the Assisted-by: commit trailer.

@GenerelSchwerz

Copy link
Copy Markdown
Owner

Automated preliminary review by Codex; the repository owner plans a separate manual review.

Verdict: CONDITIONAL PASS.

Blocking

  • None. The changed match at src/llama-model.cpp:418, together with the existing output branch at src/llama-model.cpp:580-584, cleanly gives tied token_embd.weight the same axis-1 split as untied output.weight while retaining the DeepSeek-v4 mirrored exception. No correctness, security, performance, scope, or pristine-upstream blocker was found.

Will slow review

  • src/llama-model.cpp:418: There is no direct automated coverage for the case where output.weight is omitted and the duplicated token_embd.weight fallback is selected. Add coverage for that tied-output tensor-split path.

Nits

  • src/llama-model.cpp:415-417: The explanatory comment is split mid-sentence, contrary to the repository style for prose comments.

Developmental progress

  • The change is now isolated to src/llama-model.cpp at +4/-1. The earlier broad-scope concerns are fixed.

Reviewed head: 2471f26

A model with tied embeddings has no output.weight - the output projection reuses
the embedding table under the table's own name. That copy reaches a meta buffer,
missed pattern_output_weight and fell through to MIRRORED, so every device held
the whole table and ran the whole projection.

Assisted-by: Claude Opus 5
Also shorten the comment that explains the pattern.

Assisted-by: Claude Opus 5
@Piggidragon
Piggidragon force-pushed the mgpu/tied-output-split branch from 48cb614 to 2ea2944 Compare September 6, 2026 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants