Skip to content

onnx EP: claim the A8W8 QDQ group as one fused node (TQDQ task 1) - #689

Merged
bong-water-water-bong merged 3 commits into
mainfrom
1bit/onnx-ep-qdq-fused
Oct 31, 2026
Merged

bong-water-water-bong merged 3 commits into
mainfrom
1bit/onnx-ep-qdq-fused

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Task 1 of the A8W8 QDQ fast path (docs/onnx-pack.md, "What works today, and what does not").

What changed

DequantizeLinear -> MatMul -> QuantizeLinear (the shape Quark's ONNX-to-ONNX flow emits) is now
claimed as one fused node instead of the MatMul alone, so the dequantised float tensors never
exist.

  • E_GetCapability: matches the group (both DequantizeLinear nodes, the MatMul, and its single
    QuantizeLinear consumer), checks int8 types, rank 2/3 activations, rank-2 constant int8 weights,
    per-tensor float scales and zp = 0, and claims all four nodes with
    EpGraphSupportInfo_AddNodesToFuse(.., 4, ..).
  • E_Compile: turns the fused node back into a plan. The fused node keeps the group's values as
    inputs under their original names, so the roles (activation, weight, s_x, s_w, s_y) are read
    by value name -- the three float scalars are indistinguishable by type and rank -- and recorded by
    index plus the fused node's name, which is what NodeComputeContext_NodeName hands to
    CreateState. No initializer is copied: the weight and the scales are read out of the kernel
    context on every run.
  • QC_Compute: int8 in, int8 out. onebit_ep::gemm_q8 (new, in onebit_ep_kernel.*) accumulates
    exactly in int32 and folds the whole dequantise/requantise chain into one multiply by
    s_x*s_w/s_y and one round-half-to-even per element (ONNX QuantizeLinear), with saturation.
  • ONEBIT_EP_QDQ=0 disables fusion, so the same graph can be measured both ways.

Not claimed, deliberately: per-axis scales and non-zero zero points. Those need the kernel epilogue
(tasks 2 and 3 of the brief); such a graph keeps the per-node path -- slower, still correct.

Measured gate (strixhalo, ORT 1.23.2, cmake -S onnx -B build && ctest --test-dir build)

5/5 tests pass. onebit_ep_qdq_proof:

claim measured
int8 outputs identical to the CPU EP worst |d| = 0 LSB on the 8x16x6 proof graph; 0 on 256x1024x1024 (262144/262144 values)
the float intermediates are gone peak RSS growth of the run: CPU EP +6404 kB vs fused +260 kB = 6144 kB, exactly the 1 MB x_dq + 4 MB w_dq + 1 MB y of float32 it no longer allocates (3 runs: 6144, 6144, 6140 kB). Peak reset via /proc/self/clear_refs plus malloc_trim between warm-up and measurement, so neither session's freed buffers flatter the other
the fused kernel, not the float one, produced it trace: 3 qdq lines, 0 matmul lines, qdq_groups=1 per graph
control ONEBIT_EP_QDQ=0: per-node path, 3 float matmul lines, no qdq line, phase-1 exact match

By-product worth knowing: the int8-domain path is more exact than the float path on the large case.
The control shows 5 of 262144 values off by 1 LSB, because the CPU EP accumulates
(x_q*s_x)*(w_q*s_w) in float where the fused path sums exact int32 first.

What is not in this PR

  • The device leg. The fused node's int8 GEMM runs on the host. Feeding int8 activations into the
    device input BO needs a runtime entry that writes Am from int8 and packs int8 weights without the
    amax/127 step (packB is float-in), and it cannot be validated without an NPU window. So this is
    a correctness and memory win today, not yet a speed one. npu/ort/onebit_npu_gemm.cpp is untouched
    (pi-ortep owns the weight-cache slot there); the question is filed in ~/briefs/TORTEP-STATE.md.
  • The ONEBIT_EP_NPU=ON build could not be compiled here: it needs ONEBIT_NPU_RT_DIR holding
    npu_rt.cpp and npu_rt_api.cpp, and that private runtime source is not on strixhalo. Nothing in
    this PR changes NPU code paths, but I did not build that configuration.
  • Pre-existing noise, not fixed here: registering the plugin library makes ORT print ~630
    "Schema error: Trying to register schema ... already registered" lines to stderr. It happens on
    main too, in fused and unfused runs alike.

Docs: docs/onnx-pack.md moves the fused-group item from "Not done" to "Works" with the numbers
above and lists what is still open (per-axis, zero points, device leg). docs/onnx.md describes the
fused claim, the new control test and the 5-test roster.

Append-only report: ~/briefs/TQDQ-STATE.md.


Update 2026-10-31T07:55:49-03:00: the proof was borrowing caller memory for its initializers (CreateTensorWithDataAsOrtValue), and the zero point is a local — a no-optimization build reuses that stack slot and runs the graph with a garbage zp (all 48 outputs off by up to 68 LSB, deterministically; optimized builds hid it). Fixed with CreateTensorAsOrtValue + memcpy in commit f3b17f7. Verified in a fresh no-optimization build: 5/5 tests pass, worst |d| = 0.

DequantizeLinear -> MatMul -> QuantizeLinear is the shape Quark's ONNX-to-ONNX
flow emits, and until now our EP claimed only the MatMul inside it, so the CPU
EP still materialised x_dq, w_dq and y as float32 tensors.

GetCapability now matches the group (both DequantizeLinear nodes, the MatMul and
its single QuantizeLinear consumer) and claims all four as ONE fused node via
EpGraphSupportInfo_AddNodesToFuse. Compile turns the fused node back into a plan
(which of its inputs is the activation, the weight and the three scales, by value
name, keyed by the fused node's name so CreateState finds it again), and the
compute runs onebit_ep::gemm_q8: int8 activations and int8 weights, exact int32
accumulation, one multiply by s_x*s_w/s_y and one round-half-to-even per output
element. No float tensor is ever written.

Symmetric per-tensor only. A graph with per-axis scales or a non-zero zero point
keeps the per-node path -- those need the kernel epilogue (docs/onnx-pack.md).
ONEBIT_EP_QDQ=0 disables fusion so the same graph can be measured both ways.

Proof (ctest -R onebit_ep_qdq_proof), measured on strixhalo, ORT 1.23.2:
  * worst |d| = 0 LSB against the CPU EP on the 8x16x6 proof graph, and 0 on a
    256x1024x1024 case (262144/262144 int8 values identical);
  * peak RSS growth of the run 6404 kB (CPU EP) vs 260 kB (fused) on
    256x1024x1024, delta 6144 kB = the 1 MB x_dq + 4 MB w_dq + 1 MB y of float32
    that are no longer allocated; three runs: 6144, 6144, 6140 kB;
  * the trace shows the fused `qdq` line and zero float `matmul` lines, so the
    float kernel did not produce these numbers;
  * the control (ONEBIT_EP_QDQ=0) runs the per-node path and asserts the float
    kernel ran instead.

The fused node's int8 GEMM is the host kernel: feeding int8 straight into the
device input BO is not wired yet (needs a float-free packB/A entry in the NPU
runtime and a window to validate it), so this is a correctness and memory win,
not yet a speed one on a real model.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY (tprs-ready applied). The byte-identical gate is in the PR and it is a gate, not a printout — I built the branch and ran the whole roster — and nothing NPU-internal is exposed.

The gate is enforced in code and reproduces here. onnx/ep/onebit_ep_qdq_proof.cpp fails on any of five conditions, each with its own exit path: worst_small != 0 (phase 1 not byte-identical), worst_big != 0 on the fused path, float_lines != 0 (the float MatMul ran inside the fused path), groups == 0 (no group claimed as one node), and peak_fused >= peak_cpu (the float tensors did not go away). Built -DONEBIT_ORT=ON at c86378823 (gcc-16, 36/36, build and CodeQL already SUCCESS) and ran it:

$ ctest -R "ort_info|onebit_ep"
47 ort_info ................... Passed  0.01 s
48 onebit_ep_proof ............ Passed  0.05 s
49 onebit_ep_proof_cpu_fallback  Passed  0.05 s
50 onebit_ep_qdq_proof ........ Passed  0.29 s
51 onebit_ep_qdq_proof_unfused  Passed  0.31 s      -> 5/5, 0 failed

$ ./onnx/ep/onebit_ep_qdq_proof ./onnx/ep/libonebit_ep.so        # fused
  capability nodes=4 claimed=0 qdq_groups=1 … compile node=… qdq=1 … qdq rows=256 k=1024 n=1024 (x3)
  path: fused (2 QDQ group(s) claimed as one node, 3 int8 kernel runs, 0 float MatMul runs)
  PASS OneBitEp fused the A8W8 QDQ group, worst |d|=0 LSB (exact), 3 int8 kernel runs, 0 float MatMul
       runs, peak RSS -6780 kB

$ ONEBIT_EP_QDQ=0 ./onnx/ep/onebit_ep_qdq_proof ./onnx/ep/libonebit_ep.so   # control
  matmul batch=1 m=256 k=1024 n=1024 npu=0 (x3)
  path: unfused (3 float MatMul runs, no fused QDQ kernel)
  PASS … unfused …, phase 1 worst |d|=0 LSB (exact), phase 2 worst |d|=1 (informational)

The claimed=0 on the capability line is the point, not a bug: the MatMul is no longer claimed singly, the group is (qdq_groups=1). The RSS gate is relative (mine reported −6780 kB where the body records 6144 kB), so it does not depend on this box's allocator — good design for a memory claim. And the control reproduces the by-product the body mentions: the float path is 1 LSB off on a few of 262144 values while the fused int8 path is exact.

No NPU kernel internals in the public tree. grep -nE "npu/ort|I8Ctx|full_i8_|\.elf|loom|npu_rt|ONEBIT_NPU|xdna|kernel-corpus|briefs/" over the entire 1212-line diff → no matches, and npu/ort/* is untouched (as the body says). The new public surface is the fused claim and gemm_q… in onnx/ep/, all CPU-side.

Two observations, neither blocking:

  1. The PR body itself points at a private path — "the question is filed in ~/briefs/TORTEP-STATE.md". Same class as the #685 nit you had fixed, and milder in a body than in a doc page, but the body is public text on a public repo; a bare "filed with the ONNX-lane follow-ups" would do.
  2. The 3 non-nlohmann warnings in this build are -Wunused-result in onnx/ort_run.cpp (lines 288, 368, 370), which pre-date this PR (that file came in with #683 and is not in this diff) — a main-tree cleanup for whoever next touches ort_run, not something to fix here.

@bong-water-water-bong bong-water-water-bong added the tprs-ready TPRS: review verdict READY posted by the standing reviewer; merge gate satisfied label Oct 31, 2026
bong-water-water-bong and others added 2 commits October 31, 2026 07:55
add_scalar/add_matrix handed ORT a pointer to the caller's storage via
CreateTensorWithDataAsOrtValue, so the scales and the zero point had to outlive
the session -- and the zero point is a local of build_model. A no-optimization
build reuses that stack slot and runs the graph with a garbage zp (found while
adding task 2's per-axis cases: all 48 outputs off by up to 68 LSB,
deterministically). Optimized builds happened to keep the value and hid it.

CreateTensorAsOrtValue + memcpy instead, which is the documented path for small
initializers. Verified in a fresh, no-optimization build: 5/5 pass, worst |d| = 0.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY — re-issued at the new head 1f7d64605 (my earlier verdict was at c86378823, so it does not cover 4fa2fb3 + the main merge; tprs-ready stays on, since this comment is the verdict for this head).

The new commit is a real lifetime fix, and I verified it in the configuration that exposed the bug. 4fa2fb3 replaces CreateTensorWithDataAsOrtValue (which borrows the caller's buffer) with an ORT-owned CreateTensorAsOrtValue + memcpy, after the borrowed scale/zero-point locals produced a garbage zero point in a no-optimisation build. So I built the proof at -O0 as well as Release, at this head:

Release (5/5 ctests pass)
  PASS OneBitEp fused the A8W8 QDQ group, worst |d|=0 LSB (exact), 3 int8 kernel runs,
       0 float MatMul runs, peak RSS -6144 kB
Debug / -O0 (the configuration the borrowed-initializer bug needed)
  PASS OneBitEp fused the A8W8 QDQ group, worst |d|=0 LSB (exact), 3 int8 kernel runs,
       0 float MatMul runs, peak RSS -6292 kB

i.e. the fix holds where the old code failed, not only where it happened to work. ctest -R "ort_info|onebit_ep" → 5/5 (including the onebit_ep_qdq_proof_unfused control). CI on this head: no failures, build and pr_agent still running at the time of writing (pr_agent red is the expected coder-llm-off noise, as always).

Everything from my earlier review still holds at this head: the byte-identical gate is enforced by exit code on five conditions (phase-1 worst != 0, fused phase-2 worst != 0, any float matmul batch= line in the fused path, qdq_groups == 0, peak_fused >= peak_cpu), the fused run emits qdq rows=… and zero float MatMul lines, and no NPU internals appear in the diff (grep -nE "npu/ort|I8Ctx|full_i8_|\.elf|loom|npu_rt|ONEBIT_NPU|xdna|kernel-corpus" → no matches).

One thing to know before you merge this: the main merge in 1f7d64605 moved this branch, and #690 is now CONFLICTING against it (it was stacked on c86378823). Merge #689 first, then #690 needs a rebase onto the new head — the collision is in the two files both touch (onebit_ep_qdq_proof.cpp, docs/onnx-pack.md) and should be mechanical, since #689's side is the new add_initializer()/add_scalar() wrapper.

@bong-water-water-bong
bong-water-water-bong merged commit 7d39a53 into main Oct 31, 2026
7 of 8 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/onnx-ep-qdq-fused branch October 31, 2026 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tprs-ready TPRS: review verdict READY posted by the standing reviewer; merge gate satisfied

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant