[MLAS] Route ARM64 S8U8 QGEMM to the UDOT kernel - #29787
Conversation
ARM64 quantized GEMM never routed the signed-A/unsigned-B (S8U8) combination to a SIMD dispatch. x86_64 (AVX2/VNNI), LoongArch64 (LSX), and RISC-V64 all have a GemmS8U8Dispatch, but ARM64's dispatch selector fell through to the generic scalar reference kernel for this combination, and MLAS_PLATFORM did not even declare the member. Reuse the existing UDOT kernel (already shared by U8U8 and U8S8) instead of adding new assembly, by mirroring the sign-flip trick already used for B onto A: XOR 0x80 on real A data bytes only, leaving K-padding untouched so it keeps contributing zero to the dot product and RowSumBuffer. MlasGemmQuantFixupZeroPointA gets the matching specialization so the zero-point correction stays consistent. Also guard bench_qgemm.cpp against calling MlasGemmPackB when MlasGemmPackBSize returns 0 (packing unsupported for the active dispatch) instead of dereferencing a null CopyPackBRoutine, and add benchmark cases for the S8U8 combination.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Followed up on whether the unconditional Methodology: swapped Median ratio (after / before) by thread count:
At 1 and 4 threads (properly subscribed), the difference is within measurement noise in both directions — no regression. At 16 threads the numbers swing widely, but that's oversubscription scheduling noise rather than a code effect: the same binary against itself across the 3 rounds shows the same order of swing (e.g. Makes sense given what actually changed: a single pipelined NEON XOR against an all-zero constant, folded into an already memory-bound copy loop. |
|
@microsoft-github-policy-service agree |
Review: PR #29787 — [MLAS] Route ARM64 S8U8 QGEMM to the UDOT kernelAuthor: Verdict: LGTM — approve. Small, well-scoped, mathematically sound, and thoroughly benchmarked. No blockers. What it doesAdds
Correctness — the signed→unsigned shift mathUDOT is unsigned × unsigned. The standard "flip bit 7" trick converts a signed byte
Full quantized-GEMM identity (dropping i,j indices, with Substituting and the correction step (using shifted values consistently) evaluates to The The Blast radius on the existing U8U8 / U8S8 pathsThose two combinations also flow through const uint8_t AByteFlip = AIsSigned ? 0x80 : 0;
const uint8x16_t AFlipVector = vdupq_n_u8(AByteFlip);
...
uint32x4_t v0 = veorq_u32(vld1q_u32(...), AFlipVector32);For The author addressed this explicitly in the PR conversation: swapped the pre-PR and PR-PR versions of Dispatch selection changeThe qgemm.h diff collapses: if (BIsSigned) {
GemmQuantDispatch = AIsSigned ? GemmS8S8Dispatch : GemmU8S8Dispatch;
} else if (!AIsSigned) {
GemmQuantDispatch = GemmU8U8Dispatch;
}into: if (BIsSigned) {
GemmQuantDispatch = AIsSigned ? GemmS8S8Dispatch : GemmU8S8Dispatch;
} else {
GemmQuantDispatch = AIsSigned ? GemmS8U8Dispatch : GemmU8U8Dispatch;
}Semantically equivalent for the pre-existing three combinations, and previously S8U8 was falling through to Benchmark guardif (packed_b_size == 0) {
state.SkipWithError("Packing is not supported for this A/B signedness combination on this target");
return;
}Small but valuable — before, Scope observations (not blockers)
Nits
CIPR page shows Bottom lineApprove. The math is sound and symmetric to the existing signed-B UDOT handling, the perf regression on the unsigned-A paths has been empirically ruled out on the two most relevant ARM64 microarchitectures, the no-dot-product path is correctly preserved, and the benchmark null-deref fix is a nice bonus. Merge when Microsoft-side signoff arrives. |
There was a problem hiding this comment.
Pull request overview
This PR improves ARM64 MLAS quantized GEMM performance by enabling SIMD dispatch for the A=int8 / B=uint8 (S8U8) case, routing it through the existing UDOT kernel path instead of falling back to the scalar reference implementation. It also expands the MLAS QGEMM benchmark coverage for signedness combinations and prevents a packed-B benchmark crash when packing is unsupported by the active dispatch.
Changes:
- Add a new ARM64 platform dispatch slot for S8U8 QGEMM and route ARM64 S8U8 to the UDOT dispatch when dot-product is available.
- Extend the UDOT QGEMM kernel’s A-side fixup/packing to correctly handle signed A by XOR sign-bit flipping and matching zero-point adjustment.
- Update
bench_qgemmto parameterize B signedness, add S8U8 coverage, and guard packed-B benchmarks when packing is unsupported.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| onnxruntime/test/mlas/bench/bench_qgemm.cpp | Parameterizes QGEMM benchmark by A/B signedness; adds S8U8 cases and a packed-B unsupported guard. |
| onnxruntime/core/mlas/lib/qgemm.h | Updates ARM64 dispatch selection to use the new S8U8 dispatch for AIsSigned && !BIsSigned. |
| onnxruntime/core/mlas/lib/qgemm_kernel_udot.cpp | Adds signed-A handling for UDOT via A zero-point fixup and XOR sign-bit flipping during A packing while preserving padding semantics. |
| onnxruntime/core/mlas/lib/platform.cpp | Initializes and conditionally overrides ARM64 GemmS8U8Dispatch to UDOT under the existing dot-product capability check. |
| onnxruntime/core/mlas/lib/mlasi.h | Adds GemmS8U8Dispatch to the ARM64 MLAS_PLATFORM layout. |
SkipWithError marks the benchmark run as errored, but a target without packed-B support for this signedness combination is an expected condition, not an error. bench_qnbitgemm.cpp and bench_lutgemm.cpp use SkipWithMessage for the same kind of "not available on this configuration" skip.
The generic buffer fill in MatrixGuardBuffer::GetBuffer() only produces bytes in [21,63], so existing S8U8 tests never exercised the A-side XOR 0x80 sign flip at the int8 extremes (-128/-1/0/127) the UDOT kernel actually handles. Add a focused test that pins those values across the K-padding and M row-block boundaries.
|
Pushed 12e6e89 for the S8U8 test coverage point — turned out MatrixGuardBuffer's generic fill only ever produces bytes in [21,63], so the existing S8U8 tests never actually put A at the signed extremes (-128/-1/0/127) the XOR 0x80 flip has to handle. This pins those values across the K-padding and M row-block boundaries, 840 cases, all passing alongside the existing suite. Also added a note to the description about native Windows ARM64 picking up the same routing via the MLAS_TARGET_ARM64 guard, and fixed the SkipWithError/SkipWithMessage nit Copilot caught on the benchmark guard. |
Re-review: PR #29787 — [MLAS] Route ARM64 S8U8 QGEMM to the UDOT kernel (head
|
Description
ARM64 quantized GEMM never routed the signed-A/unsigned-B (S8U8) combination to a SIMD dispatch —
MLAS_PLATFORMdidn't even declare aGemmS8U8Dispatchmember on ARM64, unlike x86_64 (AVX2/VNNI), LoongArch64 (LSX), and RISC-V64, which all have one. It fell through to the generic scalar reference kernel.This reuses the existing UDOT kernel (already shared by U8U8 and U8S8) instead of adding new assembly. UDOT only does unsigned x unsigned, so B is already shifted into the unsigned domain with an XOR 0x80 sign flip when signed; this does the same for A (
MlasGemmQuantFixupZeroPointA,MlasGemmQuantCopyPackA). K-padding bytes are left untouched across all four row-count paths (8/4/2/1-row) so they keep contributing zero to the dot product andRowSumBuffer.platform.cppinitializes the new dispatch member to the scalar default and only overrides it to UDOT inside the existing dot-product-support check, so cores without dot-product stay on the scalar path exactly as before.Also parameterized
bench_qgemm.cppto cover S8U8, and added a guard around the packed-B benchmark:MlasGemmPackBSizereturns 0 for combinations the active dispatch doesn't support packing for, and callingMlasGemmPackBanyway was dereferencing a nullCopyPackBRoutine.Motivation and Context
QLinearMatMul,MatMulInteger, andQLinearConvregister their signed-A kernels withT2accepting bothint8anduint8, so S8U8 is reachable from real models, not just a synthetic case. Existingtest_qgemm.cppcorrectness tests already coverint8_t/uint8_t, they were just exercising the scalar fallback on ARM64.Tested on Apple M1 (macOS) and Ampere Neoverse-N1 (Ubuntu), both with NEON dot-product:
bench_qgemm(NoPackB, real_time) before/after by temporarily forcing the scalar dispatch for comparison:Packed-B for S8U8 works correctly on both machines now that they're on the UDOT dispatch. Without the guard mentioned above, the new packed-B benchmark case would crash on any target that still resolves to the scalar dispatch, since
MlasGemmPackBSizereturns 0 there andMlasGemmPackBwould call a nullCopyPackBRoutine.ARM64 cores without dot-product support are unaffected by the dispatch change — they stay on
MlasGemmQuantDispatchDefault(scalar) exactly as before,MlasGemmPackBSizestill returns 0 for S8U8 there, and the benchmark guard keeps that case skipping cleanly instead of crashing. This PR doesn't add an optimized S8U8 path or packed-B support for those cores; that's left for a follow-up if there's interest.The routing change in
qgemm.hlives inside the#if defined(MLAS_TARGET_ARM64)guard, so native Windows ARM64 (MSVC,MLAS_TARGET_ARM64) picks up the same S8U8 → UDOT routing as macOS/Linux ARM64 — it wasn't separately benchmarked here. ARM64EC (MLAS_TARGET_ARM64EC) and 32-bit ARM stay on their existing logic, unaffected.