diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index 75e4b7b0b..d0ea3d274 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -2233,6 +2233,21 @@ TileW decode_tile(const uint8_t* tile, int k, int rows) { std::vector encode_tile(const TileW& t) { if (t.k <= 0 || t.k % kKBlock) throw std::runtime_error("tile: K is not a positive multiple of 128"); + /* A DEFENSIVE range check on this writer, not a fix for a measured defect -- the distinction matters, and an earlier + * version of this comment got it wrong: it cited #1918, whose mechanism (a pack's two's-complement int4 code landing in + * the container's unsigned field) was the EXTERNAL-TILE writer's, and whose cause was fixed on main by #1874. Nothing + * here ever produced an out-of-range code: the pack legs on two real 1.9 packs are unchanged with this guard in place. + * + * What it does buy: the nibble packer below applies `& 0x0F`, so IF a future caller ever hands this function a code + * outside 0..15, it would wrap a full code range rather than saturate -- a weight one range away that still looks + * plausible. Nobody documents whether that is the caller's job or the writer's, so the writer refuses, naming the + * position and the value, instead of silently packing a different number. A caller that means 15 says 15. */ + for (size_t i = 0; i < t.code.size(); ++i) + if (t.code[i] > 0x0F) + throw std::runtime_error("tile: code " + std::to_string(int(t.code[i])) + " at k=" + std::to_string(t.k) + + " rows=" + std::to_string(t.rows) + ", element " + std::to_string(i) + + ": a 4-bit code is 0..15, and packing it would wrap a full code range " + "(engine#1921: clamp or re-quantise before encoding)"); const int64_t sv = subv_bytes(t.rows); if (sv == 0) throw std::runtime_error("tile: rows is not a positive multiple of 64"); const int nsubv = t.rows / 2, kbs = t.k / kKBlock; diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index acd95805f..132bce709 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -4184,6 +4184,35 @@ void test_qk_norm_refusal() { std::printf(" qk norm: 7 arms (declared+not-found refuses; found-only and neither are allowed)\n"); } +/* engine#1921: the nibble packer applies `& 0x0F`, so a code outside 0..15 WRAPS a full code range instead of + * saturating -- measured as `container == expected mod 4.0` over 32/32 columns of a group, i.e. a wrong weight that still + * looks plausible. The writer now refuses, and these arms pin the two ends of the legal range plus the refusal, because an + * arm that only checked the throw would pass on a writer that refused everything. */ +void test_encode_tile_code_range() { + const auto tile_with = [](int v) { + pi::TileW t; + t.k = 128; // one 128-wide group, the minimum the layout allows + t.rows = 64; + t.code.assign(size_t(t.rows) * size_t(t.k), uint8_t(v)); + t.scale.assign(size_t(t.rows), 1.0f); + t.zp.assign(size_t(t.rows), 0); + t.bias.assign(size_t(t.rows), 0); + return t; + }; + bool ok0 = true, ok15 = true; + try { (void)pi::encode_tile(tile_with(0)); } catch (const std::exception&) { ok0 = false; } + try { (void)pi::encode_tile(tile_with(15)); } catch (const std::exception&) { ok15 = false; } + check(ok0, "encode tile: code 0 (the bottom of the legal range) encodes"); + check(ok15, "encode tile: code 15 (the top of the legal range) encodes"); + std::string why; + try { (void)pi::encode_tile(tile_with(16)); } catch (const std::exception& e) { why = e.what(); } + check(!why.empty(), "encode tile: code 16 is REFUSED, not wrapped to 0"); + check(why.find("engine#1921") != std::string::npos, "encode tile: the refusal carries its issue"); + check(why.find("0..15") != std::string::npos, "encode tile: and states the range it wants"); + check(why.find("wrap") != std::string::npos, "encode tile: and says what would have happened instead"); + std::printf(" encode tile: 6 arms (0 and 15 encode, 16 refuses by name)\n"); +} + // engine#1889: the arithmetic that claims the per-head QK gains out of the 640 B Hunyuan-7B carries between its last qkv // tile and o_proj. A wrong offset there reads tile bytes as bf16 and mis-rotates every pack of the family, so both the // right shape (2 * (64 + head_dim*2) = 640 at head_dim 128) and the neighbours that must NOT be taken for it are driven. @@ -4203,6 +4232,7 @@ void test_qk_gain_offsets() { } int main(int argc, char** argv) { + test_encode_tile_code_range(); test_qk_norm_refusal(); test_qk_gain_offsets(); test_tail_head_placement();