From 2650185b7dafdc46f340343d95712cceccca5f4e Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Wed, 2 Sep 2026 14:23:59 +0000 Subject: [PATCH 1/2] spec(FIX-UNALIGNED-CONSUMERS-2540): repair the consumers of unaligned safetensors bytes, not the borrow A safetensors payload starts at `8 + `, and a header length is arbitrary, so a 16-bit weight begins on an odd byte in roughly half of all checkpoints. That is an ordinary file. Three CPU consumers read those bytes through a `const uint16_t*` and `-fsanitize=alignment` aborts on the load, which is the second and third findings holding the `sanitize-cpu` lane red (#2540, #2558). The spec records why the fix goes in the consumer. Refusing to borrow a misaligned tensor also takes the sanitizer green, and it switches direct upload off for every tensor in half of all checkpoints: recomputed here from the direct-upload fixture, whose 171-byte header puts `w` at 179, `v` at 195 and `f` at 211, all odd. `vt::LoadUnaligned` costs nothing on that path, so the borrow survives. The bullet this row takes over is struck from the DEBTFIX spec's `## Owed`, together with the guess it recorded. The root cause is not "wherever `ResidentWeight` produced an odd `bytes.data()`"; it is the file, and a file is not a defect. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5-1m [claude-code] --- ...btfix-glue-rank-bound-and-repack-device.md | 18 +- .../specs/unaligned-safetensors-consumers.md | 211 ++++++++++++++++++ 2 files changed, 221 insertions(+), 8 deletions(-) create mode 100644 .agents/specs/unaligned-safetensors-consumers.md diff --git a/.agents/specs/debtfix-glue-rank-bound-and-repack-device.md b/.agents/specs/debtfix-glue-rank-bound-and-repack-device.md index 829aa9b87..e838b12c2 100644 --- a/.agents/specs/debtfix-glue-rank-bound-and-repack-device.md +++ b/.agents/specs/debtfix-glue-rank-bound-and-repack-device.md @@ -471,14 +471,16 @@ a bounds question, and it is a different row. * Dequantizing the `qwen4_exp` hyper-connection weights at load (`MODEL-MM-QWEN4-EXP`), with the residency and token evidence that needs. * The aarch64 CUDA measurement of the repack trade that #2406 asks for. -* **The misaligned bf16 gamma load** - ([#2540](https://github.com/mudler/vllm.cpp/issues/2540)). `WidenRowToF32` - reads an RMSNorm gamma at an odd address on `dots3_note` and `muse_glimmer`, - reached from `ModelRegistry::Forward` on both. Found by this wave's sweep, - filed rather than fixed: it is a different subsystem, the root cause is - wherever `ResidentWeight` produced an odd `bytes.data()`, and silencing it with - a `memcpy` in `WidenRowToF32` would fix the wrong end. It is the second - `sanitize-cpu` finding and it blocks that lane after this branch lands. +* ~~**The misaligned bf16 gamma load** + ([#2540](https://github.com/mudler/vllm.cpp/issues/2540)).~~ NO LONGER OWED + HERE. `FIX-UNALIGNED-CONSUMERS-2540` + ([`unaligned-safetensors-consumers.md`](unaligned-safetensors-consumers.md)) + claimed and fixed it on 2026-09-02, and the issue's own `Row:` line now names + that row. The guess recorded here — that the root cause was "wherever + `ResidentWeight` produced an odd `bytes.data()`" — was measured and is FALSE: + the producer is the safetensors payload base, which is `8 + ` and therefore odd for roughly half of all files. Nothing produced a + defect; `WidenRowToF32` read bytes it was not entitled to assume were aligned. ## Now diff --git a/.agents/specs/unaligned-safetensors-consumers.md b/.agents/specs/unaligned-safetensors-consumers.md new file mode 100644 index 000000000..5bb7b4ffd --- /dev/null +++ b/.agents/specs/unaligned-safetensors-consumers.md @@ -0,0 +1,211 @@ +# Unaligned safetensors bytes — repair the CONSUMERS, keep the zero-copy borrow + +Identity: `FIX-UNALIGNED-CONSUMERS-2540` + +Issues: [#2540](https://github.com/mudler/vllm.cpp/issues/2540) (bf16 RMSNorm +gamma), [#2558](https://github.com/mudler/vllm.cpp/issues/2558) (EXL3 `suh`, +`svh` and trellis). + +Class predecessors: [#301](https://github.com/mudler/vllm.cpp/issues/301), which +left the `vt::LoadUnaligned` seam; +[`.agents/specs/unaligned-safetensors-loaders.md`](unaligned-safetensors-loaders.md) +(`FIX-UNALIGNED-LOADERS-772`), which repaired the four loader casts and states +the design this row reuses; and +[#1359](https://github.com/mudler/vllm.cpp/issues/1359). + +Status: `ACTIVE`. Base `aedad724c974f3da73f70981bd3653bdca0821b6`. + +## Scope + +Make the three CPU consumers that read a borrowed safetensors payload as +`uint16_t` tolerate an odd base address, using the existing `vt::LoadUnaligned` +seam and the byte-cursor shape `vt::cpu::LoadF32` and `cpu_layernorm.cpp` +already use: + +| Consumer | Site at the base SHA | Reached by | +|---|---|---| +| `vt::cpu::WidenRowToF32` | `src/vt/cpu/cpu_matmul_elem.cpp:577` | bf16 RMSNorm gamma (#2540) | +| `HadRowBlock` / `HadRows` | `src/vt/cpu/cpu_exl3_kernels.cpp:130` | EXL3 `suh` / `svh` (#2558) | +| `TileWord32` | `src/vt/cpu/cpu_exl3_dequant.cpp:64` | EXL3 trellis, kI8 read as `uint16_t` (#2558) | + +The pointer chains that feed those three from a borrowed mapping move to byte +arithmetic in the same change, because advancing a misaligned `uint16_t*` is +undefined in its own right and a fix that only repairs the final load leaves the +`Exl3DecodeTile` / `Exl3ReconstructInner` / `MoeGemm` walk forming one. + +Change no loaded value, no shape rule, no dtype rule, no error message, no +default and no public behavior. `include/vllm.h` is untouched. Four declarations +in `include/vt/ops.h` widen a parameter from `const uint16_t*` to `const void*`; +every existing caller passes a `uint16_t*`, which converts implicitly, so no +call site outside the two `.cpp` files under repair changes. + +Explicitly excluded: + +* `BorrowStTensorBytes` and every other producer. See `## Why not the producer`. +* The device arms. CUDA, ROCm and Vulkan read device allocations, which are + 256-byte aligned by the allocator; no borrowed host pointer reaches them. +* The `NarrowRowFromF32` / `StoreF32At` store side, which writes engine-allocated + output buffers, never a file mapping. That is the polarity + `cpu_layernorm.cpp:44-46` states and this row keeps it. +* `test_qwen4_exp_layer_loop`, repaired by `33b08e463` before this base. + +## Root cause, re-verified at the base SHA + +A safetensors payload starts at `8 + ` +(`safetensors_reader.cpp:78`, `data_base = bytes + 8 + header_len`). A header +length is arbitrary, so a tensor routinely begins on an odd byte. That is an +ordinary file, not a corrupt one — the same argument the class spec makes. + +`BorrowStTensorBytes` (`qwen3_5_weights.cpp:490`) borrows the mapped bytes +verbatim, `OwnedTensor::View` aliases them into a `vt::Tensor`, and the three +consumers above then form a `const uint16_t*` over an odd address and load +through it. Under `sanitize-cpu (address,undefined)` the lane's own +`-fno-sanitize-recover=all` turns that into an abort before any doctest +assertion is reported. + +The trellis case is the sharpest statement of the polarity: `vt::Exl3Gemm` +requires `trellis.dtype == DType::kI8` ("the trellis travels as opaque i8 +BYTES", `ops.cpp:5490`), so its natural alignment requirement is 1 and an odd +base is not merely legal but expected. `TileWord32`'s own comment already says +the tile "may sit at any alignment inside a safetensors mmap" — it assembles the +uint32 by hand for exactly that reason — and then reads it through +`tile[2 * index]`. The comment states the contract the code does not keep. + +## Why not the producer + +External PR #2561 fixes the same seven tests the other way: a fail-closed +2-byte-alignment gate inside `BorrowStTensorBytes`, so a misaligned tensor falls +back to `MakeOwned` + `memcpy`. It works — its sanitize run reported no +`misaligned address` — and it is the shape #2558's own body proposes. This row +rejects it, and the measurement is the reason. + +`tests/vllm/test_load_direct_upload.cpp`'s fixture header is 171 bytes, so the +payload base is 179, and **every** tensor in the file is refused: + +| Tensor | Base | `% 2` | `% 4` | +|---|---:|---:|---:| +| `w` | 179 | 1 | 3 | +| `v` | 195 | 1 | 3 | +| `f` | 211 | 1 | 3 | + +Recomputed from `Header()` and `U64Le(h.size())` at this base SHA, independently +of #2561. Since a header length is arbitrary, the parity of the base is a coin +flip per file, so a producer gate switches off direct upload for every tensor in +roughly half of all bf16 checkpoints — for a hazard that costs nothing to +tolerate. It also turns two passing tests red: +`tests/vllm/test_load_direct_upload.cpp:142` (`CHECK(w.bytes.borrowed())`) and +`tests/vllm/models/test_qwen3_5_dense_load_residency.cpp:296` ("the MODELOPT +NVFP4 arm BORROWS the mapping, it does not copy"). Both must stay green, and +they are this row's separating criterion. + +`vt::LoadUnaligned` is a `std::memcpy` of `sizeof(T)` bytes. The class spec +measured it at `-O2` as instruction-for-instruction identical to the raw cast +(`movzwl (%rdi,%rax,2)`), with no `memcpy` call emitted. The consumer repair +therefore costs nothing on the borrow path, and the borrow survives. + +#2540's body asks for the fix at the producer too ("find where the odd `w.data` +comes from and fix it THERE"). That instruction was written before #2558 +identified the producer. The producer is the file, and a file is not a defect. + +## Design + +**`WidenRowToF32`.** The `kF16` and `kBF16` arms take a `const unsigned char*` +byte cursor and read each element with `vt::LoadUnaligned`. The `kF32` +arm already went through `std::memcpy` and is untouched. This is the single +choke point: `RmsNormKernel` (`cpu_ops.cpp:557`), the elementwise matmul +(`cpu_ops.cpp:270`) and paged attention (`cpu_paged_attn.cpp:233`) all widen +through it, and it already takes `const void* src`, so no caller changes. + +**`HadRowBlock` / `HadRows`.** `pre` and `post` become `const void*`. They are +the weight vectors `suh` and `svh`, which the borrow hands over as mapped bytes; +`in` and `out` stay typed because they are the activation and the engine's own +scratch, which `cpu_backend.cpp:20,42` allocates through `std::aligned_alloc(64, +...)`. Both call sites pass `suh.data` / `svh.data` rather than +`Ptr()`, so no misaligned typed pointer is formed at all. The MoE +arm's `table` lambda returns `const void*` for the same reason. + +**The trellis chain.** `TileWord32` takes a `const void*` tile base and reads its +two halves with `vt::LoadUnaligned`. `Exl3TileCodeword`, +`Exl3DecodeTile`, `Exl3ReconstructInner`, `Exl3DequantLinear` and `MoeGemm` take +`const void*` for the trellis, and every walk over it advances a `const unsigned +char*` by `tile_words * sizeof(uint16_t)` bytes. + +**That `sizeof(uint16_t)` is the whole risk of this change**, exactly as it was +for `CopyRawNK` in the class spec: the tile stride counts 16-bit WORDS and the +new cursor counts BYTES. It is what the value gate below targets, and it is why +the gate asserts byte equality against the aligned run rather than a tolerance — +a factor of two on the tile stride decodes a different weight, which a tolerance +on a random-bit trellis could plausibly absorb and an equality cannot. + +## Tests and RED evidence + +Two cases, both added to the suite that already owns the subject, so no new +`tests/CMakeLists.txt` row is created and no existing registration moves. + +1. `tests/vt/test_ops_rmsnorm_weight_dtype.cpp` — a bf16 gamma placed at an ODD + byte address inside an over-allocated buffer, run through `vt::RmsNorm` on + the CPU, asserted byte-identical to the same gamma at an even address. The + case `REQUIRE`s the address parity first, so a case that silently landed on + an even address cannot read as a pass. +2. `tests/vt/test_exl3_gemm.cpp` — `suh`, `svh` and the trellis each placed at an + ODD byte address, run through `vt::Exl3Gemm` on the CPU, asserted + byte-identical to the aligned run. Same `REQUIRE` on each parity. + +RED is taken two ways, because the two failures are different: + +* Under `-fsanitize=address,undefined` on the pre-fix tree, each case aborts + with `runtime error: load of misaligned address ... for type 'const uint16_t'` + at the site named in `## Scope`. That is the sanitize-cpu lane's failure. +* Without a sanitizer the pre-fix binary passes on x86, which is precisely why + this class survived three sweeps. The equality assertion is therefore aimed at + the byte-stride mutation the refactor could introduce, not at the original + defect, and it is red on any lane if the stride is wrong. + +## Gates + +```sh +cmake -S . -B build-sanitize -DVLLM_CPP_BUILD_TESTS=ON -DVLLM_CPP_CUDA=OFF \ + -DVLLM_CPP_SANITIZE='address,undefined' +cmake --build build-sanitize -j 4 --target \ + test_llama_embedding_fold test_muse_glimmer_text test_muse_glimmer_text_fallback \ + test_dots3_note_attn test_openai_api_server test_capi test_qwen35_exl3 \ + test_ops_rmsnorm_weight_dtype test_exl3_gemm +VT_POOL_BYPASS=1 UBSAN_OPTIONS=print_stacktrace=1 ./build-sanitize/tests/ +``` + +Zero `misaligned address` reports, and none at `cpu_matmul_elem.cpp:577`, +`cpu_exl3_kernels.cpp:130` or `cpu_exl3_dequant.cpp:64`. + +The separating gate, on the same build: + +```sh +ctest --test-dir build -R 'test_load_direct_upload|test_qwen3_5_dense_load_residency' +``` + +Both must PASS. A change that reds either of them has become #2561 and is not +this row. + +## Risks and stop conditions + +* **Byte-vs-word stride.** Bounded by the equality gate above and by the + existing bit-exact EXL3 decode suites (`test_exl3_dequant`, `test_exl3_gemv`, + `test_exl3_moe`, `test_exl3_gemm`), which compare against references built + from definitions rather than from the implementation. +* **A `const void*` parameter accepts anything.** The four widened declarations + lose the compiler's `uint16_t*` type check. They are host decoders of a + checkpoint FORMAT whose operand is stated in the declaration's own comment as + "the tile's 16\*bits int16 words as stored", and every caller in the tree + passes exactly that. Recorded as the cost of removing the misaligned typed + pointer, not as a free change. +* **No belt is added to `vt::Exl3Gemm`.** #2561 added `VT_CHECK`s asserting + 2-byte alignment of `suh`, `svh` and `trellis`. After this repair those + operands are legitimately misaligned on any ordinary checkpoint, so such a + check would refuse the borrow path this row exists to preserve — it would be + #2561's producer gate relocated one frame up. A refusal must name something + the kernel cannot represent, and misalignment is no longer one. +* Stop and report on ENOSPC or an OOM symptom rather than pressing on. The box + has both histories. + +## Now + +`ACTIVE`. From 3a0d6c5667059c91dfeca82530ab529ea41754df Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Wed, 2 Sep 2026 14:44:13 +0000 Subject: [PATCH 2/2] fix(FIX-UNALIGNED-CONSUMERS-2540): read a borrowed 16-bit weight off a byte cursor, so the odd base costs nothing Closes #2540. Closes #2558. Closes #2578. `WidenRowToF32`, `HadRowBlock`, `TileWord32` and `WeightF32` each formed a `const uint16_t*` over bytes the direct-upload borrow handed them straight out of an mmap. A safetensors payload starts at `8 + `, a header length is arbitrary, and `vt::Exl3Gemm` types the trellis kI8, so an odd base is the expected case rather than an exotic one. The load is undefined even where x86 executes it, and the `sanitize-cpu` lane aborts on it before reporting a single assertion. All four now read through `vt::LoadUnaligned` off an `unsigned char` cursor, which is the shape `vt::cpu::LoadF32` and `cpu_layernorm.cpp` already use and which compiles to the same instruction at `-O2`. The trellis walk moves to byte arithmetic for the same reason: advancing a misaligned `uint16_t*` is undefined in its own right, so `Exl3DecodeTile`, `Exl3ReconstructInner`, `Exl3DequantLinear` and `MoeGemm` take a `const void*` and the stride carries the `sizeof(uint16_t)` the pointer type used to supply. Every existing caller passes a `uint16_t*`, which converts implicitly, so no call site changed. `in`, `out` and the activation keep their typed pointers. They are engine-allocated through `std::aligned_alloc(64, ...)`, never a borrowed file mapping, which is the polarity `cpu_layernorm.cpp` states. The alternative fix refuses the borrow. It works, and it turns direct upload off for every tensor in half of all checkpoints -- the direct-upload fixture's 171-byte header puts all three of its tensors on odd addresses -- so it reds `test_load_direct_upload` and `test_qwen3_5_dense_load_residency`. Both stay green here, which is what separates the two fixes. No alignment `VT_CHECK` is added to `vt::Exl3Gemm` for the same reason: after this change a misaligned operand is representable, and a refusal must name something the kernel cannot do. `WeightF32` was not in the dispatch. It is the SAME defect one frame further on, and it only became visible because `-fno-sanitize-recover=all` stops at the first report: repairing the EXL3 operands made `test_qwen35_exl3` abort in `WeightF32` on the very next run. It is filed as #2578 and fixed here, because the target state is not reached without it. A grep finds 20 further sites of the shape in ten model files; whether any is reached with an odd base is unmeasured, so that is filed as #2579 and named under the spec's `## Owed` rather than swept blind. Two cases gate it, each in the suite that already owns its subject, and each `REQUIRE`s the address parity it depends on before it asserts anything. They compare byte-for-byte against the same operands at even addresses, because a byte cursor that lost its factor of two decodes a different weight and a tolerance over a random-bit trellis could absorb that. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5-1m [claude-code] --- .../specs/unaligned-safetensors-consumers.md | 32 ++++++- include/vt/ops.h | 19 ++-- src/vllm/model_executor/models/qwen3_5.cpp | 17 +++- src/vt/cpu/cpu_exl3_dequant.cpp | 43 ++++++--- src/vt/cpu/cpu_exl3_kernels.cpp | 89 +++++++++++++------ src/vt/cpu/cpu_matmul_elem.cpp | 29 +++++- tests/vt/test_exl3_gemm.cpp | 84 +++++++++++++++++ tests/vt/test_ops_rmsnorm_weight_dtype.cpp | 64 +++++++++++++ 8 files changed, 326 insertions(+), 51 deletions(-) diff --git a/.agents/specs/unaligned-safetensors-consumers.md b/.agents/specs/unaligned-safetensors-consumers.md index 5bb7b4ffd..4820cbc40 100644 --- a/.agents/specs/unaligned-safetensors-consumers.md +++ b/.agents/specs/unaligned-safetensors-consumers.md @@ -4,7 +4,8 @@ Identity: `FIX-UNALIGNED-CONSUMERS-2540` Issues: [#2540](https://github.com/mudler/vllm.cpp/issues/2540) (bf16 RMSNorm gamma), [#2558](https://github.com/mudler/vllm.cpp/issues/2558) (EXL3 `suh`, -`svh` and trellis). +`svh` and the trellis), [#2578](https://github.com/mudler/vllm.cpp/issues/2578) +(`WeightF32`, found by this row's own green run). Class predecessors: [#301](https://github.com/mudler/vllm.cpp/issues/301), which left the `vt::LoadUnaligned` seam; @@ -27,6 +28,16 @@ already use: | `vt::cpu::WidenRowToF32` | `src/vt/cpu/cpu_matmul_elem.cpp:577` | bf16 RMSNorm gamma (#2540) | | `HadRowBlock` / `HadRows` | `src/vt/cpu/cpu_exl3_kernels.cpp:130` | EXL3 `suh` / `svh` (#2558) | | `TileWord32` | `src/vt/cpu/cpu_exl3_dequant.cpp:64` | EXL3 trellis, kI8 read as `uint16_t` (#2558) | +| `WeightF32` | `src/vllm/model_executor/models/qwen3_5.cpp:1028` | a bf16 attention weight widened for the f32 kernels (#2578) | + +**The fourth row was not in the dispatch, and it is not scope creep.** +`-fno-sanitize-recover=all` stops the binary at the FIRST report, so each repair +in this class reveals the next site rather than a clean run. Repairing the EXL3 +operands made `test_qwen35_exl3` abort one frame later, in `WeightF32`, reached +from `ModelRegistry::Forward` through `FullAttnBlockPaged`. It is the same +defect, the same idiom and the same test, so it is fixed in this flow with its +own issue, as AGENTS.md's in-flow rule requires. The target state is not reached +without it. The pointer chains that feed those three from a borrowed mapping move to byte arithmetic in the same change, because advancing a misaligned `uint16_t*` is @@ -130,6 +141,14 @@ two halves with `vt::LoadUnaligned`. `Exl3TileCodeword`, `const void*` for the trellis, and every walk over it advances a `const unsigned char*` by `tile_words * sizeof(uint16_t)` bytes. +**`WeightF32`.** The same byte cursor plus `vt::LoadUnaligned` over +`w.bytes.data()`, which is already a `const uint8_t*` — the `reinterpret_cast` +to `const uint16_t*` was the only thing making the load undefined. It has no +unit case and gets none: it lives in an anonymous namespace in `qwen3_5.cpp` and +nothing outside that file can name it. Its gate is `test_qwen35_exl3` under the +sanitizer, which reaches it through `ModelRegistry::Forward`, and that is the +stronger gate — a production entry point rather than a hand-constructed operand. + **That `sizeof(uint16_t)` is the whole risk of this change**, exactly as it was for `CopyRawNK` in the class spec: the tile stride counts 16-bit WORDS and the new cursor counts BYTES. It is what the value gate below targets, and it is why @@ -206,6 +225,17 @@ this row. * Stop and report on ENOSPC or an OOM symptom rather than pressing on. The box has both histories. +## Owed + +* [#2579](https://github.com/mudler/vllm.cpp/issues/2579). A grep at the base SHA + finds 20 further `reinterpret_cast(.bytes.data())` + across ten model files (`gemma4_moe.cpp` alone has 10). Whether any of them is + actually reached with an odd base is UNMEASURED, so the list is a population of + the SHAPE and not a list of findings; turning it into findings needs a run per + model rather than a grep. Fixing 20 sites across ten models is also not the unit + of work #2540 and #2558 describe, and each wants its own model suite green + beside it. Filed rather than swept, and named here so the debt is visible. + ## Now `ACTIVE`. diff --git a/include/vt/ops.h b/include/vt/ops.h index 557d419d6..582453c6b 100644 --- a/include/vt/ops.h +++ b/include/vt/ops.h @@ -5657,7 +5657,16 @@ void SharedExpertGate(Queue& q, Tensor& out, const Tensor& sd, const Tensor& gl) // `tile` is the tile's 16*bits int16 words as stored. Mirrors `dq` // (`exl3_dq.cuh:15-31`): the window ENDS at weight t, so t's own `bits` bits sit // in the low positions and weights t-1, t-2 … wrap around the tile above them. -uint16_t Exl3TileCodeword(const uint16_t* tile, int bits, int t); +// +// WHY `const void*` AND NOT `const uint16_t*` HERE AND IN THE THREE BELOW. A +// trellis is a borrowed safetensors payload typed kI8 (`vt::Exl3Gemm`: "the +// trellis travels as opaque i8 BYTES"), so its base is 1-byte aligned and lands +// on an odd address in roughly half of all checkpoints. Forming a +// `const uint16_t*` over it is undefined and `-fsanitize=alignment` aborts on +// the load (#2558); these decoders read it through `vt::LoadUnaligned` off a +// byte cursor instead. Every caller still passes the int16 words as stored, and +// a `uint16_t*` converts implicitly, so no call site changed. +uint16_t Exl3TileCodeword(const void* tile, int bits, int t); // The MCG codebook (cb == 1), three instructions (`codebook.cuh:67-75`): // `x *= 0xCBAC1FED; x = (x & 0x8fff8fff) ^ 0x3b603b60;` then the two fp16 @@ -5700,12 +5709,12 @@ float Exl3DecodeCodeword(uint16_t codeword, int codebook); int Exl3TileRowMajorIndex(int t); // Decode one packed tile into 256 f32 values in ROW-MAJOR 16x16 order. -void Exl3DecodeTile(const uint16_t* tile, int bits, int codebook, float* out256); +void Exl3DecodeTile(const void* tile, int bits, int codebook, float* out256); // `LinearEXL3.get_inner_weight_tensor` (`exl3.py:222-225`): the pre-Hadamard // reconstruct. `out` is f32 [k, n] row-major and holds exact fp16 codebook // values. `k` and `n` must be multiples of 16. -void Exl3ReconstructInner(const uint16_t* trellis, int64_t k, int64_t n, int bits, int codebook, +void Exl3ReconstructInner(const void* trellis, int64_t k, int64_t n, int bits, int codebook, float* out); // `LinearEXL3.get_weight_tensor` (`exl3.py:227-237`): the full dequantized @@ -5720,8 +5729,8 @@ void Exl3ReconstructInner(const uint16_t* trellis, int64_t k, int64_t n, int bit // performs after each transform (`quantize.py:342-346,351-355` `.to(x_dtype)`) // absorbs it for all but a fraction of entries, and MODEL-DSV4-EXL3 W2's device // parity gate is stated against THIS function, not against torch. -void Exl3DequantLinear(const uint16_t* trellis, const uint16_t* suh, - const uint16_t* svh, int64_t k, int64_t n, int bits, int codebook, +void Exl3DequantLinear(const void* trellis, const void* suh, + const void* svh, int64_t k, int64_t n, int bits, int codebook, float* out); // ─── EXL3 device kernels — MODEL-DSV4-EXL3 W2a / W2b ───────────────────────── diff --git a/src/vllm/model_executor/models/qwen3_5.cpp b/src/vllm/model_executor/models/qwen3_5.cpp index 269154c86..cd68416ba 100644 --- a/src/vllm/model_executor/models/qwen3_5.cpp +++ b/src/vllm/model_executor/models/qwen3_5.cpp @@ -66,6 +66,7 @@ #include "vt/dtype.h" #include "vt/ops.h" #include "vt/recipes.h" +#include "vt/unaligned.h" #ifdef VT_MARLIN_NVFP4 #include "vt/cuda/marlin_repack.h" #endif @@ -1024,11 +1025,23 @@ float Silu(float x) { return x / (1.0F + std::exp(-x)); } // conv kernels require the weight dtype to match the activation dtype; where // activations are f32 (GDN conv/gated-norm, attention qk-norm, final-norm // replay), the bf16 weight must be presented as f32. +// +// `w.bytes` MAY BE A BORROWED MAPPING AT AN ODD ADDRESS, so the bytes are read +// through `vt::LoadUnaligned` off a byte cursor rather than a `const uint16_t*`. +// A safetensors payload starts at `8 + ` +// (`safetensors_reader.cpp:78`), a header length is arbitrary, and +// `BorrowStTensorBytes` hands those bytes over verbatim. This site aborted +// `test_qwen35_exl3` under `-fsanitize=alignment`, reached from +// `ModelRegistry::Forward` through `FullAttnBlockPaged`, and it was hidden +// behind the EXL3 finding because that lane stops at the first report (#2578, +// the sixth recurrence of the class in +// `.agents/specs/unaligned-safetensors-consumers.md`). std::vector WeightF32(const OwnedTensor& w) { - const auto* src = reinterpret_cast(w.bytes.data()); + const auto* src = w.bytes.data(); const int64_t n = w.Numel(); std::vector out(static_cast(n)); - for (int64_t i = 0; i < n; ++i) out[static_cast(i)] = vt::BF16ToF32(src[i]); + for (int64_t i = 0; i < n; ++i) + out[static_cast(i)] = vt::BF16ToF32(vt::LoadUnaligned(src + i * 2)); return out; } diff --git a/src/vt/cpu/cpu_exl3_dequant.cpp b/src/vt/cpu/cpu_exl3_dequant.cpp index 873ab6187..bf5dc1549 100644 --- a/src/vt/cpu/cpu_exl3_dequant.cpp +++ b/src/vt/cpu/cpu_exl3_dequant.cpp @@ -50,6 +50,7 @@ #include "vt/dtype.h" #include "vt/ops.h" +#include "vt/unaligned.h" namespace vt { namespace { @@ -60,9 +61,22 @@ constexpr int kHadDim = 128; // The tile's uint32 view (exl3_dq.cuh:25-26 `ptr[...]`), assembled by hand so // the trellis may sit at any alignment inside a safetensors mmap. -inline uint32_t TileWord32(const uint16_t* tile, int index) { - return static_cast(tile[2 * index]) | - (static_cast(tile[2 * index + 1]) << 16); +// +// THE COMMENT ABOVE WAS THE CONTRACT AND THE CODE DID NOT KEEP IT. Assembling +// the uint32 from two halves removes the 4-byte requirement and leaves the +// 2-byte one, and `tile[2 * index]` still indexes a `const uint16_t*`. A +// safetensors payload starts at `8 + ` +// (`safetensors_reader.cpp:78`), a header length is arbitrary, and +// `vt::Exl3Gemm` types the trellis as kI8 — opaque BYTES, alignment +// requirement 1 — so an odd base is not an exotic case here but the expected +// one. `-fsanitize=alignment` aborted `test_qwen35_exl3` on it (#2558). The +// cursor is therefore a byte cursor and both halves come through +// `vt::LoadUnaligned`, which at `-O2` is the same load the raw index compiled +// to. See `.agents/specs/unaligned-safetensors-consumers.md`. +inline uint32_t TileWord32(const void* tile, int index) { + const auto* bytes = static_cast(tile) + index * 4; + return static_cast(LoadUnaligned(bytes)) | + (static_cast(LoadUnaligned(bytes + 2)) << 16); } inline float RoundHalf(float v) { return F16ToF32(F32ToF16(v)); } @@ -89,7 +103,7 @@ void Fwht128(float* base, int64_t stride, int64_t lanes) { } // namespace -uint16_t Exl3TileCodeword(const uint16_t* tile, int bits, int t) { +uint16_t Exl3TileCodeword(const void* tile, int bits, int t) { // exl3_dq.cuh:18-29, verbatim. `+ 256*bits` is upstream's way of keeping the // tail-biting wrap non-negative; the `% (bits*256/32)` on the word index is // the wrap itself. @@ -179,7 +193,7 @@ int Exl3TileRowMajorIndex(int t) { return r * 16 + c; } -void Exl3DecodeTile(const uint16_t* tile, int bits, int codebook, float* out256) { +void Exl3DecodeTile(const void* tile, int bits, int codebook, float* out256) { VT_CHECK(bits >= 1 && bits <= 8, "exl3: bits must be in [1, 8]; got " + std::to_string(bits)); for (int t = 0; t < 256; ++t) { @@ -188,7 +202,7 @@ void Exl3DecodeTile(const uint16_t* tile, int bits, int codebook, float* out256) } } -void Exl3ReconstructInner(const uint16_t* trellis, int64_t k, int64_t n, int bits, int codebook, +void Exl3ReconstructInner(const void* trellis, int64_t k, int64_t n, int bits, int codebook, float* out) { VT_CHECK(bits >= 1 && bits <= 8, "exl3: bits must be in [1, 8]; got " + std::to_string(bits)); @@ -197,11 +211,14 @@ void Exl3ReconstructInner(const uint16_t* trellis, int64_t k, int64_t n, int bit "tile is 16x16); got k=" + std::to_string(k) + " n=" + std::to_string(n)); const int64_t tiles_k = k / 16; const int64_t tiles_n = n / 16; - const int64_t tile_words = 16 * bits; + // BYTES, not words: `16 * bits` counts int16 words, so the cursor carries the + // `sizeof(uint16_t)` the `const uint16_t*` used to supply (#2558). + const int64_t tile_bytes = 16 * static_cast(bits) * static_cast(sizeof(uint16_t)); + const auto* tw = static_cast(trellis); float tile_out[256]; for (int64_t i = 0; i < tiles_k; ++i) { for (int64_t j = 0; j < tiles_n; ++j) { - Exl3DecodeTile(trellis + (i * tiles_n + j) * tile_words, bits, codebook, tile_out); + Exl3DecodeTile(tw + (i * tiles_n + j) * tile_bytes, bits, codebook, tile_out); for (int r = 0; r < 16; ++r) { std::memcpy(out + (i * 16 + r) * n + j * 16, tile_out + r * 16, 16 * sizeof(float)); @@ -210,8 +227,8 @@ void Exl3ReconstructInner(const uint16_t* trellis, int64_t k, int64_t n, int bit } } -void Exl3DequantLinear(const uint16_t* trellis, const uint16_t* suh, - const uint16_t* svh, int64_t k, int64_t n, int bits, int codebook, +void Exl3DequantLinear(const void* trellis, const void* suh, + const void* svh, int64_t k, int64_t n, int bits, int codebook, float* out) { VT_CHECK(k % kHadDim == 0 && n % kHadDim == 0, "exl3: both features must be multiples of 128 (each side was " @@ -257,7 +274,7 @@ void Exl3DequantLinear(const uint16_t* trellis, const uint16_t* suh, // w *= suh[:, None] (exl3.py:233) — an fp16 multiply. for (int64_t i = 0; i < k; ++i) { - const float s = F16ToF32(suh[i]); + const float s = F16ToF32(LoadUnaligned(static_cast(suh) + i * 2)); float* row = out + i * n; for (int64_t j = 0; j < n; ++j) row[j] = RoundHalf(row[j] * s); } @@ -277,7 +294,9 @@ void Exl3DequantLinear(const uint16_t* trellis, const uint16_t* suh, // w *= svh[None, :] (exl3.py:235). for (int64_t i = 0; i < k; ++i) { float* row = out + i * n; - for (int64_t j = 0; j < n; ++j) row[j] = RoundHalf(row[j] * F16ToF32(svh[j])); + for (int64_t j = 0; j < n; ++j) + row[j] = RoundHalf( + row[j] * F16ToF32(LoadUnaligned(static_cast(svh) + j * 2))); } } diff --git a/src/vt/cpu/cpu_exl3_kernels.cpp b/src/vt/cpu/cpu_exl3_kernels.cpp index 54497192a..5665b1b1e 100644 --- a/src/vt/cpu/cpu_exl3_kernels.cpp +++ b/src/vt/cpu/cpu_exl3_kernels.cpp @@ -32,6 +32,7 @@ #include "vt/dtype.h" #include "vt/op_provider.h" #include "vt/ops.h" +#include "vt/unaligned.h" namespace vt::cpu { namespace { @@ -120,14 +121,35 @@ constexpr bool HadHalfOut(HadIo io) { return io == HadIo::kHalfHalf || io == HadIo::kFloatHalf; } -void HadRowBlock(HadIo io, const void* in, void* out, const uint16_t* pre, const uint16_t* post, +// THE SCALES ARE WEIGHTS AND MAY SIT AT ANY BYTE. `pre` and `post` are `suh` +// and `svh`, which `BorrowStTensorBytes` hands over as the mapping's own bytes. +// A safetensors payload starts at `8 + ` +// (`safetensors_reader.cpp:78`) and a header length is arbitrary, so an F16 +// scale vector begins on an odd byte in roughly half of all checkpoints. That is +// an ordinary file, not a corrupt one. They are therefore `const void*` here and +// are read one element at a time with `vt::LoadUnaligned`: forming a +// `const uint16_t*` over an odd address is undefined even where x86 executes +// the load, and `-fsanitize=alignment` aborted `test_qwen35_exl3` on exactly +// this operand (#2558). The load is free — at `-O2` it is the same `movzwl` +// the raw index compiled to. +// +// `in` and `out` KEEP their typed pointers, and the asymmetry is the point: +// they are the activation and the engine's own scratch, which +// `cpu_backend.cpp:20,42` allocates through `std::aligned_alloc(64, ...)`. This +// is the polarity `cpu_layernorm.cpp:44-46` states — an output is an +// engine-allocated buffer, never a borrowed file mapping. +inline uint16_t ScaleAt(const void* scales, int64_t i) { + return LoadUnaligned(static_cast(scales) + i * 2); +} + +void HadRowBlock(HadIo io, const void* in, void* out, const void* pre, const void* post, float r_scale, int64_t block_base) { float buf[128]; if (HadHalfIn(io)) { const uint16_t* p = static_cast(in); for (int i = 0; i < 128; ++i) { // pre_scale rides an fp16 multiply BEFORE the widen (hadamard_inner.cuh:112-114). - const uint16_t v = pre != nullptr ? MulF16(p[i], pre[block_base + i]) : p[i]; + const uint16_t v = pre != nullptr ? MulF16(p[i], ScaleAt(pre, block_base + i)) : p[i]; buf[i] = F16ToF32(v); } } else { @@ -135,7 +157,7 @@ void HadRowBlock(HadIo io, const void* in, void* out, const uint16_t* pre, const for (int i = 0; i < 128; ++i) { // the float inners widen the fp16 scale and multiply in f32 // (hadamard_inner.cuh:171-174). - buf[i] = pre != nullptr ? p[i] * F16ToF32(pre[block_base + i]) : p[i]; + buf[i] = pre != nullptr ? p[i] * F16ToF32(ScaleAt(pre, block_base + i)) : p[i]; } } @@ -145,19 +167,19 @@ void HadRowBlock(HadIo io, const void* in, void* out, const uint16_t* pre, const if (!HadHalfOut(io)) { float* o = static_cast(out); for (int i = 0; i < 128; ++i) - o[i] = post != nullptr ? res[i] * F16ToF32(post[block_base + i]) : res[i]; + o[i] = post != nullptr ? res[i] * F16ToF32(ScaleAt(post, block_base + i)) : res[i]; } else { // Both half-output inners round FIRST and apply the post-scale as an fp16 // multiply afterwards (hadamard_inner.cuh:137-146 and :264-278). uint16_t* o = static_cast(out); for (int i = 0; i < 128; ++i) { const uint16_t r = F32ToF16(res[i]); - o[i] = post != nullptr ? MulF16(r, post[block_base + i]) : r; + o[i] = post != nullptr ? MulF16(r, ScaleAt(post, block_base + i)) : r; } } } -void HadRows(HadIo io, const void* in, void* out, const uint16_t* pre, const uint16_t* post, +void HadRows(HadIo io, const void* in, void* out, const void* pre, const void* post, float r_scale, int64_t rows, int64_t cols) { const bool half_in = HadHalfIn(io); const bool half_out = HadHalfOut(io); @@ -177,8 +199,10 @@ void Exl3HadR128KernelCpu(Queue& q, Tensor& out, const Tensor& in, const Exl3Had const int64_t rows = in.shape[0]; const int64_t cols = in.shape[1]; if (rows == 0 || cols == 0) return; - const uint16_t* pre = args.pre_scale != nullptr ? args.pre_scale->Ptr() : nullptr; - const uint16_t* post = args.post_scale != nullptr ? args.post_scale->Ptr() : nullptr; + // `.data`, not `Ptr()`: these scales may be borrowed mapping bytes + // at an odd address, so no typed pointer is formed over them at all. + const void* pre = args.pre_scale != nullptr ? args.pre_scale->data : nullptr; + const void* post = args.post_scale != nullptr ? args.post_scale->data : nullptr; const float r_scale = args.scale * kInvSqrt128; // hadamard.cu:107 HadRows(in.dtype == DType::kF16 ? HadIo::kHalfHalf : HadIo::kFloatFloat, in.data, out.data, pre, post, r_scale, rows, cols); @@ -204,13 +228,18 @@ void Exl3GemmKernelCpu(Queue& q, Tensor& c, const Tensor& a, const Tensor& trell if (m == 0 || k == 0 || n == 0) return; // 1. the input transform, into the caller's scratch (which may alias A). - HadRows(HadIo::kHalfHalf, a.data, a_had.data, suh.Ptr(), nullptr, kInvSqrt128, m, k); + HadRows(HadIo::kHalfHalf, a.data, a_had.data, suh.data, nullptr, kInvSqrt128, m, k); // 2. the matmul against the decoded trellis, f32 accumulators. const uint16_t* ah = a_had.Ptr(); - const uint16_t* tw = trellis.Ptr(); + // The trellis is kI8 — opaque BYTES whose alignment requirement is 1 — and it + // is borrowed straight out of the mapping, so the tile cursor advances in + // bytes. `tile_bytes` is `tile_words * sizeof(uint16_t)`, and that factor of + // two is the one thing this cursor must not lose (#2558). + const auto* tw = static_cast(trellis.data); const int64_t tiles_n = n / 16; const int64_t tile_words = 16 * static_cast(args.bits); + const int64_t tile_bytes = tile_words * static_cast(sizeof(uint16_t)); std::vector raw(static_cast(m) * static_cast(n), 0.0f); // PARALLEL OVER OUTPUT TILES, and the loop order is inverted for it: `tj` @@ -230,7 +259,7 @@ void Exl3GemmKernelCpu(Queue& q, Tensor& c, const Tensor& a, const Tensor& trell float tile[256]; for (int64_t tj = j0; tj < j1; ++tj) { for (int64_t ti = 0; ti < k / 16; ++ti) { - Exl3DecodeTile(tw + (ti * tiles_n + tj) * tile_words, args.bits, args.codebook, tile); + Exl3DecodeTile(tw + (ti * tiles_n + tj) * tile_bytes, args.bits, args.codebook, tile); for (int64_t r = 0; r < m; ++r) { float* orow = &raw[static_cast(r * n + tj * 16)]; for (int rr = 0; rr < 16; ++rr) { @@ -247,7 +276,7 @@ void Exl3GemmKernelCpu(Queue& q, Tensor& c, const Tensor& a, const Tensor& trell // 3. the output transform. The device holds this tile in f32 shared memory and // finishes with had_ff (f32 C) or had_fh (fp16 C) — the same two arms here. HadRows(c.dtype == DType::kF32 ? HadIo::kFloatFloat : HadIo::kFloatHalf, raw.data(), c.data, - nullptr, svh.Ptr(), kInvSqrt128, m, n); + nullptr, svh.data, kInvSqrt128, m, n); } // ── the fused MoE MLP, CPU arm (exl3_moe_kernel.cuh:17-283) ────────────────── @@ -273,16 +302,19 @@ void Exl3GemmKernelCpu(Queue& q, Tensor& c, const Tensor& a, const Tensor& trell // the down GEMM's load; the loop arm widens to f32, activates in f32 and rounds // back. Same algebra, different rounding, and the spec's tier 4 is the bound on // the difference. -void MoeGemm(const uint16_t* a_had, const uint16_t* trellis, float* raw, int64_t m, int64_t k, +void MoeGemm(const uint16_t* a_had, const void* trellis, float* raw, int64_t m, int64_t k, int64_t n, int bits, int codebook) { - // The same tile walk `Exl3GemmKernelCpu` step 2 performs, over an m-row batch. + // The same tile walk `Exl3GemmKernelCpu` step 2 performs, over an m-row batch, + // and the same byte cursor for the same reason: an expert's trellis is kI8 + // borrowed straight out of the mapping (#2558). const int64_t tiles_n = n / 16; - const int64_t tile_words = 16 * static_cast(bits); + const int64_t tile_bytes = 16 * static_cast(bits) * static_cast(sizeof(uint16_t)); + const auto* tw = static_cast(trellis); float tile[256]; for (int64_t i = 0; i < m * n; ++i) raw[i] = 0.0f; for (int64_t ti = 0; ti < k / 16; ++ti) { for (int64_t tj = 0; tj < tiles_n; ++tj) { - Exl3DecodeTile(trellis + (ti * tiles_n + tj) * tile_words, bits, codebook, tile); + Exl3DecodeTile(tw + (ti * tiles_n + tj) * tile_bytes, bits, codebook, tile); for (int64_t r = 0; r < m; ++r) { float* orow = &raw[r * n + tj * 16]; for (int rr = 0; rr < 16; ++rr) { @@ -371,8 +403,11 @@ void Exl3MoeMlpKernelCpu(Queue& q, Tensor& output_state, const Tensor& hidden_st const uint16_t* hid = hidden_state.Ptr(); float* out = output_state.Ptr(); - auto table = [](const Tensor* tt, int64_t e) -> const uint16_t* { - return reinterpret_cast( + // `const void*`, not `const uint16_t*`: every one of these is a borrowed + // safetensors payload and may begin at an odd byte, so no typed pointer is + // formed over one (#2558). The callees below take byte cursors. + auto table = [](const Tensor* tt, int64_t e) -> const void* { + return reinterpret_cast( static_cast(tt->Ptr()[e])); }; @@ -398,15 +433,15 @@ void Exl3MoeMlpKernelCpu(Queue& q, Tensor& output_state, const Tensor& hidden_st // per-expert path covers the second case, exactly as upstream's does. if (tokens == 0 || tokens > max_rows) continue; - const uint16_t* g_tr = table(tables.gate_trellis, e); - const uint16_t* g_su = table(tables.gate_suh, e); - const uint16_t* g_sv = table(tables.gate_svh, e); - const uint16_t* u_tr = table(tables.up_trellis, e); - const uint16_t* u_su = table(tables.up_suh, e); - const uint16_t* u_sv = table(tables.up_svh, e); - const uint16_t* d_tr = table(tables.down_trellis, e); - const uint16_t* d_su = table(tables.down_suh, e); - const uint16_t* d_sv = table(tables.down_svh, e); + const void* g_tr = table(tables.gate_trellis, e); + const void* g_su = table(tables.gate_suh, e); + const void* g_sv = table(tables.gate_svh, e); + const void* u_tr = table(tables.up_trellis, e); + const void* u_su = table(tables.up_suh, e); + const void* u_sv = table(tables.up_svh, e); + const void* d_tr = table(tables.down_trellis, e); + const void* d_su = table(tables.down_suh, e); + const void* d_sv = table(tables.down_svh, e); // stage 1: gather + input Hadamard, one 128-block per warp upstream. for (int64_t r = 0; r < tokens; ++r) { diff --git a/src/vt/cpu/cpu_matmul_elem.cpp b/src/vt/cpu/cpu_matmul_elem.cpp index 1b0dfc165..26dec8576 100644 --- a/src/vt/cpu/cpu_matmul_elem.cpp +++ b/src/vt/cpu/cpu_matmul_elem.cpp @@ -6,6 +6,7 @@ #include "vt/cpu/cpu_isa_arm.h" #include "vt/cpu/cpu_isa_x86.h" #include "vt/quant.h" +#include "vt/unaligned.h" #include #include @@ -562,19 +563,39 @@ bool ElemGemmUseRef() { return v; } +// `src` MAY BE ODD, and on the model path it routinely is. A safetensors +// payload starts at `8 + ` (`safetensors_reader.cpp:78`) +// and a header length is arbitrary, so a 16-bit weight begins on an odd byte in +// roughly half of all checkpoints. `BorrowStTensorBytes` then hands those bytes +// to the kernel verbatim, which is the whole point of direct upload. Forming a +// `const uint16_t*` over that address is undefined even where x86 executes the +// load, and `-fsanitize=alignment` aborts on it: the bf16 RMSNorm gamma of +// `dots3_note` and `muse_glimmer` reached here at an odd address and took the +// `sanitize-cpu` lane down (#2540). +// +// The byte cursor plus `vt::LoadUnaligned` is the shape `vt::cpu::LoadF32` +// (cpu_ops.cpp:32) and `cpu_layernorm.cpp:47` already use, and it is free: at +// `-O2` it compiles to the same `movzwl (%rdi,%rax,2)` the raw cast did, with +// no `memcpy` call emitted (measured for `FIX-UNALIGNED-LOADERS-772`). Fixing +// the CONSUMER rather than refusing the borrow is deliberate — a producer-side +// alignment gate would switch direct upload off for every tensor in half of all +// checkpoints. See `.agents/specs/unaligned-safetensors-consumers.md`. +// +// The kF32 arm already read its bytes through `std::memcpy` and is unchanged. void WidenRowToF32(DType dt, const void* src, int64_t n, float* dst) { + const auto* bytes = static_cast(src); switch (dt) { case DType::kF32: std::memcpy(dst, src, static_cast(n) * sizeof(float)); break; case DType::kF16: { - const uint16_t* s = static_cast(src); - for (int64_t i = 0; i < n; ++i) dst[i] = F16ToF32(s[i]); + for (int64_t i = 0; i < n; ++i) + dst[i] = F16ToF32(LoadUnaligned(bytes + i * 2)); break; } case DType::kBF16: { - const uint16_t* s = static_cast(src); - for (int64_t i = 0; i < n; ++i) dst[i] = BF16ToF32(s[i]); + for (int64_t i = 0; i < n; ++i) + dst[i] = BF16ToF32(LoadUnaligned(bytes + i * 2)); break; } default: diff --git a/tests/vt/test_exl3_gemm.cpp b/tests/vt/test_exl3_gemm.cpp index 92c3f12c3..2cbfffd43 100644 --- a/tests/vt/test_exl3_gemm.cpp +++ b/tests/vt/test_exl3_gemm.cpp @@ -701,3 +701,87 @@ TEST_CASE("exl3 device: the widened (bits, codebook) arms agree with the CPU arm } cb_dev.DestroyQueue(dq); } + +// ─── The operands' BYTE ADDRESSES — #2558 ──────────────────────────────────── +// +// `suh`, `svh` and the trellis are WEIGHTS, and the borrow path hands the kernel +// the mapping's own bytes. A safetensors payload starts at +// `8 + ` (`safetensors_reader.cpp:78`) and a header length is +// arbitrary, so any of the three begins on an ODD byte in roughly half of all +// checkpoints. The trellis is the sharpest case: `vt::Exl3Gemm` types it kI8 -- +// "opaque i8 BYTES" -- so its alignment requirement is 1 and an odd base is the +// expected case rather than an exotic one. +// +// `HadRowBlock` (`cpu_exl3_kernels.cpp:130`) read `suh`/`svh` through a +// `const uint16_t*` and `TileWord32` (`cpu_exl3_dequant.cpp:64`) read the trellis +// the same way, and `-fsanitize=alignment` aborted `test_qwen35_exl3` on the +// first of those, reached through `ModelRegistry::Forward`. +// +// THE FIX IS IN THE CONSUMER, NOT THE PRODUCER, and this case is what separates +// the two. Refusing to borrow a misaligned tensor also takes the sanitizer green +// -- and switches direct upload off for every tensor in half of all checkpoints, +// because the parity of the payload base is a coin flip per FILE. +// `tests/vllm/test_load_direct_upload.cpp` and +// `tests/vllm/models/test_qwen3_5_dense_load_residency.cpp` hold that lever; this +// case holds the correctness, and both must be green at once. +// +// The bar is BYTE EQUALITY against the same operands at even addresses. The +// repair replaces word arithmetic with byte arithmetic over the trellis, and a +// stride that lost its `sizeof(uint16_t)` decodes a different weight -- which a +// tolerance over a random-bit trellis could plausibly absorb and an equality +// cannot. Each `REQUIRE` on a parity is the instrument's own precondition: a run +// that quietly landed on an even address would exercise nothing and still pass. +TEST_CASE("exl3 gemm: suh, svh and the trellis at ODD byte addresses decode the same") { + vt::Queue q = CpuQueue(); + const int64_t m = 2, k = 256, n = 128; + const Exl3Fixture f = MakeFixture(k, n, 3, 0x2545F491u); + + Rng rng; + rng.s = 0x27220A95u; + std::vector a_h(static_cast(m * k)); + for (auto& v : a_h) v = vt::F32ToF16(rng.next(1.0f)); + + // `off` bytes of lead-in before each of the three weight payloads. 0 is the + // aligned control; 1 is what a 171-byte header produces. + auto run = [&](size_t off) { + const size_t tb_bytes = f.trellis.size() * sizeof(uint16_t); + const size_t su_bytes = f.suh.size() * sizeof(uint16_t); + const size_t sv_bytes = f.svh.size() * sizeof(uint16_t); + std::vector tb_raw(tb_bytes + off, 0), su_raw(su_bytes + off, 0), + sv_raw(sv_bytes + off, 0); + std::memcpy(tb_raw.data() + off, f.trellis.data(), tb_bytes); + std::memcpy(su_raw.data() + off, f.suh.data(), su_bytes); + std::memcpy(sv_raw.data() + off, f.svh.data(), sv_bytes); + void* tb_p = tb_raw.data() + off; + void* su_p = su_raw.data() + off; + void* sv_p = sv_raw.data() + off; + REQUIRE(reinterpret_cast(tb_p) % 2 == off % 2); + REQUIRE(reinterpret_cast(su_p) % 2 == off % 2); + REQUIRE(reinterpret_cast(sv_p) % 2 == off % 2); + + std::vector c_h(static_cast(m * n), 0); + std::vector a_had(static_cast(m * k), 0); + vt::Tensor ta = vt::Tensor::Contiguous(a_h.data(), vt::DType::kF16, q.device, {m, k}); + vt::Tensor tb = vt::Tensor::Contiguous(tb_p, vt::DType::kI8, q.device, + {k / 16, n / 16, 32 * f.bits}); + vt::Tensor tsuh = vt::Tensor::Contiguous(su_p, vt::DType::kF16, q.device, {k}); + vt::Tensor tsvh = vt::Tensor::Contiguous(sv_p, vt::DType::kF16, q.device, {n}); + vt::Tensor tc = vt::Tensor::Contiguous(c_h.data(), vt::DType::kF16, q.device, {m, n}); + vt::Tensor tah = vt::Tensor::Contiguous(a_had.data(), vt::DType::kF16, q.device, {m, k}); + vt::Exl3GemmArgs args; + args.bits = f.bits; + args.codebook = 1; + vt::Exl3Gemm(q, tc, ta, tb, tsuh, tsvh, tah, args); + return c_h; + }; + + const std::vector aligned = run(0); + // The control must have DECODED something. An all-zero output would satisfy the + // equality below without either arm reading a single trellis byte. + size_t nonzero = 0; + for (const uint16_t v : aligned) + if (v != 0) ++nonzero; + REQUIRE(nonzero > aligned.size() / 2); + + CHECK(run(1) == aligned); +} diff --git a/tests/vt/test_ops_rmsnorm_weight_dtype.cpp b/tests/vt/test_ops_rmsnorm_weight_dtype.cpp index 69dee6e0b..102154e2c 100644 --- a/tests/vt/test_ops_rmsnorm_weight_dtype.cpp +++ b/tests/vt/test_ops_rmsnorm_weight_dtype.cpp @@ -409,3 +409,67 @@ TEST_CASE("rmsnorm: CUDA serves a kF16 gamma and agrees with CPU byte-for-byte") CHECK(RunBf16ActW16(gpu, xb, w16, DType::kF16, kIdxH) == RunBf16ActW16(Cpu(), xb, w16, DType::kF16, kIdxH)); } + +// ───────────────────────────────────────────────────────────────────────────── +// CASE 4. THE GAMMA'S BYTE ADDRESS. #2540. +// +// A safetensors payload starts at `8 + ` +// (`safetensors_reader.cpp:78`) and a header length is arbitrary, so a bf16 +// gamma begins on an ODD byte in roughly half of all checkpoints. +// `BorrowStTensorBytes` hands those bytes to the kernel verbatim -- that is what +// direct upload IS -- and `WidenRowToF32` (`cpu_matmul_elem.cpp:577`) read them +// through a `const uint16_t*`. `-fsanitize=alignment` aborted `test_dots3_note_attn` +// and `test_muse_glimmer_text` on exactly this operand, reached through +// `ModelRegistry::Forward` on both. +// +// TWO THINGS ARE GATED HERE AND THEY FAIL DIFFERENTLY. +// +// * Under `sanitize-cpu (address,undefined)` the pre-fix binary ABORTS in this +// case with `load of misaligned address ... for type 'const uint16_t'`. That +// is the lane's red. +// * Without a sanitizer the pre-fix binary PASSES on x86, which is why this +// class survived three UBSan sweeps. The equality below is therefore aimed at +// the byte-cursor arithmetic the fix introduces: an element stride counted in +// words instead of bytes reads the wrong half of the buffer and lands here on +// any lane. +// +// The `REQUIRE` on the parity is not decoration. A case that quietly landed on an +// EVEN address would exercise nothing and still report a pass, which is the +// "instrument that never ran" trap. +TEST_CASE("rmsnorm: a bf16 gamma at an ODD byte address is read as its bytes") { + const std::vector x = SpreadX(kRows * kProdH); + const std::vector wf = SpreadW(kProdH); + std::vector wb(static_cast(kProdH)); + for (int64_t j = 0; j < kProdH; ++j) + wb[static_cast(j)] = F32ToBf16(wf[static_cast(j)]); + + // The same gamma bytes, one byte into an over-allocated buffer. + std::vector raw(wb.size() * sizeof(uint16_t) + 1, 0); + std::memcpy(raw.data() + 1, wb.data(), wb.size() * sizeof(uint16_t)); + void* odd = raw.data() + 1; + REQUIRE(reinterpret_cast(odd) % 2 == 1); + + std::vector got(static_cast(kRows * kProdH), 0); + Queue q{Cpu(), nullptr}; + Tensor tx = Tensor::Contiguous(const_cast(x.data()), DType::kF32, Cpu(), {kRows, kProdH}); + Tensor tw = Tensor::Contiguous(odd, DType::kBF16, Cpu(), {kProdH}); + Tensor to = Tensor::Contiguous(got.data(), DType::kBF16, Cpu(), {kRows, kProdH}); + vt::RmsNorm(q, to, tx, tw, RmsNormArgs{1e-6f, /*gemma=*/true}); + + // Byte equality against the SAME gamma at an even address. Not a tolerance: a + // stride defect moves whole elements, and a tolerance on a smooth gamma could + // absorb a neighbour's value. + const std::vector aligned = RunF32ActBf16W(Cpu(), x, wb, kProdH); + CHECK(got == aligned); + + // The odd gamma must be LOAD-BEARING, or the equality above would also hold for + // a kernel that read the aligned copy or ignored `w` entirely. + std::vector bumped = raw; + const uint16_t hot = F32ToBf16(Bf16ToF32(wb[7]) + 4.0f); + std::memcpy(bumped.data() + 1 + 7 * sizeof(uint16_t), &hot, sizeof(hot)); + std::vector got2(static_cast(kRows * kProdH), 0); + Tensor tw2 = Tensor::Contiguous(bumped.data() + 1, DType::kBF16, Cpu(), {kProdH}); + Tensor to2 = Tensor::Contiguous(got2.data(), DType::kBF16, Cpu(), {kRows, kProdH}); + vt::RmsNorm(q, to2, tx, tw2, RmsNormArgs{1e-6f, /*gemma=*/true}); + CHECK(got2[7] != got[7]); +}