Skip to content

rfcs : Added Embedding Bag RFC#5643

Open
sshinde-amd wants to merge 1 commit into
uxlfoundation:rfcsfrom
sshinde-amd:zendnn-onednn-embag
Open

rfcs : Added Embedding Bag RFC#5643
sshinde-amd wants to merge 1 commit into
uxlfoundation:rfcsfrom
sshinde-amd:zendnn-onednn-embag

Conversation

@sshinde-amd

Copy link
Copy Markdown
  • Added embedding bag RFC in rfcs/20260715-embedding-bag.

Description

This RFC adds an Embedding Bag as a public oneDNN primitive dnnl::embedding_bag, backed first by a portable CPU reference implementation. A thin dnnl::embedding wrapper covers the lookup-only (no-reduction) mode.

This primitive closely resembles PyTorch Embedding Bag semantics.

oneDNN currently has no primitive equivalent to PyTorch Embedding Bag. The closest dense neighbous (reduction, graph gather) do not cover indexed lookup + variable-length bag reduction semantics. Framework users that integrate with oneDNN either need to depend on its framework implementation, or assemble multiple ops by hand.

The first PR promotes the primitive plumbing to a public C++ API, adds the CPU reference implementation, and validates the behavior needed by recommendation and NLP workloads: sum / mean / max reduction over variable-length bags, optional per-sample weights, padding_idx skipping, and the include_last_offset flag matching PyTorch semantics. Future optimized implementations (AVX-512 intrinsics) reuse the same API without any framework changes.

https://github.com/amd/oneDNN-Zen/tree/zen-rfcs/rfcs/20260715-embedding-bag

Fixes # (github issue)

Checklist

General

  • Do all unit and benchdnn tests (make test and make test_benchdnn_*) pass locally for each commit?
  • Have you formatted the code using clang-format?

Performance improvements

  • Have you submitted performance data that demonstrates performance improvements?

New features

  • Have you published an RFC for the new feature?
  • Was the RFC approved?
  • Have you added relevant tests?

Bug fixes

  • Have you included information on how to reproduce the issue (either in a github issue or in this PR)?
  • Have you added relevant regression tests?

RFC PR

  • Does RFC document follow the template?
  • Have you added a link to the rendered document?

 - Added embedding bag RFC in rfcs/20260715-embedding-bag.

Signed-off-by: Sudarshan Shinde <Sudarshan.Shinde@amd.com>
@sshinde-amd
sshinde-amd requested a review from a team as a code owner July 21, 2026 07:29
@github-actions github-actions Bot added documentation A request to change/fix/improve the documentation. Codeowner: @oneapi-src/onednn-doc RFC A design document labels Jul 21, 2026

@vpirogov vpirogov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the proposal, @sshinde-amd. Posted a few comments.

+@EikanWang for framework integration perspective.

Comment on lines +30 to +35
Embedding lookups and bag reductions are the dominant CPU operation in
recommendation models (DLRM, DCNv2, MMoE) and token-id-consuming NLP models.
PyTorch `nn.EmbeddingBag`, TensorFlow `embedding_lookup_sparse`, and ONNX
Runtime `EmbedLayerNormalization` each implement this on CPU today — either
through per-framework hand-rolled kernels or FBGEMM — because oneDNN offers no
equivalent primitive.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to include performance data to support this statement.

Comment on lines +50 to +52
Embedding Bag Operator on AMD CPUs] (https://dl.acm.org/doi/10.1109/MM.2024.3423785) )
that embedding bag bandwidth can be improved by 9x over state of the art
implementations using different threading strategies.**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The paper does not show improvement over PyTorch baseline. Is there indication that Pytorch implementation is suboptimal?

Comment on lines +20 to +21
The first PR promotes the primitive plumbing to a public C++ API, adds the CPU
reference implementation, and validates the behavior needed by recommendation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New functionality is normally introduced as opt-in experimental feature first to avoid breaking library API during early adoption stages.

See Experimental Features section of the developer guide.

Comment on lines +32 to +33
PyTorch `nn.EmbeddingBag`, TensorFlow `embedding_lookup_sparse`, and ONNX
Runtime `EmbedLayerNormalization` each implement this on CPU today — either

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparison of the semantics of these operations with semantics of proposed API would be very useful.

Comment on lines +292 to +295
oneDNN primitive uses Xbyak JIT. The embedding kernel logic (gather a row,
multiply by a scalar weight, reduce element-wise) is simple enough that there
is no shape-dependent code-gen opportunity that benefits from runtime
emission. ZenDNN's intrinsic implementation is well-validated today. The

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two other reasons why oneDNN largely relies on Xbyak across platforms:

  • Intrinsics require the compiler to support specific instruction set targeted. This would push minimal required compiler version in particular for newer instruction sets.
  • Intrinsics were not compatible between GCC/Clang and MSVS compilers. Is this not the case anymore?

7. All MDs must be fully described.
8. `dst_desc.dims[1] == src_desc.dims[1]` (embedding dim matches).

## 4. PoC — embedding_bag primitive end-to-end with benchdnn

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "end-to-end" mean here?

Comment on lines +56 to +58
`lowoha::embag` API. This RFC upstreams those intrinsic kernels into oneDNN as a
first-class primitive lifted into `src/cpu/x64/`, with ZenDNN removed from the
runtime path after integration.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivation part only covers reference implementation. Why this paragraph discusses intrinsics and ZenDNN?

Comment on lines +300 to +303
2. **Threading strategy for very long bags.** Phase 1 parallelizes across bags
(`parallel_nd(B, ...)`). For workloads where a single bag is very long
relative to thread count, an optional inner split via `balance211` could
improve scaling. Deferred to Phase 2.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem relevant to the proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation A request to change/fix/improve the documentation. Codeowner: @oneapi-src/onednn-doc RFC A design document

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants