[bong-water-water-bong]

Root cause — it is two defects, not one

#869 (7e64bbd) gave I8Ctx::args_fit_elf() a call to q4nx_elf_arg_sizes(), and the ONNX EP's NPU leg broke in
two independent ways:

1. Undeclared at compile time. q4nx_elf_arg_sizes() is declared in npu/forward/include/q4nx_pack.h:257.
   npu/forward/src/q4nx_forward.cpp happens to include that header before pulling in
   npu_forward/engine-src/npu_engine_i8ctx_inc.h, so the forward route compiles; npu/ort/onebit_npu_gemm.cpp:21
   does not, so the EP leg does not. The inc header is now self-contained — q4nx_pack.h joins the three q4nx
   headers it already includes, and it is extern "C"-guarded for C++ callers and pulls in nothing of its own.

2. Unlinked at link time — which the issue's proposed one-line fix does not cover.
   q4nx_elf_arg_sizes() is defined in npu/forward/src/q4nx_pack.c:470, and onnx/ep/CMakeLists.txt's
   _npu_src list did not compile that file. With only the #include added, onebit_ep.so links (a shared
   library tolerates the undefined symbol) but the two probe executables that share _npu_src do not:

   ld.bfd: CMakeFiles/onebit_npu_gemm_q8_probe.dir/.../onebit_npu_gemm.cpp.o: in function
     `I8Ctx::args_fit_elf(char const*)': undefined reference to `q4nx_elf_arg_sizes'
   collect2: error: ld returned 1 exit status
   

   So the fix is two lines: the include, and q4nx_pack.c in _npu_src. q4nx_pack.c is self-contained (its own
   header plus math.h/stdio.h/string.h), so nothing else comes with it.

Measured before/after

Reproducer from the issue, on a clean worktree off origin/main (23d6972):

g++-16 -DONEBIT_NPU_RUNTIME_OWN=1 -Inpu/forward/engine-src -Inpu -Inpu/forward/include -Inpu/ort \
       -I<npu_rt> -std=c++26 -O1 -fsyntax-only npu/ort/onebit_npu_gemm.cpp

	result
before	error: 'q4nx_elf_arg_sizes' was not declared in this scope (line 219), rc 1
after the #include only	rc 0 — but the full build then died at link (the trace above)
after both lines	rc 0, and the full configuration builds

Full configuration, the one CI never compiles (cmake -S onnx -B … -DONEBIT_EP_NPU=ON -DONEBIT_NPU_RT_DIR=… ):

	before	after
ONEBIT_EP_NPU=ON build	fails	rc 0, 9 targets (onebit_ep.so, ort_run, onebit_npu_gemm_probe, onebit_npu_gemm_q8_probe, …), no warnings
q4nx_elf_arg_sizes in the probe binaries	—	defined text symbol in both (nm -C | grep -c 'T q4nx_elf_arg_sizes' = 1 each)
forward consumer (npu/forward/src/q4nx_forward.cpp, the route that always compiled)	rc 0	rc 0 — the added include changes nothing there

Device known-answer probe on the array (the target #869 actually broke, and the one #853/#862 could not rebuild):
onebit_npu_gemm_q8_probe 1024 1024 32 5 with ONEBIT_EP_NPU_ELFS=32x1024x1024=…/KV_K1024_N1024/design.elf,
plus the float probe at ctest's own args — one npu-hold ticket (01793677294039422075.qdq, token after 4 s,
die 63 °C, released after 1 s, amdxdna errors thereafter: 0):

PASS NPU gemm matches the host reference, max_abs=1.007e-01                         (float probe, 6.64 ms/call)
PASS NPU q8 gemm matches the exact host accumulators on 32768 values,
     device 1.228 ms/call vs host 125.727 ms/call                                   (q8 probe, the #869 target)

The device column is the number to read; the 102x "speedup" is an artifact of this build being unoptimized
(no CMAKE_BUILD_TYPE, the stricter build for correctness), so the host loop it is measured against is -O0 —
at -O2 the same loop is ~2.5 ms (docs/onnx.md, #853). What the probe proves here is the part #874 is about:
the target that could not be built on main now builds, runs the array, and its exact int32 accumulators
match the host element for element.

Not addressed, on purpose

The gap behind it — .github/workflows/ci.yml never sets ONEBIT_EP_NPU (or ONEBIT_NPU), so this whole
configuration can go red on main while CI is green, exactly as #657 did. I did not add a CI job in this PR: both
configurations need the private NPU runtime (rt.h / npu_rt.h), which CI does not have, so a job needs a stub
runtime header first — a separate change with its own review, not a rider on a build fix.

Closes #874.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit b1a8805)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 869 - Partially compliant Compliant requirements: Added q4nx_elf_arg_sizes() function that reads argument widths from ELF symbols Modified I8Ctx::init_elf() to refuse launches when buffer sizes are smaller than declared ELF argument sizes Added tests for ELF argument size checking Verified the fix prevents buffer overruns in NPU kernel launches Non-compliant requirements: None Requires further human verification: None 853 - Partially compliant Compliant requirements: Added instrumentation to measure A8W8 kernel performance on both device and host Verified device and host accumulators match exactly Documented that device is dispatch-bound and slower for small shapes Reported performance metrics for various shapes and row counts Non-compliant requirements: None Requires further human verification: None 862 - Partially compliant Compliant requirements: Changed gemm_q8 routing to use a minimum row threshold (16 rows) for device execution Updated routing logic to use host loop for shapes below threshold Added assertion in proof tests to verify routing behavior Documented the performance measurement results showing device becomes faster at 16+ rows Non-compliant requirements: None Requires further human verification: None

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing Include The PR adds the missing include for q4nx_pack.h to npu_engine_i8ctx_inc.h. This is necessary for the I8Ctx::args_fit_elf() function to properly reference the q4nx_elf_arg_sizes() function. This change ensures that the ONNX EP's NPU leg can compile correctly by providing the required declaration. #include "q4nx_pack.h" // q4nx_elf_arg_sizes(): args_fit_elf() checks the ELF's own widths Missing Link Dependency The PR adds q4nx_pack.c to the _npu_src list in CMakeLists.txt. This ensures that the q4nx_elf_arg_sizes() function is properly linked when building the ONNX EP NPU components. Without this, the link step would fail with an undefined reference error. ${CMAKE_CURRENT_SOURCE_DIR}/../../npu/forward/src/q4nx_pack.c

[bong-water-water-bong]

VERDICT: READY

This closes the break I filed as #874, and I verified the thing my report could not: the whole EP NPU configuration now
links, not just compiles.

cmake -DONEBIT_ORT=ON -DONEBIT_EP_NPU=ON -DONEBIT_NPU_RT_DIR=… -DONEBIT_NPU_ENGINE_INCS=…
[9/11]  Linking CXX shared library onnx/ep/libonebit_ep.so
[10/11] Linking CXX executable onnx/ep/onebit_ep_qdq_proof
[11/11] Linking CXX executable onnx/ep/onebit_ep_proof
$ ./onnx/ep/onebit_ep_qdq_proof ./onnx/ep/libonebit_ep.so   -> (the five quark cases, as before)

Both halves of the fix are load-bearing and you have them the right way round: #include "q4nx_pack.h" in
npu/ort/onebit_npu_gemm.cpp fixes the compile (‘q4nx_elf_arg_sizes’ was not declared in this scope — the inc file is
included before any header that declares it), and adding npu/forward/src/q4nx_pack.c to _npu_src fixes the link. My
issue named only the first because the syntax-only compile I could run at the time dies before the linker; your comment is
right that CI stayed green because neither NPU configuration is built there.

The comment in onnx/ep/CMakeLists.txt naming #874 at the point where the source is added is the part that keeps this from
coming back, and the include's own comment says which call site needs it (args_fit_elf(), #869).

Worth noting for the #874 follow-up: with this landed, the two ctest entries that live on this path (#853's
onebit_npu_gemm_q8_probe, #882's DEQUANT-independent work) can finally be rebuilt and run in the NPU configuration, so the
"cannot reproduce the device numbers" caveat I attached to #853 goes away.

[github-actions]

Persistent review updated to latest commit b1a8805

[bong-water-water-bong]

VERDICT: READY

At b1a88058a (merge of main into the head I reviewed; both files blob-identical, so the content is the content I verified).
Build success at this head.

Evidence: with -DONEBIT_EP_NPU=ON the whole EP NPU configuration now links — libonebit_ep.so, onebit_ep_qdq_proof and onebit_ep_proof all link (11/11 targets), and the QDQ proof runs to PASS … worst |d| = 0/0/0/0 LSB, per-K refused, peak RSS -5616 kB. Both halves are load-bearing: the #include "q4nx_pack.h" fixes the compile (args_fit_elf() declared), adding npu/forward/src/q4nx_pack.c to _npu_src fixes the link. This closes the break I filed as #874, which my syntax-only repro could not see past.