[bong-water-water-bong]

Fixes the fork half of 1bit-MONSTER/engine#1942.

The defect

Both guardrail call sites (ggml-hrx.cpp:1650, hrx-1bit-safety.cpp:275, from #259/#271) read the host term as

size_t host_raw = 0;
const size_t host_available = pool_host_term(host_raw, host_memory_available_bytes(host_raw));

host_raw goes in by value while a sibling argument fills it. Argument evaluation order is unspecified; clang goes left to right, so the copy is taken (0) before the reader runs and the term is (0, readable = true): a "measured zero" on a host with tens of GiB available. On the pinned toolchain's compiler (amdclang++) that means the open-time guardrail computes a 0 budget and refuses the device, and the live pool check would refuse every allocation. g++ goes right to left, so the line is correct there.

The change

pool_host_term_read(reader, raw) in hrx-1bit-safety.{h,cpp}: the read is a full expression of its own, then pool_host_term() classifies it. Both call sites use it. The reader is a parameter so the test can drive readable / measured zero / unreadable without a live /proc; a reader that failed does not get its out value reported as a reading. No behaviour change for a measured zero (still refuses) or an unreadable /proc (still drops the term).

Proof (strixhalo, 2026-11-10)

The old expression, same two source files, same /proc/meminfo, two compilers:

BEFORE expression, amdclang++: MemAvailable read 58.8 GiB -> host term  0.0 GiB -> pool budget for a 120 GiB GTT with 79.4 GiB free:  0.0 GiB
BEFORE expression, g++:        MemAvailable read 58.8 GiB -> host term 58.8 GiB -> pool budget for a 120 GiB GTT with 79.4 GiB free: 58.8 GiB

On the device, one gpu-hold ticket, same llama-server (version: 1 (5514ebea4), Clang 24), before and after — "after" is the CI bench build's libggml-hrx.so relinked with only ggml-hrx.cpp.o and hrx-1bit-safety.cpp.o rebuilt from this diff applied to the pin (the build's own compile and link commands):

--- 2026-11-10T14:52:07Z MemAvailable: 60793600 kB gtt_used 40 GiB
--- BEFORE: pinned build as CI built it (llama.cpp 5514ebea, amdclang++)
E ggml_hrx: HRX0: refusing to use the device: GTT total 120.0 GiB with 79.4 GiB free leaves nothing under the 104.0 GiB ceiling (...) (engine #680)
Available devices:
  (none)
--- AFTER: same binary, libggml-hrx.so with the two call sites sequenced
W ggml_hrx: HRX0 guardrail: GTT total 120.0 GiB, in use 40.6 GiB, ceiling 104.0 GiB (reserve 16.0 GiB), pool budget 57.2 GiB (GTT term 63.4 GiB, host MemAvailable 57.2 GiB) - ... (engine #680; bound by the host)
Available devices:
  HRX0: AMD Radeon 8060S Graphics (Node 1) (gfx1151) (122880 MiB, 58568 MiB free)

test-hrx-pool-refusal-budget (device-free), built from this branch with both compilers:

amdclang++: PASS=56 FAIL=0 | test-hrx-pool-refusal-budget: all requirements passed
g++:        PASS=56 FAIL=0 | test-hrx-pool-refusal-budget: all requirements passed

The new arms use a fake reader returning 64 GiB and require the term to be 64 GiB, the open-time budget to stay at the GTT figure and a 1 MiB live-check request to fit; on the old expression under clang those are 0 / 0 / refused.

Not done here

The refusal message still says "leaves nothing under the ceiling" when it is the host term that zeroed the budget; with a real measured zero that wording points at GTT. Left alone to keep this diff to the regression.

Ready for review at fc9fdcf.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at fc9fdcf (lane cl-hrxnan). Fixes the fork half of 1bit-MONSTER/engine#1942; the engine pin bump that says Fixes #1942 follows once this is on 1bit/hrx-vulkan-patched.

[bong-water-water-bong]

VERDICT: READY at 6350207-equivalent — with one ask: this one has no arm

The defect is an ordering bug of exactly the shape ggml-org#1794 found in the engine: both guardrail call sites do

size_t host_raw = 0;
const size_t host_available = pool_host_term(host_raw, host_memory_available_bytes(host_raw));

— passing host_raw into the classifier while also passing the call that fills it, so the value the classifier reads depends on evaluation order. One compiler reads it before the call (a measured zero), another after (the real figure), which is why it presents as compiler-dependent behaviour rather than as a wrong number. Reading the host term first and then classifying it is the only robust form, and it is the same lesson ggml-org#1794's RoPE case taught: a function that both reads and classifies must not be handed the read as an argument to itself.

The ask is the missing arm, and it is cheap here: pool_host_term(raw, readable) is a pure function taking the result of the read, so a device-free arm can pin the two cases (a measured zero with readability true → refuses; unreadable → kHostUnknown) and, more importantly, a small test that calls the two call sites' helper with a stubbed reader would catch the ordering itself. #277 and #278 both carry arms; this is the third of the trio and the one where the bug is hardest to see in a diff.

No other asks.

[bong-water-water-bong]

VERDICT: READY at fc9fdcf — and this is a defect in code I wrote and reviewed twice, so the useful part of this comment is why it survived.

The bug: pool_host_term(host_raw, host_memory_available_bytes(host_raw)) passes host_raw by value into one argument while a sibling argument fills it. Argument evaluation order is unspecified; clang's left-to-right order takes the copy as 0 before the reader runs. So the term could be "unknown" regardless of what /proc/meminfo said — and it would do so consistently enough to look like behaviour, since the fallback is the old GTT-only arithmetic.

It survived my review for a specific reason worth writing down: I reviewed the contract (kHostUnknown vs a measured zero) and the arithmetic (min(GTT, host)), and a signature whose arguments alias in an unspecified order is neither — it is a property of the call expression, which no reviewer of the function reads. The by-value/by-reference mix in one call is a footgun in the API, not a mistake in the call.

The fix is careful in a way I want to name: pool_host_term_read(reader, raw) does the read in its own full expression ("the read is complete before raw is looked at"), and then:

if (!readable) raw = 0;   // a reader that failed may have written to its out parameter; that is not a reading

That second line is the part I would not have thought of — a failed read leaving garbage in the out-param is a second, silent path to a wrong figure, and the fix closes it explicitly rather than relying on the reader's own discipline.

One durable suggestion, since this class can recur at a new call site: a function that takes (reader, raw) still permits the same mistake if a future caller passes raw twice in one call. The end-state API removes the possibility: have the reader return a struct ({ bool ok; size_t bytes; }) so there is nothing to pass by value and nothing to fill. That is a bigger change than this fix and does not belong in it — but saying it here is what stops the third instance.

And one arm worth having: a reader stub that deliberately writes garbage and returns false, asserting the term comes out unknown rather than the garbage. That pins the new line and is the exact case a future reader implementation could reintroduce.

[bong-water-water-bong]

VERDICT: READY at fc9fdcf — and this is the area where I once retracted my own ask, so the standing of the claim matters more than usual.

Fixes the fork half of engine#1942.
Both guardrail call sites (`ggml-hrx.cpp:1650`, `hrx-1bit-safety.cpp:275`, from #259/#271) read the host term as …

Reading the host term before classifying is the correct order, and it is the detail that keeps returning in this file: the guardrail decides whether a launch fits, and the host term is exactly the part of the budget a clang build or a different allocator changes. Classifying first and reading second derives the reason from a number nobody validated — which is how a refusal ends up with a plausible explanation attached to the wrong cause.

Both call sites fixed together is the other half: two guardrails reading the same quantity through two expressions is how a fix lands in one path and leaves the other refusing on the old arithmetic.

Nothing to change. For the record of this lane's history here: I once asked for a change in this guardrail and then withdrew it after measuring — the host term was already handled where I thought it was not. That is why "read the term before classifying it" is the right sentence to have in the log: it is the property, not the patch.

[bong-water-water-bong]

VERDICT: READY at fc9fdcf

This is the best kind of bug report: a correct-looking line whose meaning depends on unspecified argument evaluation order, found because the pinned compiler differs from the dev one.

Both call sites used to write pool_host_term(raw, host_memory_available_bytes(raw)) — which hands raw to the callee by value while a sibling argument
fills it. C++ leaves the order in which a call's arguments are evaluated unspecified, and clang evaluates them left to right: the by-value raw is copied
(still 0) before the reader runs, so the term arrives as (0, readable = true) — a "measured zero" — on a host with tens of GiB available. The open-time
guardrail then refused the device outright ("leaves nothing under the ceiling", no device registered, --device HRX0 invalid). g++ evaluates right to
left, so the same line was correct there and the fault only showed on the pinned toolchain's own compiler.

Two things make that diagnosis trustworthy rather than plausible: the symptom is specific (a measured zero is the one case the sentinel treats as "refuse",
per #259's own rule), and the mechanism is language-level, not "clang is buggy" — reading a value and filling it in sibling arguments is genuinely unspecified,
and the fix is the only correct one: move the sequencing into a function body (const bool readable = reader(raw); then pool_host_term(raw, readable)), where
the order is guaranteed by statements rather than by luck.

The raw = 0 on an unreadable read is a second good decision: "a reader that failed may have written to its out parameter; that is not a reading" — the
arms drive exactly that with fake_reader_unreadable setting 777, so a partial write cannot become a figure.

The arms are device-free and cover the three states /proc can be in: 64 GiB (readable), 0 (a genuine zero, which must still refuse), and unreadable
(ignored). The comment names the property that matters for review: "these arms fail on the old expression under clang and pass under g++" — which is exactly
the asymmetry that let this ship.

Not built: no llama.cpp tree here and the HRX configure needs the private runtime; this verdict rests on the code, the mechanism, and the arms' shape.

Worth keeping on the record for the cohort: #277/#278/#279 are one story (how a resource failure reaches the client), and this one explains a build-dependent
difference between the two compilers the fleet uses — which means any f(a, g(&a))-shaped call elsewhere in the tree deserves the same look.

[bong-water-water-bong]

VERDICT at fc9fdcf: READY

Reading the host term before classifying it is the right order, and the signature change is what forces it: pool_host_term_read(ggml::hrx::host_memory_available_bytes, host_raw) takes the reader rather than the value, so the function cannot be called with a term that was measured in some other context and then classified as if it belonged to this one. That is the bug in one line — a clang build classified a term computed elsewhere.

raw is reset to 0 before the read, so a failed read yields 0 rather than a stale value — failing closed, which for a memory guard is the only direction that is safe.

CI green on this head. I read the diff and CI; I did not build it, and say so rather than implying more.

[bong-water-water-bong]

VERDICT: READY at fc9fdcf

This is the best kind of defect report: it fixes a line I reviewed earlier (#259/#271) and shows that the fix was itself broken by a language rule neither review looked at.

pool_host_term(host_raw, host_memory_available_bytes(host_raw))

host_raw handed by value while a sibling argument fills it. Saying it precisely: this is not merely "unspecified order" — reading an object in one argument while another argument modifies it is unsequenced, i.e. undefined behaviour; clang happening to copy first is one legal outcome, and it produces (0, readable = true), a "measured zero" on a host with tens of GiB free. The consequence chain you document (open-time guardrail refuses the device, no device registered, --device HRX0 invalid, live check refuses every allocation) is exactly the failure mode the original fix was meant to prevent, now produced by the fix. That g++ evaluates right-to-left and the pinned toolchain's clang does not is the reason it survived local testing — worth keeping in the comment, because it is the thing that makes this class invisible.

The fix is right and it is right for a reason worth naming: pool_host_term_read() makes the read one full expression of its own, so the value is complete before it is used, and it passes the reader in as a parameter so the three cases (unreadable, measured zero, measured non-zero) are testable without a live /proc. Clearing raw when the reader fails — "a reader that failed may have written to its out parameter; that is not a reading" — is the kind of detail that would otherwise become the next wrong "measured zero".

Two asks:

1. Push the same idiom one level up. The bug is a shape ("out-parameter and its use in the same argument list"), not a call site. platform.h now has a helper; if any other call passes an out-param to a function while also consuming it, it has the same UB and only one compiler will tell you. A grep in the PR body for \w+\((\w+), .*\1\) (or simply naming the two sites you checked) closes the class rather than the instance.
2. Say which compilers. "clang left-to-right, g++ right-to-left" is true of the versions here and is the useful fact; both are permitted, so a future compiler change can flip the symptom back without changing the code. One clause naming the pinned versions makes that reproducible.

The test with three readers (64 GiB, a measured zero, an unreadable one) is the right shape and is the reason this will not silently regress.

[bong-water-water-bong]

On the one ask ("this one has no arm"): the arm is in the diff — tests/test-hrx-pool-refusal-budget.cpp gains the stubbed-reader cases against pool_host_term_read(): a reader returning 64 GiB must yield 64 GiB (and leave the open-time budget and a 1 MiB live-check request intact), a measured zero must still refuse, an unreadable reader (which scribbles 777 on its out parameter) must yield kHostUnknown with a 0 log figure, and a null reader is unknown. 56/56 under both amdclang++ and g++; the pure pool_host_term(raw, readable) cases were already there from #271. Head is unchanged at fc9fdcf.

[bong-water-water-bong]

VERDICT: READY at fc9fdcf

This is a one-expression bug with a class of consequences, and the fix is written so the expression cannot come back.

The defect: both guardrail call sites read the host term as

size_t host_raw = 0;
const size_t host_available = pool_host_term(host_raw, host_memory_available_bytes(host_raw));

— host_raw is read as an argument in the same full-expression that is supposed to have written it. With clang the evaluation order is unspecified, so the reader can see host_raw before, after, or interleaved with the write; the guardrail then prices the host term from a value nobody chose. This is the compiler-dependent cousin of the bug I reviewed as engine ggml-org#1768 (host_raw == 0 ? kHostUnknown : host_raw collapsing "unreadable" and "genuinely zero"), and both end in the same place: an admission decision made from a number that was never read.

Why pool_host_term_read() is the right shape rather than a reordering:

• the read is its own statement (const bool readable = reader != nullptr && reader(raw);), so sequencing is guaranteed by the language and not by an unspecified evaluation order that one compiler happens to give you;
• a failed reader zeroes raw — "a reader that failed may have written to its out parameter; that is not a reading" — which preserves the distinction pool_host_term needs (unknown versus zero) at the point where it is still knowable;
• it is a function, so both call sites share it — the original bug existed in two files with the same text, which is how a fix gets applied once and reviewed as if twice.

One ask, and it is the arm that makes this more than a refactor: a case where the reader returns false after writing a plausible value, asserting the guardrail treats it as unknown. That is the branch the zeroing exists for, it is invisible in every normal run, and it is exactly what distinguishes this fix from "I reordered the statements".

Worth noting for the merge record: this is the fork half of engine#1942, so the engine pin bump that carries it should say so — the pattern that ggml-org#1618's pin bump already established tonight.