From 5baf20b66b46947dccfa1d3d807aa968eb40dbda Mon Sep 17 00:00:00 2001 From: James Burton Date: Fri, 7 Aug 2026 13:37:37 +0100 Subject: [PATCH 1/3] fix(cpu/matmul): widen GEMV weight row offsets to long (#429) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two GEMV weight-offset expressions computed a row offset with int arithmetic, so a single tensor past the wrap point produced a negative offset and an out-of-bounds read rather than a clean failure. ComputeRows (`weightsQ8 + row * rowBytes`, 7 sites across the AVX-512+VNNI, AVX-512, AVX2 and scalar tiers) is the reachable one: the stride is in bytes (~1.0625 per weight), so the product wraps at 2 GB of tensor. Llama 3.1 405B's LM head — vocab 128256, hidden 16384, rowBytes (16384/32)*34 = 17408 — gives 128256 * 17408 = 2,232,680,448, over int.MaxValue by ~85 MB. The first affected row is 123,362, so the last 4,894 rows (3.8% of vocab) read from a wrapped negative offset on a shipping model. GemvF16 (`weightsHalf + row * k`, 2 sites) counts elements, not bytes, so on that same model m*k = 2,101,346,304 stays ~2.2% under the limit. Widened for consistency, but it needs a tensor larger than any current model to trigger. Only the GEMV paths were exposed. ComputeGemmTiled clamps row < tileRows <= 256 and pre-offsets its base pointer via `(long)mStart * q8RowBytes`, so the tiled path was already safe and is untouched. Activation-buffer offsets are left alone: each indexes a buffer whose size is the same product, so wrapping would need an >8 GB logits allocation. This matches the widening convention already used by GemmF16, GemmF32, ComputeGemmTiled and the F16 tiled row read — the Gemm paths had the cast, the Gemv paths did not. No performance cost. Verified by diffing the JIT's x64 for GemvQ8_0 between builds: the baseline already sign-extended the product (`imul ecx,r12d` then `movsxd rcx,ecx`); the cast moves the widening ahead of the multiply (`imul rcx,rax` on an already-widened row), which is what fixes the overflow. The 4-row fast path is one instruction shorter; total method size 1024 -> 1028 bytes. The arithmetic is in the outer row loop, amortized over 16-128 VNNI block iterations per row. A position-balanced timing harness could not resolve any delta: within-arm spread was 46-88%, dwarfing all measured differences. Reported-by: unsafePtr --- src/DotLLM.Cpu/Kernels/MatMul.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/DotLLM.Cpu/Kernels/MatMul.cs b/src/DotLLM.Cpu/Kernels/MatMul.cs index 8b81bc1e..a6c8a795 100644 --- a/src/DotLLM.Cpu/Kernels/MatMul.cs +++ b/src/DotLLM.Cpu/Kernels/MatMul.cs @@ -138,7 +138,7 @@ internal static void ComputeRows(byte* weightsQ8, byte* xQ8, float* result, int for (; row + 3 < m; row += 4) { VecDotQ8_0Vnni_4Rows( - weightsQ8 + row * rowBytes, + weightsQ8 + (long)row * rowBytes, weightsQ8 + (row + 1) * rowBytes, weightsQ8 + (row + 2) * rowBytes, weightsQ8 + (row + 3) * rowBytes, @@ -146,7 +146,7 @@ internal static void ComputeRows(byte* weightsQ8, byte* xQ8, float* result, int } for (; row < m; row++) { - result[row] = VecDotQ8_0Avx512(weightsQ8 + row * rowBytes, xQ8, blockCount); + result[row] = VecDotQ8_0Avx512(weightsQ8 + (long)row * rowBytes, xQ8, blockCount); } } else if (Avx512BW.IsSupported) @@ -156,7 +156,7 @@ internal static void ComputeRows(byte* weightsQ8, byte* xQ8, float* result, int for (; row + 3 < m; row += 4) { VecDotQ8_0Avx512_4Rows( - weightsQ8 + row * rowBytes, + weightsQ8 + (long)row * rowBytes, weightsQ8 + (row + 1) * rowBytes, weightsQ8 + (row + 2) * rowBytes, weightsQ8 + (row + 3) * rowBytes, @@ -164,7 +164,7 @@ internal static void ComputeRows(byte* weightsQ8, byte* xQ8, float* result, int } for (; row < m; row++) { - result[row] = VecDotQ8_0Avx512(weightsQ8 + row * rowBytes, xQ8, blockCount); + result[row] = VecDotQ8_0Avx512(weightsQ8 + (long)row * rowBytes, xQ8, blockCount); } } else if (Avx2.IsSupported) @@ -174,7 +174,7 @@ internal static void ComputeRows(byte* weightsQ8, byte* xQ8, float* result, int for (; row + 3 < m; row += 4) { VecDotQ8_0Avx2_4Rows( - weightsQ8 + row * rowBytes, + weightsQ8 + (long)row * rowBytes, weightsQ8 + (row + 1) * rowBytes, weightsQ8 + (row + 2) * rowBytes, weightsQ8 + (row + 3) * rowBytes, @@ -182,14 +182,14 @@ internal static void ComputeRows(byte* weightsQ8, byte* xQ8, float* result, int } for (; row < m; row++) { - result[row] = VecDotQ8_0Avx2(weightsQ8 + row * rowBytes, xQ8, blockCount); + result[row] = VecDotQ8_0Avx2(weightsQ8 + (long)row * rowBytes, xQ8, blockCount); } } else { for (int row = 0; row < m; row++) { - result[row] = VecDotQ8_0Scalar(weightsQ8 + row * rowBytes, xQ8, blockCount); + result[row] = VecDotQ8_0Scalar(weightsQ8 + (long)row * rowBytes, xQ8, blockCount); } } } @@ -1564,7 +1564,7 @@ public static void GemvF16(nint weights, float* x, float* y, int m, int k) float* rowBuf = stackalloc float[k]; for (int row = 0; row < m; row++) { - var srcRow = new ReadOnlySpan(weightsHalf + row * k, k); + var srcRow = new ReadOnlySpan(weightsHalf + (long)row * k, k); var destRow = new Span(rowBuf, k); TensorPrimitives.ConvertToSingle(srcRow, destRow); y[row] = TensorPrimitives.Dot(destRow, new ReadOnlySpan(x, k)); @@ -1577,7 +1577,7 @@ public static void GemvF16(nint weights, float* x, float* y, int m, int k) { for (int row = 0; row < m; row++) { - var srcRow = new ReadOnlySpan(weightsHalf + row * k, k); + var srcRow = new ReadOnlySpan(weightsHalf + (long)row * k, k); var destRow = rented.AsSpan(0, k); TensorPrimitives.ConvertToSingle(srcRow, destRow); y[row] = TensorPrimitives.Dot(destRow, new ReadOnlySpan(x, k)); From 09242339ff613af38a84b81b77fe6ca919b6062f Mon Sep 17 00:00:00 2001 From: James Burton Date: Fri, 7 Aug 2026 14:47:00 +0100 Subject: [PATCH 2/3] fix(cpu/matmul): widen the sibling 4-row GEMV offsets too (#429) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first pass widened only `weightsQ8 + row * rowBytes` and left the three sibling pointers in the VNNI/AVX-512 4-row unrolled calls (`weightsQ8 + (row + 1|2|3) * rowBytes`) in 32-bit arithmetic. Those overflow on exactly the same tensors — and slightly sooner, since they address further into the matrix — so the original fix was incomplete on the hot path it was meant to protect. Also widens GemvF32's weight row offsets (`a + row * k`, 2 sites). `a` is documented as "weight matrix A [M×K]", making these the same class as the GemvF16 sites; wrapping needs >2^31 float elements (~8.6 GB), so like F16 this is consistency rather than a reachable defect. The tiled F16 row read (`tileWeightsHalf + row * k`) remains deliberately untouched: row < tileRows <= 256 and the base is pre-offset via `(long)mStart * k`. Caught by Copilot review on the PR. --- src/DotLLM.Cpu/Kernels/MatMul.cs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/DotLLM.Cpu/Kernels/MatMul.cs b/src/DotLLM.Cpu/Kernels/MatMul.cs index a6c8a795..592e58dc 100644 --- a/src/DotLLM.Cpu/Kernels/MatMul.cs +++ b/src/DotLLM.Cpu/Kernels/MatMul.cs @@ -58,7 +58,7 @@ public static void GemvF32(float* a, float* x, float* result, int m, int k) for (int row = 0; row < m; row++) { - var rowSpan = new ReadOnlySpan(a + row * k, k); + var rowSpan = new ReadOnlySpan(a + (long)row * k, k); result[row] = TensorPrimitives.Dot(rowSpan, xSpan); } } @@ -72,7 +72,7 @@ internal static void GemvF32Scalar(float* a, float* x, float* result, int m, int for (int row = 0; row < m; row++) { float sum = 0; - float* rowPtr = a + row * k; + float* rowPtr = a + (long)row * k; for (int j = 0; j < k; j++) sum += rowPtr[j] * x[j]; result[row] = sum; @@ -139,9 +139,9 @@ internal static void ComputeRows(byte* weightsQ8, byte* xQ8, float* result, int { VecDotQ8_0Vnni_4Rows( weightsQ8 + (long)row * rowBytes, - weightsQ8 + (row + 1) * rowBytes, - weightsQ8 + (row + 2) * rowBytes, - weightsQ8 + (row + 3) * rowBytes, + weightsQ8 + (long)(row + 1) * rowBytes, + weightsQ8 + (long)(row + 2) * rowBytes, + weightsQ8 + (long)(row + 3) * rowBytes, xQ8, blockCount, result + row); } for (; row < m; row++) @@ -157,9 +157,9 @@ internal static void ComputeRows(byte* weightsQ8, byte* xQ8, float* result, int { VecDotQ8_0Avx512_4Rows( weightsQ8 + (long)row * rowBytes, - weightsQ8 + (row + 1) * rowBytes, - weightsQ8 + (row + 2) * rowBytes, - weightsQ8 + (row + 3) * rowBytes, + weightsQ8 + (long)(row + 1) * rowBytes, + weightsQ8 + (long)(row + 2) * rowBytes, + weightsQ8 + (long)(row + 3) * rowBytes, xQ8, blockCount, result + row); } for (; row < m; row++) @@ -175,9 +175,9 @@ internal static void ComputeRows(byte* weightsQ8, byte* xQ8, float* result, int { VecDotQ8_0Avx2_4Rows( weightsQ8 + (long)row * rowBytes, - weightsQ8 + (row + 1) * rowBytes, - weightsQ8 + (row + 2) * rowBytes, - weightsQ8 + (row + 3) * rowBytes, + weightsQ8 + (long)(row + 1) * rowBytes, + weightsQ8 + (long)(row + 2) * rowBytes, + weightsQ8 + (long)(row + 3) * rowBytes, xQ8, blockCount, result + row); } for (; row < m; row++) From 58cdf4cbcd277b677b8db7b1272a0c9758bad6dd Mon Sep 17 00:00:00 2001 From: James Burton Date: Sun, 9 Aug 2026 18:44:10 +0100 Subject: [PATCH 3/3] docs(cpu/matmul): record why the tiled F16 row offsets stay int (#429) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A follow-up audit of the whole CPU backend for this bug class found no further reachable sites, but the two tiled F16 row reads look exactly like the bug and will keep attracting "fix" attempts. Records the bound that makes them safe: the tensor-scale offset is already carried in 64-bit by `tileWeightsHalf` (`(long)mStart * k`), and `row < tileRows <= tileM <= 256` because every TileM in the project originates from ComputeTileM's `Math.Clamp(tileM, 4, 256)`, so the residual product tops out at 255 * k — 4.2M for a 16384-wide 405B tensor. Comments only, no behaviour change. --- src/DotLLM.Cpu/Kernels/MatMul.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/DotLLM.Cpu/Kernels/MatMul.cs b/src/DotLLM.Cpu/Kernels/MatMul.cs index 592e58dc..e66fc607 100644 --- a/src/DotLLM.Cpu/Kernels/MatMul.cs +++ b/src/DotLLM.Cpu/Kernels/MatMul.cs @@ -1619,6 +1619,10 @@ public static void GemmF16(nint weights, float* b, float* c, int m, int k, int n var xSpan = new ReadOnlySpan(xPtr, k); var destRow = new Span(rowBuf, k); + // `row * k` stays in int on purpose (#429 audit): the whole-tensor offset + // is already carried in 64-bit by `tileWeightsHalf`, and + // row < tileRows <= tileM <= 256 (ComputeTileM clamps), so the residual + // product tops out at 255 * k — 4.2M for a 16384-wide 405B tensor. for (int row = 0; row < tileRows; row++) { var srcRow = new ReadOnlySpan(tileWeightsHalf + row * k, k); @@ -2512,6 +2516,8 @@ private static void GemmTiledF16Worker(nint ctxPtr, int threadIdx, int threadCou var xSpan = new ReadOnlySpan(xPtr, ctx.K); for (int row = 0; row < tileRows; row++) { + // See GemmF16: `row * ctx.K` is deliberately int — the tensor-scale offset + // lives in `tileWeightsHalf` (64-bit) and row < tileRows <= ctx.TileM <= 256. var srcRow = new ReadOnlySpan(tileWeightsHalf + row * ctx.K, ctx.K); TensorPrimitives.ConvertToSingle(srcRow, destRow); outPtr[row] = TensorPrimitives.Dot(destRow, xSpan);