[MLAS] Add a NEON fused kernel for LinearAttention - #32178
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an ARM64 NEON fused kernel for LinearAttention to improve recurrence performance.
Changes:
- Implements fused single- and two-pass NEON kernels.
- Adds ARM64 dispatch registration and declarations.
- Includes the kernel in Windows and non-Windows ARM64 builds.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
onnxruntime/core/mlas/lib/platform.cpp |
Selects the NEON dispatch on ARM64. |
onnxruntime/core/mlas/lib/mlasi.h |
Declares the NEON dispatch. |
onnxruntime/core/mlas/lib/linear_attention_kernel_neon.cpp |
Implements the fused NEON kernel and fallback logic. |
cmake/onnxruntime_mlas.cmake |
Adds the kernel to ARM64 builds. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Review — PR #32178: [MLAS] Add a NEON fused kernel for LinearAttention Scope Five-file PR that adds an ARM64 NEON peer to the existing portable and AVX-512 LinearAttention kernels. New TU linear_attention_kernel_neon.cpp (~447 lines), plus the standard trio of scaffolding: declaration in mlasi.h, assignment in platform.cpp, and source additions in onnxruntime_mlas.cmake (both the Windows and non-Windows ARM64 branches). Author reports 3x speedup vs generic dispatch. The kernel is a NEON-specific rewrite, not a port of the AVX-512 one: it takes the single-pass form where the algebra allows it, which is the right call on 128-bit vectors even though the AVX-512 kernel uses two-pass uniformly. Everything below is verifiable from the diff alone. Two-shape design
Author's write-up in the module-header comment is accurate. Working the math for Panel geometry and register budget
Two-pass live set at NLANE=8:
Well within the 32 aarch64 SIMD registers, with 16 independent FMA chains to cover FMLA latency. This is exactly why
The scalar-broadcast FMA replaces AVX-512's embedded-broadcast set1-from-memory. Author's note "At 8 lanes each scalar is amortized over 8 FMAs, so loading weights four at a time for Shape envelope (fallback logic) const bool shape_ok = (d_k % 4 == 0) &&
(d_k <= MlasLinearAttentionNeonMaxKHeadSize) &&
(d_v % MlasLinearAttentionNeonPanelWidth == 0);
if (!shape_ok || Work->HeadsPerGroup != 1) {
MlasLinearAttentionProcessHead(Work);
return;
}The d_k bound is the 256-float staging buffer for the two-pass form ( q.k dot product
The Decay materialisation strategy Per-key-dim decay routes through Small nit: the per-head splat writes the same value Reassociation callout
Important and correct. Panel-wise vector accumulation reorders sums vs the portable per-element loop. The existing Test expansion in test_linear_attention.cpp Three new shapes with a specific test-selection rationale in the comment:
Author's comment explicitly says "The last four entries exist for specific edges, so do not drop them without checking what they cover" — reviewer-friendly future-proofing. Coverage is thoughtful. The Concerns and nits
Non-issues I verified
Recommendation Approve. High-quality NEON kernel with the right trade-offs called out and defended in code comments:
Suggested pre-merge tightening (all non-blocking):
|
|
Re-review — PR #32178: [MLAS] Add a NEON fused kernel for LinearAttention One new commit since the previous review: Nit 1 (rule switch Previous form used switch (Work->Rule) {
case MlasLinearAttentionRuleLinear: ProcessHeadNeon<false, false>(Work); return;
case MlasLinearAttentionRuleGated: ProcessHeadNeon<true, false>(Work); return;
case MlasLinearAttentionRuleDelta: ProcessHeadNeon<false, true >(Work); return;
case MlasLinearAttentionRuleGatedDelta: ProcessHeadNeon<true, true >(Work); return;
}
//
// Deliberately no default label above: -Wswitch turns a newly added rule
// into a compile error here rather than silently routing it to one of the
// existing specializations. A value outside the enum can still arrive at
// runtime, so defer to the portable kernel rather than guess at its
// semantics.
//
MlasLinearAttentionProcessHead(Work);This is strictly safer than the
The docstring comment explains why the missing One follow-up worth verifying at merge time: ORT's MSVC build config should have Nit 6 (positional initializer) — addressed const MLAS_LINEAR_ATTENTION_DISPATCH MlasLinearAttentionDispatchNeon = {
.ProcessHead = MlasLinearAttentionProcessHeadNeon
};Designated initializer as suggested. If the dispatch struct grows a second field, this stays a valid initialization with the new field default-initialized, and reviewers can see at a glance which slot is being wired. Same treatment should be applied to Nits still open
Non-issues re-verified against the new diff Kernel body is identical to the prior review (single-pass / two-pass split, Recommendation Approve. Both concrete nits from the prior review are addressed, and the switch fix is strictly better than what I proposed. Remaining suggestions (perf breakdown, decay-per-head optimization, applying the designated-initializer style to the AVX-512 / Default dispatches) are all non-blocking follow-ups. |
Description
Fused NEON kernel for LinearAttention
Motivation and Context
3x speedup compared to generic dispatch