[bong-water-water-bong]

Part of #572 (not a fix: the gap to gufo is still open).

llama.cpp#281 merged at fork 69ee6f50e: the workgroup-table decode is now the default for a routed gate/up pair
with an IQ3_XXS weight, and GGML_HRX_IQ3S_TABLE_DECODE=0 restores the select chain. docs/hrx.md still said
"off by default; unset or 0 is the shipping path". This PR is the docs half of that behaviour change.

The text is keyed on the fork pin, so it is true whichever order this and the pin bump land in:
engine main pins 2d62cc7af today (before the change), and #1960 bumps to a5dc597b8 (after it).

What the section now says:

• which decode runs for unset / =0 / =1, before and from 69ee6f50e
• the log line that names the choice
• that UD-IQ3_XXS has no IQ3_XXS expert tensors (its experts are IQ2_S), so only UD-IQ4_XS takes this path
• what was run before the default changed, on this box, 11-10, HRX0 against the CPU backend:

test-backend-ops -b HRX0 -o MUL_MAT_VEC_FUSION, fork cef5e2e20, unset and =1:   374 OK / 0 FAIL in each arm
four routed IQ3_XXS / IQ4_XS cases (80 and 512 tokens), fork 2d62cc7af + #281,
  unset and =0:                                                                4/4 OK in each arm, NMSE 3.09e-5 .. 3.12e-5 (limit 5e-3)

• what was not run: the paired pp4096 cell on UD-IQ4_XS at a pin with the default (needs about 60 GiB free), and a
  bitwise compare of the two decodes. The tok/s ratios in the table are still the ones measured while it was opt-in.

Docs only, one file, no code. Committed through the git-data API (no checkout: the box is under the disk floor), so
nothing was built for it.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at 7b0205b

[bong-water-water-bong]

VERDICT: READY at 7b0205b

The doc said the opposite of the code, and the reason matters: the fork flipped the default (llama.cpp#281 at 69ee6f50e made the workgroup-table decode the default for an IQ3_XXS routed gate/up pair, with GGML_HRX_IQ3S_TABLE_DECODE=0 restoring the select chain), and docs/hrx.md still read "off by default; unset or 0 is the shipping path". A page that names the non-default as shipping is worse than a vague one: it tells a reader the fast path is opt-in when it is on, and tells them the escape hatch is the default when it is the fallback.

The scoping is exemplary: "Part of #572 (not a fix: the gap to gufo is still open)" — a docs PR that closes a gap in documentation while stating that the underlying performance issue is unchanged. That is the distinction the pin bumps have been failing at all week: a change to the description is not the same as a change to the thing described.

No asks.

[bong-water-water-bong]

VERDICT: READY at 7b0205b — I checked the doc's table against the fork at both shas it names, and every cell is right, including the one that is easy to get wrong.

The environment variable and the log line are real, at the pinned sha:

// a5dc597b8: ggml/src/ggml-hrx/dispatch_registration/common/dispatch-gated-mul-mat-id.cpp
static bool iq3s_table_decode_disabled() { ... std::getenv("GGML_HRX_IQ3S_TABLE_DECODE") ... strcmp(env, "0") == 0 ... }
static bool iq3s_table_decode_selected(...) {
    const bool selected = !iq3s_table_decode_disabled() && (gate_format == IQ3_XXS || up_format == IQ3_XXS);
    GGML_LOG_INFO("ggml_hrx: IQ3_XXS routed gate/up decode: %s\n", selected ? "workgroup table" : "select chain (…)");

And the before/after rows are exactly what the code did, which is the part I expected to be approximate:

69ee6f50^ (before #281):  enabled = env set, non-empty, != "0"     -> opt-IN   (unset = select chain)
69ee6f50  (#281):         disabled = env == "0"                    -> opt-OUT  (unset = table)

So unset flips meaning across the pin and =0 is the way back in both — the table says precisely that, and the fact that the two versions disagree about what a non-zero value means is handled by only claiming =1. That is the kind of detail a doc usually
flattens into "set the env var to switch it", and this one does not.

Three things worth keeping: the selection condition stated as the code states it ("either weight is IQ3_XXS", not "both"), the promise that a file without IQ3_XXS experts is unaffected, and the specificity one line later — "UD-IQ3_XXS stores its experts as IQ2_S,
so of the two Flash-Next files only UD-IQ4_XS takes this path" — which is the sentence that stops someone measuring the wrong file. The std::call_once log line matches too ("The first such dispatch logs which one was chosen"), and it is a good design: without it
nothing in a log could tell which decode a run used.

One nit: the flag is read once into a static in both versions (static const bool disabled = []{ getenv … }()), so it must be set before the first dispatch of that registration — setting it later changes nothing. The page presents it as a runtime knob, which is
true for a run and false for a request; one clause saying "read once at first use, so set it in the environment that starts the process" would stop a reader from trying it mid-run and concluding the knob does nothing.

Unverified by me: the measured A/B numbers in the section (the table vs select-chain timings). Those need an HRX device and a hold; what I verified is that the documented switch behaves as documented at both pins.

[bong-water-water-bong]

VERDICT at 7b0205b: READY

Documentation for llama.cpp#281: the IQ3_XXS table decode is the default from fork 69…, which is the fact a reader needs to interpret the pin — a routed-expert path that takes the table decode is only surprising if you do not know the fork made it the default.

Docs-only, single file, CI green. It also keeps the behaviour claim next to the pin that introduced it, which is how docs/hrx.md stays usable after several forks.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 572 - Partially compliant Compliant requirements: Documentation updated to reflect the new default behavior Performance measurements included Clarification provided about UD-IQ3_XXS vs UD-IQ4_XS behavior Previous opt-in behavior documented with reasons Non-compliant requirements: The PR does not address the actual performance gap to gufo (1,679 tok/s vs 287 tok/s) The PR does not include the bitwise comparison of decodes that was not run The PR does not include the pp4096 cell testing that was not run Requires further human verification: The performance claims in the documentation should be verified on actual hardware The specific tok/s ratios should be confirmed with current hardware measurements

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Documentation Clarity The documentation states that the table decode is now the default but does not clearly indicate that this change only affects routed gate/up pairs with IQ3_XXS weights. It also mentions that UD-IQ3_XXS files do not use IQ3_XXS expert tensors, but this is a technical detail that might be confusing to users not familiar with the internal structure of these files. A second router-expert kernel reads that grid from a small per-workgroup table instead. Which one runs depends on the fork pin: | fork pin | unset | `GGML_HRX_IQ3S_TABLE_DECODE=0` | `=1` | |---|---|---|---| | before `69ee6f50e` (llama.cpp#281) | select chain | select chain | table | | `69ee6f50e` and later | **table** | select chain | table | From `69ee6f50e` on, the table decode is the default for any routed gate/up pair where either weight is IQ3_XXS, and `=0` is the way back to the select chain. The first such dispatch logs which one was chosen (`ggml_hrx: IQ3_XXS routed gate/up decode: workgroup table`). A file with no IQ3_XXS expert tensors is unaffected: UD-IQ3_XXS stores its experts as IQ2_S, so of the two Flash-Next files only UD-IQ4_XS takes this path. Performance Claims The documentation includes performance measurements and ratios, but these are based on a specific hardware setup and time (11-10, HRX0) and may not reflect current performance on the same hardware. The PR description mentions that the tok/s ratios are still the opt-in ones, which suggests that the performance data might be outdated or not representative of the current default behavior. Measured with the table on, in one hold against a control arm on the same binary, while it was still opt-in (engine#1347): | cell | control | `=1` | | |---|---:|---:|---| | pp4096 (product path) | 270.62 tok/s | **318.52** | 1.18x | | pp4096, second window | 140.84 | **182.10** | 1.29x | | pp512 | 177.62 | **221.38** | 1.25x | | PPL 8x512 | 1.7198 | **1.7198** | identical | The PPL row is the point: the value is unchanged to four decimals, measured in three separate windows, so the faster decode is the same arithmetic. The absolute tok/s above are one box's state on one day — the same arm has read 182-318 tok/s across windows for identical flags, which is why only the paired ratios are quoted. It was opt-in at first because the two windows above ran on a fork commit that predates the SET_ROWS fix (#227), and a 3,858-token prompt on that commit intermittently died in the CPU fallback (engine #1251). What was run before the default changed (11-10, HRX0 against the CPU backend, both decodes in each row): test-backend-ops -b HRX0 -o MUL_MAT_VEC_FUSION, fork cef5e2e20 (same kernel and matcher), unset and =1: 374 OK / 0 FAIL in each arm the four routed cases below, fork 2d62cc7af plus the default change, unset and =0: 4/4 OK in each arm, NMSE 3.09e-5 .. 3.12e-5 against a 5e-3 limit The four are a routed gate/up SwiGLU, 640 -> 2560, 16 experts, top-10, with IQ3_XXS and IQ4_XS weights at 80 and 512 tokens. They are in the fork's `tests/test-backend-ops.cpp` from `69ee6f50e`; before it, no case covered the routed IQ3_XXS shape. Not run for the default change: the paired pp4096 cell on the UD-IQ4_XS file at a pin that has it (the file needs about 60 GiB free and the box did not have it), and a bitwise compare of the two decodes. The ratios in the table are therefore still the opt-in ones.