Skip to content

perf(embedding): use a gather-optimal device layout for nn.Embedding tables - #291

Open
ani300 wants to merge 1 commit into
torch-spyre:mainfrom
ani300:perf/embedding-gather-layout
Open

perf(embedding): use a gather-optimal device layout for nn.Embedding tables#291
ani300 wants to merge 1 commit into
torch-spyre:mainfrom
ani300:perf/embedding-gather-layout

Conversation

@ani300

@ani300 ani300 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

nn.Embedding weights on Spyre are read as a gather (indexed by token id along the vocab/leading dim), not as a matmul. Today they are merely excluded from the row-major matmul SpyreTensorLayout and left on the default layout — correct, but it leaves the gather unoptimized.

This PR gives each embedding table a gather-optimal "indirect access" device layout instead:

  • vocab dim outermost, and
  • the hidden dim split into BLOCK_SIZE-element sticks,

i.e. device dims [rows, D // BLOCK_SIZE, BLOCK_SIZE], built via the 3-arg device-dims SpyreTensorLayout overload with the device dtype from get_device_dtype() (not the host torch.dtype).

What changed

hf_adapters/hf_common.py:

  • _move_to_spyre_with_layout now chooses a device layout per weight: a gather-optimal layout for embedding tables, the row-major layout for 2-D matmul weights, and the default layout for everything else.
  • New nested helper _embedding_layout(t) builds the indirect-access layout for a table.
  • _embedding_param_ids and the _alloc_on_spyre branch/comments updated to reflect that embedding tables now get an active layout rather than being skipped.

Fallback behavior

When the hidden dim D is not a multiple of BLOCK_SIZE, the sticks can't tile the hidden dim. In that case we warnings.warn and fall back to the default layout (None) — the table still loads and runs correctly, just without the gather optimization.

Scope

Single self-contained change to the layout-selection logic. No API changes; models that previously loaded still load.

Testing

  • Load + e2e token-compare on the Spyre pod (tests/spyre/test_e2e_token_compare_spyre.py) — verify layout applies and outputs match reference.

…tables

nn.Embedding weights are read as a gather (indexed by token id along the
vocab/leading dim), not as a matmul. Previously they were merely excluded
from the row-major matmul SpyreTensorLayout and left on the default layout.

This instead gives each embedding table a gather-optimal "indirect access"
layout: vocab dim outermost and the hidden dim split into BLOCK_SIZE-element
sticks (device dims [rows, D // BLOCK_SIZE, BLOCK_SIZE]), built via the
3-arg device-dims SpyreTensorLayout overload with the device dtype from
get_device_dtype().

When the hidden dim is not a multiple of BLOCK_SIZE the sticks can't tile it,
so we warn and fall back to the default layout — the table still loads and
runs, just without the gather optimization.

Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
@arielge

arielge commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Thanks @ani300, how would this sync with #280, which basically plans to remove all hf-adapter-side layouting code? Can the embedding layouting sit in torch-spyre as well, or would we need to keep the hf-adapter-side layouting code for the embeddings specifically (cc @vinithakv)

@ani300

ani300 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

I can port this to the torch-spyre adapter instead, no problem. I'll move the PR there.

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.

2 participants