Codex/q4k avx512 q8k

[Jackson57279]

Summary by cubic

Adds AVX-512/VNNI Q4_K × Q8_K kernels with Q8_K activation quantization and optional NUMA weight replication for faster CPU GEMV, plus async layer-ahead prefetch and transparent hugepages for mmap’d GGUF weights to cut cold-cache stalls. Also fixes a security advisory by bumping anyhow to 1.0.103, makes CLI tests deterministic, and enables LTO in release builds.

• New Features

  • Async layer-ahead prefetch via readahead(2); per-layer byte ranges built in GgufModel.
  • Transparent hugepages hint (MADV_HUGEPAGE) via GgufLoadOptions and --mmap-hugepages.
  • Paging policy control: --mmap-policy demand|prefetch|sequential|random.
  • AVX-512F/BW and AVX-512VNNI Q4_K × Q8_K kernels with runtime dispatch; activations quantized once to Q8_K per GEMV.
  • NUMA replication: copy quantized weights to node 1; threads read local copies for higher bandwidth.
  • Autotune emits mmap_policy and prefetch_layers; uses interleave on large dual-socket models and prefetch when model > RAM.
  • Release builds enable -flto.
  • Tests for layer-range mapping, mmap policy parse/hugepages, and autotune output; CLI tests use a fixed base clock to avoid flakiness.

• Migration

  • API: LlamaModel now accepts prefetch_layers; load_llama_gguf(path, want_cuda, quant, prefetch_layers) updated.
  • CLI: new flags --mmap-policy, --mmap-hugepages, and --prefetch-layers <n> (use --auto --print-plan to see recommended values).

^{Written for commit 7ee4edd. Summary will update on new commits.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a3ce2e3ed

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Guard AVX-512 source on non-x86 builds

On non-x86 builds (for example ARM64 Linux/macOS), CMake’s existing file(GLOB CORE_SOURCES ... src/*.cpp) now pulls in this new file, but it unconditionally includes <immintrin.h> and defines x86-only AVX-512 target functions. Runtime dispatch cannot help because these targets fail at compile time, so the CPU build loses portability; guard this file with x86 checks and provide scalar stubs, or exclude it from CORE_SOURCES off x86.

Useful? React with 👍 / 👎.

[cubic-dev-ai]

13 issues found across 22 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="oxidize-cpp/src/tensor_cpu.cpp">

<violation number="1" location="oxidize-cpp/src/tensor_cpu.cpp:698">
P2: Replicated Q4_K GEMV now creates nested OpenMP parallel regions, which can significantly reduce throughput from oversubscription/parallel-runtime overhead. This comes from calling `gemv_q4k_range` (already parallelized) inside an outer `#pragma omp parallel`; using a per-row non-parallel dot in the outer chunk avoids that.</violation>
</file>

<file name="oxidize-cpp/src/kernels_avx512.cpp">

<violation number="1" location="oxidize-cpp/src/kernels_avx512.cpp:20">
P3: Core Q4_K/Q8_K decode helpers are duplicated across `kernels.cpp` and this new AVX-512 file, so later fixes can drift between scalar and SIMD paths. Moving these helpers to one shared internal implementation would reduce divergence risk.</violation>

<violation number="2" location="oxidize-cpp/src/kernels_avx512.cpp:258">
P3: `q4k_q8k_row_dot_x4_avx512vnni` has external linkage but is only used internally. Move it inside the anonymous `namespace { }` block (or mark it `static`) to prevent potential symbol collisions with other translation units and to match the existing pattern in this file where all other file-internal helpers are already scoped to the anonymous namespace.</violation>
</file>

<file name="oxidize-cpp/tests/gguf_mmap_policy_test.cpp">

<violation number="1" location="oxidize-cpp/tests/gguf_mmap_policy_test.cpp:15">
P1: All test assertions in this file use `assert()`, which is compiled out when `NDEBUG` is defined. The CMakeLists.txt sets `-DNDEBUG` in the Release flags and defaults to the Release build type, meaning all checks silently vanish in the default build — the test prints "ok" and exits 0 even if every assertion would fail. Follow the pattern established in `parity_test.cpp` (a `CHECK` macro that tracks failures) or `autotune_test.cpp` (a `require()` helper using unconditional `std::abort()`) so that assertions are always enforced regardless of build type.</violation>
</file>

<file name="oxidize-cpp/include/oxidize/prefetcher.hpp">

<violation number="1" location="oxidize-cpp/include/oxidize/prefetcher.hpp:25">
P1: Prefetch requests can read invalid memory when `LayerRanges` does not outlive `LayerPrefetcher`, because the class stores a reference to constructor input. Consider owning `LayerRanges` (or another owned handle) inside the class, or enforce/document the lifetime contract on `ranges` explicitly.</violation>
</file>

<file name="oxidize-cpp/src/autotune.cpp">

<violation number="1" location="oxidize-cpp/src/autotune.cpp:131">
P2: Dual-socket dense models between 4 GiB and 192 GiB now get `--numa interleave` with all logical cores, which contradicts the documented autotune rule for this port. Consider preserving the 192 GiB cutoff/single-node plan here so `--auto` remains consistent with the benchmarked behavior.</violation>
</file>

<file name="oxidize-cpp/src/prefetcher.cpp">

<violation number="1" location="oxidize-cpp/src/prefetcher.cpp:19">
P2: Model load can be forced into huge memory allocation before inference starts, because prefetch state size is derived from the largest parsed layer id instead of bounded model layer count. Consider bounding or validating layer indices before sizing `requested_` (or using a sparse requested-set) so malformed GGUF metadata cannot OOM this path.</violation>
</file>

<file name="oxidize-cpp/src/numa_util.cpp">

<violation number="1" location="oxidize-cpp/src/numa_util.cpp:243">
P1: Replicate mode with an explicit thread count (`cfg.threads > 0`) puts all threads on node 0, so `W_replica` is never read during inference. When `--numa replicate` is combined with `--threads <total_logical>`, the thread-to-CPU assignment takes the first `n_threads` entries from the combined CPU list. Since Replicate mode places all node-0 CPUs first, every thread lands on node 0 — `current_thread_numa_node()` returns 0 for all threads, and the replica buffer (`W_replica`) is silently unused. This wastes the replica allocation and the bandwidth benefit of replication. Consider either (a) distributing threads across nodes in Replicate mode by interleaving the CPU assignment, or (b) always using `total_logical` threads in Replicate mode regardless of `cfg.threads`.</violation>

<violation number="2" location="oxidize-cpp/src/numa_util.cpp:316">
P1: On single-NUMA-node systems, replicate mode still turns on global replication and duplicates every quantized weight buffer, which can cause large unnecessary memory growth and repeated `mbind(node=1)` failures. This comes from enabling replication without checking that a second NUMA node exists; gating it on discovered node count would avoid wasted copies.</violation>

<violation number="3" location="oxidize-cpp/src/numa_util.cpp:320">
P2: Replicated GEMV/GEMM routing assumes the second socket is exactly node id `1`, but this code stores raw node ids from sysfs. On non-contiguous NUMA ids, second-socket threads fall back to the primary weight copy, losing intended locality; normalizing to replica-slot indices or updating the selector logic would keep behavior correct.</violation>
</file>

<file name="oxidize-cpp/src/gguf.cpp">

<violation number="1" location="oxidize-cpp/src/gguf.cpp:683">
P2: Split GGUF load leaks the current shard fd when `parse(sbase, ssize)` throws. Since fds are now intentionally kept open past `mmap`, the parse-failure catch should close `fd` before rethrowing unless ownership has been transferred into `shards_`.</violation>

<violation number="2" location="oxidize-cpp/src/gguf.cpp:844">
P2: Layer prefetch ranges can escape the shard bounds for malformed tensor dimensions because `offset + clamped_len` can overflow before the comparison. Comparing `clamped_len` against `shards_[shard].size - offset` keeps the existing clamp without wraparound.</violation>
</file>

<file name="oxidize-cpp/src/cli/main.cpp">

<violation number="1" location="oxidize-cpp/src/cli/main.cpp:230">
P2: `--prefetch-layers -1` (documented as "auto") cannot be parsed because `parse_size` reads `size_t` via `std::from_chars`, which rejects the `-` sign. Users passing `--prefetch-layers -1` get a parse error and usage exit, even though the help text advertises `-1=auto`. Either parse `-1` separately before calling `parse_size`, or document only non-negative values and keep `-1` as the default when the flag is absent.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: All test assertions in this file use assert(), which is compiled out when NDEBUG is defined. The CMakeLists.txt sets -DNDEBUG in the Release flags and defaults to the Release build type, meaning all checks silently vanish in the default build — the test prints "ok" and exits 0 even if every assertion would fail. Follow the pattern established in parity_test.cpp (a CHECK macro that tracks failures) or autotune_test.cpp (a require() helper using unconditional std::abort()) so that assertions are always enforced regardless of build type.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/tests/gguf_mmap_policy_test.cpp, line 15:

<comment>All test assertions in this file use `assert()`, which is compiled out when `NDEBUG` is defined. The CMakeLists.txt sets `-DNDEBUG` in the Release flags and defaults to the Release build type, meaning all checks silently vanish in the default build — the test prints "ok" and exits 0 even if every assertion would fail. Follow the pattern established in `parity_test.cpp` (a `CHECK` macro that tracks failures) or `autotune_test.cpp` (a `require()` helper using unconditional `std::abort()`) so that assertions are always enforced regardless of build type.</comment>

<file context>
@@ -0,0 +1,41 @@
+  opts.mmap_hugepages = true;
+  auto model = oxidize::GgufModel::load(
+      "../tests/fixtures/valid-v3.gguf", opts);
+  assert(model.size() > 0);
+}
+
</file context>

[cubic-dev-ai]

P1: Prefetch requests can read invalid memory when LayerRanges does not outlive LayerPrefetcher, because the class stores a reference to constructor input. Consider owning LayerRanges (or another owned handle) inside the class, or enforce/document the lifetime contract on ranges explicitly.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/include/oxidize/prefetcher.hpp, line 25:

<comment>Prefetch requests can read invalid memory when `LayerRanges` does not outlive `LayerPrefetcher`, because the class stores a reference to constructor input. Consider owning `LayerRanges` (or another owned handle) inside the class, or enforce/document the lifetime contract on `ranges` explicitly.</comment>

<file context>
@@ -0,0 +1,57 @@
+ public:
+  // `shard_fds` must stay valid for the lifetime of the prefetcher. A negative
+  // fd for a shard means "no readahead for that shard".
+  LayerPrefetcher(const LayerRanges& ranges,
+                  const std::vector<int>& shard_fds,
+                  int lookahead);
</file context>

[cubic-dev-ai]

P1: On single-NUMA-node systems, replicate mode still turns on global replication and duplicates every quantized weight buffer, which can cause large unnecessary memory growth and repeated mbind(node=1) failures. This comes from enabling replication without checking that a second NUMA node exists; gating it on discovered node count would avoid wasted copies.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/src/numa_util.cpp, line 316:

<comment>On single-NUMA-node systems, replicate mode still turns on global replication and duplicates every quantized weight buffer, which can cause large unnecessary memory growth and repeated `mbind(node=1)` failures. This comes from enabling replication without checking that a second NUMA node exists; gating it on discovered node count would avoid wasted copies.</comment>

<file context>
@@ -287,18 +300,33 @@ int init_numa(const NumaConfig& cfg, const std::vector<NumaNode>& nodes) {
+            cpu_node.assign(target_cpus.size(), bound_node->id);
+        }
+    } else if (cfg.mode == NumaMode::Replicate) {
+        g_replication_enabled = true;
+        for (auto& n : nodes) {
+            for (int c : n.cpus) {
</file context>

Suggested change

	g_replication_enabled = true;
	g_replication_enabled = nodes.size() > 1;

[cubic-dev-ai]

P1: Replicate mode with an explicit thread count (cfg.threads > 0) puts all threads on node 0, so W_replica is never read during inference. When --numa replicate is combined with --threads <total_logical>, the thread-to-CPU assignment takes the first n_threads entries from the combined CPU list. Since Replicate mode places all node-0 CPUs first, every thread lands on node 0 — current_thread_numa_node() returns 0 for all threads, and the replica buffer (W_replica) is silently unused. This wastes the replica allocation and the bandwidth benefit of replication. Consider either (a) distributing threads across nodes in Replicate mode by interleaving the CPU assignment, or (b) always using total_logical threads in Replicate mode regardless of cfg.threads.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/src/numa_util.cpp, line 243:

<comment>Replicate mode with an explicit thread count (`cfg.threads > 0`) puts all threads on node 0, so `W_replica` is never read during inference. When `--numa replicate` is combined with `--threads <total_logical>`, the thread-to-CPU assignment takes the first `n_threads` entries from the combined CPU list. Since Replicate mode places all node-0 CPUs first, every thread lands on node 0 — `current_thread_numa_node()` returns 0 for all threads, and the replica buffer (`W_replica`) is silently unused. This wastes the replica allocation and the bandwidth benefit of replication. Consider either (a) distributing threads across nodes in Replicate mode by interleaving the CPU assignment, or (b) always using `total_logical` threads in Replicate mode regardless of `cfg.threads`.</comment>

<file context>
@@ -225,7 +240,13 @@ int init_numa(const NumaConfig& cfg, const std::vector<NumaNode>& nodes) {
     int n_threads = cfg.threads;
     if (n_threads <= 0) {
-        if (bound_node) {
+        if (cfg.mode == NumaMode::Replicate) {
+            // Use every logical core on both sockets; each socket works on its
+            // local weight copy.
</file context>

[cubic-dev-ai]

P2: Replicated Q4_K GEMV now creates nested OpenMP parallel regions, which can significantly reduce throughput from oversubscription/parallel-runtime overhead. This comes from calling gemv_q4k_range (already parallelized) inside an outer #pragma omp parallel; using a per-row non-parallel dot in the outer chunk avoids that.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/src/tensor_cpu.cpp, line 698:

<comment>Replicated Q4_K GEMV now creates nested OpenMP parallel regions, which can significantly reduce throughput from oversubscription/parallel-runtime overhead. This comes from calling `gemv_q4k_range` (already parallelized) inside an outer `#pragma omp parallel`; using a per-row non-parallel dot in the outer chunk avoids that.</comment>

<file context>
@@ -656,6 +663,49 @@ void gemv_quantized(float* y, QuantType quant, const uint8_t* W, size_t rows,
+      long long r0 = static_cast<long long>(tid) * chunk;
+      long long r1 = std::min(r0 + chunk, static_cast<long long>(rows));
+      if (r0 < r1) {
+        oxidize::kernels::gemv_q4k_range(
+            W_local + static_cast<size_t>(r0) * row_bytes, blocks_per_row,
+            q8k.data(), y + static_cast<size_t>(r0),
</file context>

Suggested change

	oxidize::kernels::gemv_q4k_range(
	W_local + static_cast<size_t>(r0) * row_bytes, blocks_per_row,
	q8k.data(), y + static_cast<size_t>(r0),
	static_cast<size_t>(r1 - r0));
	}
	for (long long r = r0; r < r1; ++r) {
	y[static_cast<size_t>(r)] = oxidize::kernels::q4k_q8k_row_dot(
	W_local + static_cast<size_t>(r) * row_bytes, blocks_per_row,
	q8k.data());
	}

[cubic-dev-ai]

P2: Split GGUF load leaks the current shard fd when parse(sbase, ssize) throws. Since fds are now intentionally kept open past mmap, the parse-failure catch should close fd before rethrowing unless ownership has been transferred into shards_.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/src/gguf.cpp, line 683:

<comment>Split GGUF load leaks the current shard fd when `parse(sbase, ssize)` throws. Since fds are now intentionally kept open past `mmap`, the parse-failure catch should close `fd` before rethrowing unless ownership has been transferred into `shards_`.</comment>

<file context>
@@ -624,11 +676,11 @@ GgufModel GgufModel::load_split(const std::string& first_path,
       throw std::runtime_error("gguf: io error: mmap failed for shard " + shard_path);
     }
-    ::madvise(smap, ssize, MADV_RANDOM);
+    advise_mmap(smap, ssize, options);
 
     const uint8_t* sbase = static_cast<const uint8_t*>(smap);
</file context>

[cubic-dev-ai]

P2: Layer prefetch ranges can escape the shard bounds for malformed tensor dimensions because offset + clamped_len can overflow before the comparison. Comparing clamped_len against shards_[shard].size - offset keeps the existing clamp without wraparound.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/src/gguf.cpp, line 844:

<comment>Layer prefetch ranges can escape the shard bounds for malformed tensor dimensions because `offset + clamped_len` can overflow before the comparison. Comparing `clamped_len` against `shards_[shard].size - offset` keeps the existing clamp without wraparound.</comment>

<file context>
@@ -723,6 +782,99 @@ Architecture GgufModel::architecture() const {
+    size_t offset = static_cast<size_t>(info.absolute_offset);
+    if (offset > shards_[shard].size) continue;
+    size_t clamped_len = byte_len;
+    if (offset + clamped_len > shards_[shard].size) {
+      clamped_len = shards_[shard].size - offset;
+    }
</file context>

Suggested change

	if (offset + clamped_len > shards_[shard].size) {
	if (clamped_len > shards_[shard].size - offset) {

[cubic-dev-ai]

P2: --prefetch-layers -1 (documented as "auto") cannot be parsed because parse_size reads size_t via std::from_chars, which rejects the - sign. Users passing --prefetch-layers -1 get a parse error and usage exit, even though the help text advertises -1=auto. Either parse -1 separately before calling parse_size, or document only non-negative values and keep -1 as the default when the flag is absent.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/src/cli/main.cpp, line 230:

<comment>`--prefetch-layers -1` (documented as "auto") cannot be parsed because `parse_size` reads `size_t` via `std::from_chars`, which rejects the `-` sign. Users passing `--prefetch-layers -1` get a parse error and usage exit, even though the help text advertises `-1=auto`. Either parse `-1` separately before calling `parse_size`, or document only non-negative values and keep `-1` as the default when the flag is absent.</comment>

<file context>
@@ -208,6 +218,21 @@ Args parse_args(int argc, char** argv) {
+    } else if (arg == "--prefetch-layers") {
+      std::string v = take_value(argc, argv, i, "--prefetch-layers");
+      size_t n = 0;
+      if (!parse_size(v, n) || static_cast<int>(n) < -1) {
+        std::fprintf(stderr, "error: invalid --prefetch-layers '%s'\n", v.c_str());
+        usage_and_exit(argv[0], 2);
</file context>

[cubic-dev-ai]

P3: q4k_q8k_row_dot_x4_avx512vnni has external linkage but is only used internally. Move it inside the anonymous namespace { } block (or mark it static) to prevent potential symbol collisions with other translation units and to match the existing pattern in this file where all other file-internal helpers are already scoped to the anonymous namespace.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/src/kernels_avx512.cpp, line 258:

<comment>`q4k_q8k_row_dot_x4_avx512vnni` has external linkage but is only used internally. Move it inside the anonymous `namespace { }` block (or mark it `static`) to prevent potential symbol collisions with other translation units and to match the existing pattern in this file where all other file-internal helpers are already scoped to the anonymous namespace.</comment>

<file context>
@@ -0,0 +1,299 @@
+
+
+__attribute__((target("avx512f,avx512bw,avx512vnni")))
+void q4k_q8k_row_dot_x4_avx512vnni(const uint8_t* rows_base,
+                                    size_t row_bytes,
+                                    size_t blocks_per_row,
</file context>

[cubic-dev-ai]

P3: Core Q4_K/Q8_K decode helpers are duplicated across kernels.cpp and this new AVX-512 file, so later fixes can drift between scalar and SIMD paths. Moving these helpers to one shared internal implementation would reduce divergence risk.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/src/kernels_avx512.cpp, line 20:

<comment>Core Q4_K/Q8_K decode helpers are duplicated across `kernels.cpp` and this new AVX-512 file, so later fixes can drift between scalar and SIMD paths. Moving these helpers to one shared internal implementation would reduce divergence risk.</comment>

<file context>
@@ -0,0 +1,299 @@
+namespace {
+
+// f16 (little-endian bytes) -> f32, no dependency (oxidize-kernels f16_le_to_f32).
+inline float f16_le_to_f32(const uint8_t* b) {
+  uint16_t bits = static_cast<uint16_t>(b[0]) | (static_cast<uint16_t>(b[1]) << 8);
+  uint32_t sign = (bits >> 15) & 1u;
</file context>

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[Jackson57279]

Triage note (maintainer pass): the AVX-512F/BW + VNNI Q4_K×Q8_K kernels with runtime dispatch are the valuable, novel part of this PR and CI is green.

The problem is scope: per the cubic summary this bundles at least four independent changes — (1) AVX-512 Q4_K/Q8_K kernels, (2) async prefetch + hugepages + --mmap-policy duplicated from #29, (3) NUMA weight replication that overlaps #31's autotune, and (4) release LTO. That makes it un-reviewable as one unit and guarantees conflicts with #29/#31.

Please rebase this down to just the AVX-512 kernels (+ the Q8_K activation quant they need) on top of master. The mmap/prefetch should live in #29 and the NUMA autotune in #31; once those land, this becomes a clean, focused kernel PR. Also the empty PR title/description — worth filling in given the rest of the batch is well-documented.