Repository navigation
Fuse DQ->MatMulNBits for shared initializers and skip outer-scope weights - #33800
Vijay Kapse (vijay-kapse) wants to merge 4 commits into
Conversation
…ghts The DQ->MatMulNBits selector accepted weight, scale and zero-point initializers from an enclosing graph, but TransposeDQWeightsForMatMulNBits reads them with Graph::GetInitializedTensor, which only searches the current graph. A DQ->MatMul inside an If/Loop body whose weights live in the outer graph therefore failed session creation with "Missing required weight/scale". This is the actual cause of microsoft#28306 (whisper merged decoders). microsoft#28326 avoided that crash by refusing to fuse whenever the weight or scale initializer had more than one consumer. That also disables the fusion for every flat tied-embedding model (the embedding Gather and the lm_head share weight/scale/zero point), so the full lm_head weight is dequantized on every decode step. Require the initializers to belong to the node's own graph, and drop the sharing guard.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The test captures all three inputs, so weight rejection masks the scale and zero-point scope checks; mixed-scope variants are needed to independently cover those restrictions, including the configuration behind #28306.
1 open issue
What changed in this PR
This PR refines ONNX Runtime’s MatMulNBits fusion to address the outer-scope initializer failure in #28306 while restoring fusion for shared weights previously excluded by #28326.
Changes:
- Require weight, scale, and zero-point initializers to belong to the current graph.
- Allow shared initializers and update MatMul, Gemm, and A16 tests to expect fusion.
- Add an If-branch regression test for outer-scope initializers.
| File | Description |
|---|---|
onnxruntime/test/optimizer/qdq_matmulnbits_transformer_test.cc |
Updates shared-weight fusion expectations and adds outer-scope coverage. |
onnxruntime/test/optimizer/qdq_matmulnbits_a16_test.cc |
Separates nonconstant-weight rejection from shared-weight fusion coverage. |
onnxruntime/core/optimizer/qdq_transformer/selectors_actions/qdq_selectors.cc |
Enforces local initializer lookup and removes the shared-consumer restriction. |
Review effort: Balanced
| NodeArg& weight = graph.GetOrCreateNodeArg(weight_arg->Name(), &uint8_type); | ||
| NodeArg& scale = graph.GetOrCreateNodeArg(scale_arg->Name(), &float_type); | ||
| NodeArg& zp = graph.GetOrCreateNodeArg(zp_arg->Name(), &uint8_type); |
There was a problem hiding this comment.
Good catch, you're right that with all three captured the weight check rejects first and the other two were never exercised. Fixed in c939b1a: the test is now a helper that puts each of weight, scale and zero point either in the branch or in the outer graph, with four cases:
- all local (control, must fuse to MatMulNBits)
- all outer scope
- local weight, outer-scope scale
- local weight and scale, outer-scope zero point (128, so it's nonzero)
The control fusing is what shows the other three are rejected only because of scope. I also checked that they actually guard the fix: with GetConstantInitializer(..., true) restored, the outer-weight and outer-scale cases crash with "Missing required weight/scale" in TransposeDQWeightsForMatMulNBits, and the zero-point case fails as well. With the fix, all four pass, along with the other 273 QDQ/MatMulNBits tests (macOS arm64, Release).
Adds a local-weight/outer-scale case, a local-weight-and-scale/outer-zero-point case, and an all-local control that is fused, so each scope check is exercised on its own.
|
mirounga could you take a look at this one when you have time? It lets DQ->MatMulNBits fuse when the weight is shared (tied lm_head), which was costing 1.5–2x on int8 decode, and it skips weights captured from an outer scope so the merged-decoder crash in #28306 stays fixed. I've just pushed the test cases Copilot asked for. |

Description
DQMatMulNodeGroupSelectoraccepts a DequantizeLinear whose weight, scale and zero point are initializers of an enclosing graph (GetConstantInitializer(name, true)), butTransposeDQWeightsForMatMulNBitsreads them withGraph::GetInitializedTensor, which only searches the current graph. So a DQ -> MatMul inside an If/Loop body whose weights live in the outer graph fails session creation:This is what #28306 hit: in
onnx-community/whisper-tinydecoder_model_merged_uint8.onnxthe embedding scale lives in the main graph and both DQ nodes are inside theoptimum::ifbranches. It still reproduces on 1.31.1 with a single unshared DQ -> MatMul in an If branch (repro below), because the guard added in #28326 only looks at consumer counts.That guard (reject when the weight or scale initializer has more than one consumer) also turns the fusion off for every flat tied-embedding model, where the embedding Gather and the lm_head share weight, scale and zero point. Those models dequantize the whole lm_head weight on every decode step. For
HuggingFaceTB/SmolLM2-135M-Instructmodel_int8.onnxthat is a [576, 49152] uint8 -> fp32 DequantizeLinear per token, 44% of int8 decode time on a Neoverse N2.This change:
GetConstantInitializer(name, false)), matching what the action can read, andTests: the two
*_SharedWeighttests and the A16 shared case now expect fusion (outputs still compared against the unfused graph), andDQMatMulNotConvertedToMatMulNBits_OuterScopeWeightcovers DQ -> MatMul inside If branches with outer-scope weights.Motivation and Context
Decode, ms/token, CPU EP, GitHub
ubuntu-24.04-arm(Azure Cobalt 100 / Neoverse N2, 4 threads), onnx-community / HuggingFaceTBmodel.onnxvsmodel_int8.onnx:Same models on GitHub
ubuntu-24.04(AMD EPYC 7763, 2 threads):Accuracy, against 1.31.1 on the same int8 models:
session.qdq_matmulnbits_accuracy_level(4): lm_head logits differ by at most 1.6% relative; WikiText-2 perplexity 35.277 -> 35.345 (SmolLM2) and 63.199 -> 63.269 (Qwen).session.qdq_matmulnbits_accuracy_level=1: logits are bit-identical to 1.31.1. Note this path is slow for 8-bit weights today, because there is noSQ8BitGemmVariant_CompFp32andMatMulNBits<float>::ComputeBUnpackeddequantizes B on every call (the existingTODO(fajin): move B dequant to prepack). That already applies to any 8-bit DQ -> MatMul that gets fused; with this change it also applies to tied lm_heads. Happy to follow up on that path separately.Both
whisper-tinydecoder_model_merged_uint8.onnxanddecoder_model_merged_quantized.onnxload, with the same ops as 1.31.1.Repro for the outer-scope crash on 1.31.1:
Fixes #28306 properly (follow-up to #28326). cc Tianlei Wu (@tianleiwu)