Skip to content

Fuse DQ->MatMulNBits for shared initializers and skip outer-scope weights - #33800

Open
Vijay Kapse (vijay-kapse) wants to merge 4 commits into
microsoft:mainfrom
vijay-kapse:fix/dq-matmulnbits-outer-scope
Open

Vijay Kapse (vijay-kapse) wants to merge 4 commits into
microsoft:mainfrom
vijay-kapse:fix/dq-matmulnbits-outer-scope

Conversation

@vijay-kapse

Copy link
Copy Markdown
Contributor

Description

DQMatMulNodeGroupSelector accepts a DequantizeLinear whose weight, scale and zero point are initializers of an enclosing graph (GetConstantInitializer(name, true)), but TransposeDQWeightsForMatMulNBits reads them with Graph::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:

FAIL : qdq_actions.cc:132 TransposeDQWeightsForMatMulNBits Missing required weight: w_q for node: 

This is what #28306 hit: in onnx-community/whisper-tiny decoder_model_merged_uint8.onnx the embedding scale lives in the main graph and both DQ nodes are inside the optimum::if branches. 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-Instruct model_int8.onnx that is a [576, 49152] uint8 -> fp32 DequantizeLinear per token, 44% of int8 decode time on a Neoverse N2.

This change:

  • requires the weight/scale/zero point to be initializers of the node's own graph (GetConstantInitializer(name, false)), matching what the action can read, and
  • removes the shared-initializer guard. The action only reads the originals and adds new transposed initializers, so sharing is safe; the originals are cleaned up once unused.

Tests: the two *_SharedWeight tests and the A16 shared case now expect fusion (outputs still compared against the unfused graph), and DQMatMulNotConvertedToMatMulNBits_OuterScopeWeight covers 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 / HuggingFaceTB model.onnx vs model_int8.onnx:

model fp32 int8, 1.31.1 int8, this PR
SmolLM2-135M-Instruct 13.24 17.84 (0.74x fp32) 8.99 (1.47x fp32)
Qwen2.5-0.5B-Instruct 51.68 69.17 (0.75x fp32) 29.28 (1.77x fp32)

Same models on GitHub ubuntu-24.04 (AMD EPYC 7763, 2 threads):

model fp32 int8, 1.31.1 int8, this PR
SmolLM2-135M-Instruct 21.62 19.62 (1.10x fp32) 10.79 (2.00x fp32)
Qwen2.5-0.5B-Instruct 82.27 83.39 (0.99x fp32) 40.92 (2.01x fp32)

Accuracy, against 1.31.1 on the same int8 models:

  • default 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 no SQ8BitGemmVariant_CompFp32 and MatMulNBits<float>::ComputeBUnpacked dequantizes B on every call (the existing TODO(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-tiny decoder_model_merged_uint8.onnx and decoder_model_merged_quantized.onnx load, with the same ops as 1.31.1.

Repro for the outer-scope crash on 1.31.1:

import numpy as np, onnx, onnxruntime as ort
from onnx import helper as h, TensorProto as T, numpy_helper as nh
K, N = 64, 32
def branch(name):
    dq = h.make_node("DequantizeLinear", ["w_q", "w_scale", "w_zp"], [f"w_{name}"])
    mm = h.make_node("MatMul", ["x", f"w_{name}"], [f"y_{name}"])
    return h.make_graph([dq, mm], name, [], [h.make_tensor_value_info(f"y_{name}", T.FLOAT, None)])
inits = [nh.from_array(np.zeros((K, N), np.uint8), "w_q"), nh.from_array(np.array(0.02, np.float32), "w_scale"),
         nh.from_array(np.array(3, np.uint8), "w_zp")]
g = h.make_graph([h.make_node("If", ["cond"], ["y"], then_branch=branch("then"), else_branch=branch("else"))], "g",
                 [h.make_tensor_value_info("cond", T.BOOL, []), h.make_tensor_value_info("x", T.FLOAT, [1, K])],
                 [h.make_tensor_value_info("y", T.FLOAT, None)], inits)
onnx.save(h.make_model(g, opset_imports=[h.make_opsetid("", 21)], ir_version=10), "m.onnx")
ort.InferenceSession("m.onnx")  # 1.31.1: Missing required weight: w_q

Fixes #28306 properly (follow-up to #28326). cc Tianlei Wu (@tianleiwu)

…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.
Copilot AI balanced review requested due to automatic review settings November 1, 2026 18:37
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI 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.

🟡 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

Comment on lines +523 to +525
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);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.
@vijay-kapse

Copy link
Copy Markdown
Contributor Author

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.

This branch has not been deployed

No deployments
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.

Error loading quantized Whisper models with onnxruntime 1.25

2 participants