CI does not compile npu/forward at all (ONEBIT_NPU off), so a PR touching the NPU forward gets a green build without its C++ being built

What

.github/workflows/ci.yml:64 configures cmake without -DONEBIT_NPU=ON:

run: cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc-16 -DCMAKE_CXX_COMPILER=g++-16

so the build job compiles none of npu/forward/*, npu/target/* or npu/rt/*. A pull request that only touches
the NPU forward gets a green build without any of its C++ being compiled. It is not a hypothetical: every defect
below reached a green CI.

Evidence (all on this box, all found by device gates, none by CI)

defect	PR	what CI said
`npu_gemm`'s ninth argument (`int a_k`, #854) landed in `wkey`/`sc` at 16 call sites during a rebase; 7 of them are a hard `invalid conversion from 'int' to 'const char*'`	#808 (`40750cb8`)	build green on the previous head; the tree did not compile
`const bool head_pb16i = head_pb16i && …` — a self-referencing initializer, indeterminate value, wrong kernel family launched	#808 (`f579d5eb`)	green
the rotation was re-implemented as an xor/strided butterfly instead of `q4nx_hadamard::rotate` (different transform, not different rounding)	#808 (`8e743044`)	green
`open()` recomputed the kernel family locally instead of calling `family_tag()`, refusing every `_t2s` plan	#808 (`34bad539`)	green
the decode-twin fallback became a dereference of a null context for every kernel set without `_m16` twins	#808 (`f80f44e`)	green

The same hole is why #874 (onnx EP NPU leg called q4nx_elf_arg_sizes without its declaration on main since #869)
had to be found by a person rather than by CI — the file was not compiled.

Why it is not just "turn the flag on"

ONEBIT_NPU=ON needs ONEBIT_NPU_RT_LIB (libonebit_npu_rt.a, the own NPU runtime) and xrt headers, which the
CI image does not have. So the honest options, cheapest first:

add a compile-only job that builds onebit_npu_forward against a stub/interface-only rt.h — catches every
signature, initializer and type error above, which is what all five defects were;
build the own NPU runtime in CI too (heavier, and it pulls XRT into the image);
declare it: state in CONTRIBUTING/the PR template that a change under npu/forward, npu/target or npu/rt
must show a local ninja onebit_npu_forward in the PR body, and have pr_agent/review check for it.

Option 1 would have caught 3 of the 5 outright (the call-site types, the header's missing member) and option 3 is a
one-line policy that costs nothing.

Acceptance

Either a CI job that compiles npu/forward/src/q4nx_forward.cpp and npu/target/xdna/xdna_target.cpp (it may link
nothing), or the policy sentence plus one example of a reviewer applying it.

Triaged on this box, and two parts of the report's premise are wrong in the helpful direction — the fix is smaller than option 1 as written:

No stub rt.h is needed. npu/rt.h is in the tree at origin/main and is 8 lines: an #error unless ONEBIT_NPU_RUNTIME_OWN is defined, then #include "npu_rt_api.h" + namespace xrt = npu_rt_api; (npu/rt.h:26-40). No XRT headers are involved — AMD's XRT is explicitly not supported. So compiling q4nx_forward.cpp needs the macro that CMakeLists.txt:780 already sets, and nothing else. (My own note on the board for several passes said a stub was owed; that note is retracted here.)
The hole is one file, not a directory. Two of npu/forward's C sources are compiled by the current job through tests — npu/forward/src/model.c (CMakeLists.txt:641) and q4nx_pack.c (:529, :667). npu/forward/src/q4nx_forward.cpp is the one that is only in the ONEBIT_NPU branch (:802), and npu/target/npu/rt do not exist on main at all (they are npu: extract the XDNA launch layer behind NpuTarget (P4 of #799, A1) #808's P4 branch).

Measured, twice, at 65460184 with g++-16:

clean tree         -> rc 0, no diagnostics
  npu_gemm(..., 0, "attn_qkv", g->real_k)  ->  0                    [the #808 rebase defect]
  -> error: invalid conversion from 'int' to 'const char*'
     (initializing argument 8 of npu_gemm(I8Ctx&, const float*, int, int, const float*,
      std::vector<float>&, int, const char*, int))

Labels: bug + npu. I took this one — PR #912 (ci: compile npu/forward's forward C++ as part of ``onebit ``): an OBJECT library `onebit_npu_forward_syntax` over `q4nx_forward.cpp` with the real target's include dirs/macro/OpenMP flags, plus `add_dependencies(onebit …)`, so the existing `build` job compiles it. It links nothing. `ninja -n onebit` now lists `q4nx_forward.cpp.o` at `[23/27]`, and the object fails exactly as above when the defect is injected. One file, `CMakeLists.txt`: this box's token has no `workflow` scope, so `ci.yml` cannot be edited from here — a lane that has that scope can add the target name to the explicit list as a one-line follow-up, which only makes the step visible by name.

Not covered by #912: the same hole for the ONNX EP leg (ONEBIT_EP_NPU off — the #874 class). That is a second change, and it stays unowned.

What I will do: if the merged job's first runs go red on anything the new target legitimately catches, that is a real defect and I will file or route it, not relax the job.

ci: compile and link the ONNX EP's NPU leg, so a broken ONEBIT_EP_NPU…

Navigation Menu

Repository navigation

CI does not compile npu/forward at all (ONEBIT_NPU off), so a PR touching the NPU forward gets a green build without its C++ being built #898

What

Evidence (all on this box, all found by device gates, none by CI)

Why it is not just "turn the flag on"

Acceptance

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

CI does not compile npu/forward at all (ONEBIT_NPU off), so a PR touching the NPU forward gets a green build without its C++ being built #898

Description

What

Evidence (all on this box, all found by device gates, none by CI)

Why it is not just "turn the flag on"

Acceptance

Activity

bong-water-water-bong commented on Nov 3, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions