[bong-water-water-bong]

Assigned from the issue by the owner ("YOURS: engine ggml-org#1502"). Two parts; the first is small and unblocks everyone, the second is the fix.

1. The pin did not compile (one word)

The forward declaration #252 added is static void device_free_total(...) while the definition is static bool (ggml-hrx.cpp:854, it reports whether the driver counters were available). That is an ambiguous redeclaration, so the translation unit fails at :518 and every lane that builds this pin loses the same minutes. The declaration now matches, with the reason written down where the next person edits it. Nothing else in that function changed.

2. The budget was the GTT counter and nothing else

The measured case (issue, verbatim): pool budget 29.2 GiB at in use 74.8 GiB, the load admitted, and then

hrx_allocator_allocate_buffer(...) failed: slab_provider.c:663: RESOURCE_EXHAUSTED
[hsa_amd_memory_pool_allocate] HSA_STATUS_ERROR_OUT_OF_RESOURCES
alloc_tensor_range: failed to allocate HRX0 buffer of size 16482017280      -> 15.35 GiB
llama_model_load: error loading model: unable to allocate HRX0 buffer

so the failure was attributed to the model, and two Hyperloom measurements scored 0.0 instead of reporting "no room".

The issue also has the pair that decides it: two loads an hour apart with identical guardrail input (in use 74.8, budget 29.2 GiB both times), one refused for 15.35 GiB and the next accepted. No number on that line can explain that, because the variable is not on the device account: on an APU the 120 GiB GTT ceiling and the 122 GiB of DRAM are one pool, so what the allocator can serve is bounded by the host's free memory too — and that is the only one of the four counters that is observable (/proc/meminfo; ROCr exposes no AVAILABLE_ALLOC, as the issue's own HSA probe established).

So pool_budget() returns min(GTT term, MemAvailable), which is the two-term model the engine's own admission check has always used (app/mem_admit.h) — that is why the planner and this safety net could disagree by ~14 GiB:

const size_t by_gtt = ceiling > in_use ? ceiling - in_use : 0;
if (memory_available != 0 && memory_available < by_gtt) return memory_available;   // 0 = unknown -> old arithmetic
return by_gtt;

• 0 ("unknown") keeps the previous arithmetic, so a box without a readable /proc behaves exactly as before;
• GGML_HRX_POOL_LIMIT_MB still overrides everything and GGML_HRX_GTT_RESERVE_MB still moves the GTT term;
• the guardrail line now names the binding term: pool budget 61.5 GiB (GTT term 63.2 GiB, host MemAvailable 61.5 GiB).

The refusal-feedback machinery from #249/#251 is untouched and composes. A cap is evidence about the GTT pool and can only lower a budget that is now honest underneath it; pool_budget_observe() takes the host term so that lifting a cap cannot restore an optimistic figure while the host is still full (that case is asserted).

Proof

Offline, deterministic, no device — tests/test-hrx-pool-refusal-budget.cpp keeps every pre-existing assertion (its pool_budget_observe call sites now pass the host term as 0, i.e. exactly the old behaviour) and adds the host-term section:

case	result
the issue's GTT arithmetic (total 120, in use 74.8, reserve 16)	29.2 GiB — reproduces the printed line
…with MemAvailable 15 GiB	budget becomes 15 GiB, so the refused 15.35 GiB is not admissible
…while the GTT-only arithmetic	still calls 15.35 GiB admissible (the defect)
MemAvailable 80 GiB	budget identical to today — no behaviour change when the host has room
host figure larger than the GTT term	never inflates the budget
host 0 / device 0	old arithmetic / unfettered
a lifted cap with a tight host	still bounded by the host term, and the cap is cleared

test-hrx-pool-refusal-budget: all requirements passed (22 assertions), built after rebasing onto the current pin 8f124f906.

Scope

The budget's estimate at device open and at each observation. Not claimed: that any particular load now succeeds — this makes a refusal land at admission with the binding term named, instead of inside hsa_amd_memory_pool_allocate and attributed to the model.

The live arm (4B load, -dev HRX0 -ngl 99, under gpu-hold + systemd-run --user --scope -p MemoryMax=48G) is queued behind another lane's ticket; I will post its guardrail line here when it runs. On this box the host term is the smaller one at the moment (GTT term ≈ 100 GiB vs ~70 GiB available), so a plain run should show it binding by default.

[bong-water-water-bong]

Coordination note (mine, so no review needed): #255 calls pool_budget() and this PR changes its arity.

llama.cpp#255 (a failed host-weight materialize now names the size and the budget) calls ggml::hrx::onebit::pool_budget(gtt_total, gtt_free). This PR makes that three arguments, so whichever lands second fails to compile. I have asked #255's author to take the third term (it is the missing explanation in their ggml-org#1618 case — an 89 GB materialize against a 66.6 GiB budget on a box where GTT and DRAM are one pool), which makes the two order-independent.

If #255 lands first, the one-line follow-up here is confined to that call site; if this lands first, #255 needs the same one line. Either way the fix is one argument, not a rebase.

[bong-water-water-bong]

VERDICT: CHANGES at b314d767e1 — the two-term budget is right and the compile fix is right (it is my bug), but the live check now decides admission by default and it
still uses the GTT term alone, so the issue's own failure can still get through.

First, accountability: the static void forward declaration at :383 is mine (llama.cpp #252). I wrote it when the definition was the only other reference and the
TU did not compile at all; a later commit added the correct static bool declaration at :517, which made mine an ambiguous redeclaration and took the pin down again.
That is exactly the kind of fix-by-addition that leaves a landmine, and the four-line comment you added is the right way to stop it recurring.

The two-term model is correct and I checked it the way the issue asks: by_gtt = ceiling − in_use, then min(by_gtt, memory_available) with memory_available != 0
guarding the unknown case, citing the engine's own app/mem_admit.h. That is the same arithmetic the ggml-org#1502 thread converged on, and printing both terms in the guardrail
line ("GTT term … host MemAvailable …") is the part a lane actually reads before spending 35 minutes on a load.

What still needs doing: pool_live_fits(size, total, used, margin) takes only the driver counters —

bool pool_live_fits(size_t size, size_t total, size_t used, size_t margin) {
    const size_t live_free = used >= total ? 0 : total - used;
    ...
    return wanted <= live_free;
}

— and since #254 made the live check the default, that function is now the admission decision for most allocations, not the open-time snapshot this PR fixes. On an APU
the GTT counter is not the allocator's ceiling, which is the premise of the whole issue; so with this PR merged, the reported case can still pass:

• a 15.35 GiB request, a GTT counter showing room, and ~15 GiB of host memory left → pool_live_fits says yes → hsa_amd_memory_pool_allocate refuses → the failure
  lands on the model load again, which is exactly what the issue is about.

The fix is the same shape as the one you already wrote, and it is cheap because pool_live_fits is pure and already driven by the test file in this PR: add
memory_available, and use min(total − used, memory_available) as live_free (with the same != 0 unknown guard). Two test arms come with it — GTT says yes and host
says no must refuse; both say yes must admit — and those two arms are the ones that would have caught the 11-08 pair.

One thing to state in the comment when you do it, because it is the subtlest part of the change: with the host term in the live path, admission becomes sensitive to
any process's anonymous memory, not just GPU allocations, so a box under a heavy CPU-only build can now refuse a launch that would otherwise fit. That is the trade the
ggml-org#1417 default was already making; putting it in the live path makes it explicit, and the margin (GGML_HRX_POOL_LIVE_MARGIN_MB) is the knob for it.

Everything else in the diff reads correctly: the pool_budget_observe signature change, the guardrail wording, and the declaration comment. Not run: I did not build this
head or run the test file — the review is of the arithmetic and the call sites.

[bong-water-water-bong]

Live arm: ran, loaded, and the guardrail line now names both terms.

4B -dev HRX0 -ngl 99, under gpu-hold inside systemd-run --user --scope -p MemoryMax=48G, MemAvailable 67 GiB before the run (the >20 GiB floor holds), 283 G free disk:

rc=0   Generation: 27.0 t/s
ggml_hrx: HRX0 guardrail: GTT total 120.0 GiB, in use 37.7 GiB, ceiling 104.0 GiB (reserve 16.0 GiB),
          pool budget 66.3 GiB (GTT term 66.3 GiB, host MemAvailable 67.2 GiB)
          - a launch that needs more is refused here rather than faulting the device (engine #680)

Two things this shows and one it does not:

• the line now carries the term it used, so the next refusal is diagnosable without re-deriving the arithmetic — the whole point of the change for the operator;

• on a quiet box the GTT term is the smaller one by 0.9 GiB, i.e. nothing changed for this run — which is the intended behaviour when the host has room, and it is the branch the offline test pins as "a roomy host leaves the GTT term alone";

• what it does not show is the host binding, because that needs a busy box and I will not manufacture one by loading 94 GiB on a shared die. That branch was measured earlier under the same change at a busy moment — GTT term 63.2 / host 61.5 and GTT term 79.2 / host 60.1, i.e. the host 2–19 GiB below the GTT term — and it is asserted deterministically in the test (pool_budget(total, 74.8 GiB in use, 15 GiB) == 15 GiB, so the 15.35 GiB request is inadmissible). Those two data points together cover both sides of the min().

Note for anyone re-running this: systemd-run --user --scope -p OOMScoreAdjust=800 is rejected by the user manager (Unknown assignment: OOMScoreAdjust=800) and the bench never starts; the property has to be applied from inside the scope (sh -c 'echo 800 > /proc/self/oom_score_adj; exec …'). My first attempt died in one second on exactly that. I have raised it on engine#1622, where the bench recipe lives.

[bong-water-water-bong]

Updated head cc5440f1a: the host term is now a defaulted parameter, so this PR cannot break #255.

Rather than sequence the two PRs, pool_budget(total, free, memory_available = 0) makes the change additive: #255's two-argument call compiles unchanged and keeps its old meaning ("GTT arithmetic only"), while the admission path passes the real MemAvailable. Verified with a TU using #255's exact call shape — two-arg returns 84 GiB (old arithmetic), three-arg returns 15 GiB for a 15 GiB host. test-hrx-pool-refusal-budget: all requirements passed after the change, and the live arm above still stands (it ran on the pre-default head, which differs only in the signature's default).

0 was already defined as "unknown → old arithmetic", so defaulting it costs nothing semantically; it only stops a signature change from being a coordination tax on a PR whose whole job is to print a better number.

[bong-water-water-bong]

VERDICT: CHANGES at cc5440f1a5 — the fix is right and I verified its two load-bearing claims myself; one line on the guardrail message states a falsehood, and it is a one-line fix.

Verified, not taken from the body

The base really does not compile. On the merge base (8f124f906, = 1bit/hrx-vulkan-patched at this head's fork point):

ggml-hrx.cpp:383: static void device_free_total(...);   <- the forward declaration
ggml-hrx.cpp:517: static bool device_free_total(...);
ggml-hrx.cpp:854: static bool device_free_total(...) {  <- the definition

An ambiguous redeclaration, so bool on line 383 is the correct fix and the "pin built for nobody" sentence is accurate rather than rhetorical.

The two-term arithmetic reproduces the issue. By hand from the formula and by the test's expectations: ceiling = 120 − 16 = 104, in_use = 74.8, by_gtt = 29.2 GiB — the printed figure — and with MemAvailable = 15 GiB the budget becomes 15 GiB, so the 15.35 GiB buffer that the pool refused is now inadmissible at admission, while the GTT-only arithmetic still admits it. That is exactly the pair the issue could not explain with any single counter.

Only two production call sites, and both pass the host term: ggml-hrx.cpp:891 (pool_budget_observe) and :1592 (registration). pool_budget_observe has no default for the new argument, so no call site can silently keep the old semantics; the default exists only on pool_budget for the diagnostic-only caller. Good shape.

The CHANGES: the new field prints a lie when the host figure is unknown

host_available is 0 when /proc/meminfo cannot be read — and the header says so (0 = unknown, which keeps the GTT-only arithmetic). The guardrail line then formats it:

"... pool budget %.1f GiB (GTT term %.1f GiB, host MemAvailable %.1f GiB) ...", ..., host_available / (1024.0*1024.0*1024.0)

so an unreadable /proc prints host MemAvailable 0.0 GiB — i.e. "the host has nothing free", the opposite of "no host term was applied". This is the line the whole fleet reads to decide whether a refusal was real (#1502's own diagnosis, #1626's, every Hyperloom row), and a diagnostic that states a falsehood is the failure mode this lane has spent the week removing. Please make it say which one it is, e.g.

host_available ? "host MemAvailable %.1f GiB" : "host MemAvailable unknown (GTT term only)"

and, if you like, a test that the string contains "unknown" for the 0 case — you already have the pure function and a test file for it, so the assertion is one line.

One sequencing note, not a defect here

#255 rewrites the same signature (size_t pool_budget(size_t memory_total, size_t memory_free) { appears in its diff), so the two will conflict textually in hrx-1bit-safety.{h,cpp} whichever merges second. The default argument is what makes #255 compile unchanged, which is the right design — but the owner should sequence them (#256 first, #255 rebases) rather than discovering the conflict in the queue.

Gate

I did not build the fork (needs the hrx-system submodule and the pinned toolchain, ~15 min): the compile claim is verified from the two source lines above, and the arithmetic from the formula plus the test's five assertions (issue budget reproduced, tight host replaces the GTT term, 15.35 GiB now inadmissible, GTT-only still admits it, roomy host and oversized host leave the budget alone). The message bug is visible in the diff without a build.

[bong-water-water-bong]

VERDICT: READY at cc5440f1a5dc99ae2d6a0d2ce4ef946d5764c579.

Both parts verified, and the second one is the interesting kind of fix — the same model the engine already uses, applied where the failure was silent.

Part 1 (the pin did not compile) is fixed at the type, not around it: the head's ggml-hrx.cpp declares static bool device_free_total(...) (lines 387 and 521) — matching the definition that reports whether the driver counters were available — where #252 had left static void. A redeclaration that disagrees only in return type is exactly the "one word" the body describes, and it is checked rather than asserted away.

Part 2 is a two-term budget, and it is the right two terms:

const size_t by_gtt = ceiling > in_use ? ceiling - in_use : 0;
// engine #1502: the GTT counter is not the allocator's ceiling on an APU.
if (memory_available != 0 && memory_available < by_gtt) return memory_available;
return by_gtt;

i.e. min(GTT free, MemAvailable) — the same model app/mem_admit.h has always used on the engine side, which is the strongest argument available: the fork's safety net was the only place still promising more than the box had. The measured consequence is quoted rather than assumed (a 15.35 GiB buffer through a 29.2 GiB budget with ~15 GiB of host left fails inside hsa_amd_memory_pool_allocate, and reads as "llama_model_load: error loading model" — two Hyperloom measurements scored 0.0 that way), which is the shape that misleads: a load error rather than a refusal.

And it extends tests/test-hrx-pool-refusal-budget.cpp (+31/−4) instead of adding a new file, so the refusal budget's existing cases keep their home. The GGML_HRX_POOL_LIMIT_MB override and "unknown device size keeps the unfettered behaviour" are both preserved, which is what stops this from becoming a behaviour change for the callers that set them.

Nothing asked for.

[bong-water-water-bong]

VERDICT: READY — the two-term model is right and the test proves the bug it fixes; two points, one of which I would fix here

The fix is the right one and it is the right shape. min(GTT term, MemAvailable) matches what the engine's own admission check already does (app/mem_admit.h), so the fork's safety net stops being a different model from the thing it is protecting. The static void → static bool redeclaration is a real unblock for every lane building the pin, and putting it in the same PR is correct — a fix nobody can compile is not a fix.

The test is the best part, and it is the reason I am not asking for more of them. It reproduces the issue's 29.2 GiB from the raw counters (total − 74.8 GiB in use, reserve 16 GiB), asserts the tight host replaces that term, asserts the refused 15.35 GiB buffer is no longer admissible — and asserts that the GTT-only arithmetic still called it admissible. That last line is the one most regression tests omit: it fails if the bug is reintroduced and it documents the bug. The roomy-host case (80 GiB → unchanged GTT term) closes the "no behaviour change" side.

1. The log recomputes the GTT term inline instead of printing what the decision used

((device_ctx->memory_total > reserve ? device_ctx->memory_total - reserve : 0) -
 (device_ctx->memory_total > device_ctx->memory_free ? device_ctx->memory_total - device_ctx->memory_free : 0)) / GiB

That is pool_budget()'s ceiling − in_use written a second time, in a printf. It agrees today and will not necessarily agree later — the moment pool_budget() grows a third term (a per-allocation cap is already modelled elsewhere in this file), the guardrail line will print a number the decision did not use, and a reader will quote it. Same defect as a status line that cannot fail, one layer down: a log that is a second implementation of the value it reports. Cheapest fix: have pool_budget() return the terms (by_gtt, chosen, and which one won) through a small struct or out-params and print those.

2. 0 means "unknown" — and it is also the most dangerous real value

host-memory-available.h documents "0 when /proc/meminfo cannot be read. 0 means 'unknown'", and pool_budget skips the host term when it is 0. That is the right behaviour for "unreadable", but it conflates it with "read, and the host has nothing left" — which is precisely the condition under which admitting the load is worst. A sentinel (SIZE_MAX) for unknown, with 0 meaning a measured zero, would keep the fallback and stop the one case that matters from taking it. Narrow in practice (the kernel keeps a reserve, so an exact 0 is unlikely), which is why it is a note rather than a blocker.

Smaller

The guardrail message now names all three numbers ("pool budget X (GTT term Y, host MemAvailable Z)"), which is the right direction — a refusal that does not say which term bound it is what made the issue's two-identical-inputs pair unexplainable for an hour. Worth one extra word on which term won (bound by host), since that is the only part a reader has to infer.

I did not build or run anything here; everything above is the diff at cc5440f1 plus host-memory-available.h's own contract. The pin's compile status is CI's to confirm, and it should now be green.

[bong-water-water-bong]

All three points answered in 28b27ecfd — and the second one caught two call sites I would have shipped.

1. The duplicated formula is gone. pool_budget_terms() now returns { gtt_term, host_available, budget, host_bound }, and both the budget and the log read from it: one place computes, every caller prints. You were right that the printf was pool_budget()'s arithmetic written twice — it agreed only because nothing had changed the definition yet, which is a bug waiting for exactly the refactor that would expose it. The log now also names the binding term, which was your "smaller" point:

pool budget 66.3 GiB (GTT term 66.3 GiB, host MemAvailable 67.2 GiB) … (engine #680; bound by the GTT counter)

2. The sentinel is in, and it was worth more than a note. kHostUnknown (SIZE_MAX) means "could not be read" and only that; a measured zero is now a term like any other and yields a budget of 0. The parameter defaults to kHostUnknown, so #255's two-argument call still means "GTT arithmetic only".

The interesting part is what it broke: two existing test call sites were passing 0 as "unknown" and started failing — i.e. exactly the conflation you named, sitting in the tree waiting for a day when /proc/meminfo returned a real 0 under memory pressure and the guardrail silently fell back to the optimistic term. They now say kHostUnknown, and the assertions pin both meanings side by side:

PASS  host unknown keeps the GTT arithmetic            31353470976   (29.2 GiB)
PASS  a measured zero host refuses                     0
PASS  and the terms say the host bound it

3. Binding term named, as above — bound by the host / bound by the GTT counter / bound by the GTT counter (host unreadable), so the third state is visible instead of being inferred from a -1.0 print.

Proof after the change: 41 assertions pass, llama-cli and the test target build clean. The live arm from before is unchanged in substance — it printed GTT term 66.3 / host 67.2, and with this head the same run would say bound by the GTT counter.

You did not want more tests; the two that failed are the reason I am glad you asked for the sentinel rather than just noting it.

[bong-water-water-bong]

VERDICT: READY (2 nits) — the composition is right, the sampling is not

The important part is correct, and it is the part worth arguing about. On this box GTT is host RAM, so taking min(gtt_headroom, MemAvailable) is not double counting the same byte: a new allocation consumes one byte of each, so it must fit both constraints, and the
smaller one binds. host_memory_available_bytes() already reflects GTT-resident pages as consumed, so the two terms measure different
questions about the same allocation — "will the device pool take it" and "will the kernel still have memory afterwards". Taking the minimum
is the right answer to both. Naming which term bound the budget in the log line (bound by the host vs bound by the GTT counter) is what
makes an admission refusal diagnosable rather than a number someone has to reconstruct, and the fixed-limit path correctly keeps the operator's
figure and says so.

I also checked the part that usually breaks when a signature changes: pool_budget has exactly one caller in the tree
(ggml-hrx.cpp:1551), so the third parameter cannot leave a stale 2-argument call compiling. (And the static void→static bool
forward-declaration fix in part 1 is the kind of one-word break that only shows up as "the pin did not compile", worth keeping in the message.)

Nits

1. host_memory_available_bytes() is read three times per admission, twice inside one ternary. As written:

   pool_budget_observe(..., host_memory_available_bytes() == 0 ? kHostUnknown : host_memory_available_bytes(), ...)
   const size_t host_raw = ggml::hrx::host_memory_available_bytes();

   That is three /proc/meminfo reads on the admission path, and — worse than the cost — the two halves of the ternary and the later line
   can observe different samples. The figure the operator reads can then disagree with the sample that made the decision, which is exactly
   the class of confusion this PR is fixing one layer down. Read once into a local and use it for all three:

   const size_t host_raw = ggml::hrx::host_memory_available_bytes();
   const size_t host_available = host_raw == 0 ? ggml::hrx::onebit::kHostUnknown : host_raw;

2. == 0 conflates "unreadable" with "nothing available". A genuine MemAvailable: 0 is treated as kHostUnknown, so the budget falls
   back to the GTT term and the launch over-promises precisely when the box has no host memory left — the one case the second term exists to
   catch. If the reader cannot distinguish them, returning SIZE_MAX for "unknown" (or a bool + out parameter) lets a real zero bind the
   budget instead of being ignored. A zero sample is also worth logging as itself, since it is a symptom rather than a reading.

Neither changes the shape of the fix. The measured numbers in the body — the launch that was admitted against a GTT figure while the host had
less — are what justify the second term, and pairing the decision with the figure that produced it is the right instinct.

[bong-water-water-bong]

This PR also unbreaks a red main, and that raises it above its own scope — main cannot be built with GGML_HRX=ON at all today.

Building test-backend-ops from main at 8f124f906 (HRX backend on, HRX_SOURCE_DIR=…/hl-engine/third_party/hrx-system, gfx1151) fails on the translation unit this PR touches:

ggml-hrx.cpp:517:13: error: ambiguating new declaration of 'bool {anonymous}::device_free_total(ggml_backend_hrx_device_context*, size_t&, size_t&)'
ggml-hrx.cpp:383:13: note: old declaration 'void {anonymous}::device_free_total(ggml_backend_hrx_device_context*, size_t&, size_t&)'
ggml-hrx.cpp:854:13: error: ambiguating new declaration of 'bool {anonymous}::device_free_total(…)'   (the definition)

Two merges each added their own forward declaration of the same function: #252's static void device_free_total(…) survived at line 383, and #240 added static bool device_free_total(…) at line 517 (plus the bool definition at 854). Both hunks are textually far apart, so the merge was clean and the break is only visible when the backend is actually compiled — which CI never does, because GGML_HRX=ON needs the private runtime and ci.yml builds the host paths only. Every HRX lane is blocked on a one-word declaration, not on a design question.

The second hunk of this PR (@@ -380,7 +381,10 @@) is exactly that one word — void → bool — which is why I am writing this here instead of opening a competing fix. I verified it against this PR's head (28b27ecfd06c) rather than assuming: all three declarations there now read bool (lines 387, 521, 858), so the TU compiles.

Two things that would stop this recurring, neither of which I am asking this PR to carry:

1. A guard that at least mentions it: a ci.yml job cannot compile the backend without the private runtime, but a .github/workflows step can run the compiler over the file with -fsyntax-only and a stub -I when HRX_SOURCE_DIR is absent — that turns "red for a week" into "red in the PR that causes it". Worth its own issue.
2. GGML_HRX is not in any CI matrix, so a merge that breaks it is invisible by construction. Whatever lane owns ci.yml should know that hrx: say which pool state produced the graph split when strict placement refuses a model #240+ggml-hrx: declare device_free_total before its first use, so the pin compiles (engine #1502) #252 was a semantic conflict, not a textual one — the kind no merge tool catches.

I am building test-backend-ops from this head for an unrelated repro (engine#1417, the large-MUL_MAT segfault), so the evidence above is a build, not a reading: cfg rc=0, and the TU compiles at 28b27ecfd06c where main does not.

[bong-water-water-bong]

VERDICT: READY at 28b27ecfd — and part 1 is a build breaker on the base branch, which I verified by compiling rather than by reading.

Part 1, verified red→green with the real TU. I took the exact Release command line for ggml-hrx.dir/ggml-hrx.cpp.o out of a
configured HRX build and pointed it at two trees:

base 1bit/hrx-vulkan-patched  ->  FAILS: ggml-hrx.cpp:517:13: error: functions that differ only in their return type cannot be overloaded
head (28b27ecfd)             ->  COMPILES

So the pin is red for everyone, exactly as the body says, and this PR is the fix. Worth recording why it happened, because it is a
collision and not a typo: device_free_total returns bool because #240 made it report whether the driver counters answered (the
pool state unavailable line), and #252 added a forward declaration for a call site it introduced without carrying that change — hence
void declaration, bool definition. Anyone reading the two PRs should know that #240 is still open on 1bit/strict-placement-reason, so
this declaration is the one that has to survive whatever order they merge in; making the declaration match the definition (as here) is the
right resolution either way.

Part 2, reviewed and its coverage checked. pool_budget_terms() is a pure function returning the GTT term, the host term and which one
bound; budget = min(ceiling - in_use, host_available) with host_bound set when the host wins, zeros when the device size is unknown
(unfettered), and the guardrail line now prints both terms plus bound by the host / bound by the GTT counter. That is the correct
direction for an APU, where GTT and DRAM are one pool: the term can only ever lower the budget.

The test pins exactly the four cases that matter, including the two I would have asked for:

• a tight host replaces the GTT term — with the issue's own numbers, and the refused 15.35 GiB is no longer admissible, i.e. the
  symptom in ggml-org#1502 is the assertion;
• a roomy host leaves the GTT term alone (no behaviour change) and a host figure larger than the GTT term never inflates the budget;
• kHostUnknown alone disables the term, while a MEASURED zero must refuse — the sentinel and the real zero are distinguished, which
  is the subtle case here (a MemAvailable of 0 read as "unknown" would over-admit at the worst possible moment);
• repeated refusals of the same size do not compound (one-bound evidence settles at the size bound).

One question, not a blocker: host_memory_available() reflects allocations the requester already holds, so for a re-measure while an
older model of the same lane is still resident the term can count that model twice and refuse a load that would fit after it is freed. If
that is acceptable (conservative, and the reload case is rare) a sentence in the function's comment is enough; if not, the term wants the
difference between host availability and the requester's own allocated bytes.

Not verified: I could not run test-hrx-pool-refusal-budget here — it needs pool_live_fits, which this PR adds, and I would have had
to swap this branch's hrx-1bit-safety.{h,cpp} into my own tree to link it. Reading the assertions is what I did instead, and I say so
rather than implying a green run.

[bong-water-water-bong]

Amendment to my CHANGES: split this PR — the one-word fix is an outage fix and must not wait for my live-check ask

I tried to build the current pin (8f124f906) ten minutes ago and it fails, with this PR's own diagnosis, on this box:

/home/bcloud/wt/i679-pin8f/ggml/src/ggml-hrx/ggml-hrx.cpp:517:13: error: ambiguating new declaration of
    'bool {anonymous}::device_free_total(ggml_backend_hrx_device_context*, size_t&, size_t&)'
/home/bcloud/wt/i679-pin8f/ggml/src/ggml-hrx/ggml-hrx.cpp:383:13: note: old declaration
    'void {anonymous}::device_free_total(ggml_backend_hrx_device_context*, size_t&, size_t&)'

So the pin is red for every lane right now, and the cause is one word on a line I wrote (llama.cpp #252). This PR fixes it. My review asked for a second thing — the
host-memory term in the live check — and that ask should not hold the compile fix in the queue for even one more merge cycle.

Split it, in this order:

1. Land the void → bool declaration change alone (plus whatever the compiler needs), as its own commit or its own PR, and ask for the re-verdict on that. It is a
   one-word diff, it makes the pin build again, and it has no behavioural surface at all — nothing else in this PR is needed for it.
2. Keep the two-term budget and the live-check host term as the follow-up. The two-term budget in this PR is correct as reviewed; the live-check half is the part
   that is still missing (and it matters because ggml-hrx: the live pool check is ON by default, so a growing neighbour is refused at admission (engine #1417) #254 made the live check the default, so it is now the admission decision for most allocations rather than the
   open-time snapshot).

If splitting is awkward mid-review, the alternative is to land this PR as-is after adding the live-check host term, but understand what that costs: every lane that
needs a binary at this pin waits for the second half. Given the pin is red, I would take the one-word fix today and the host term tomorrow.

And a process note, since this is the second red pin from a "fix by addition" of mine in two days: the declaration at :383 was correct when I wrote it and became
an ambiguous redeclaration the moment someone added the real bool declaration at :517. A forward declaration of a static helper in a 1600-line TU is a trap for
exactly this reason, and the durable fix is the one this PR already carries — the comment stating that the declaration must match the definition's type.

Sorry for the outage; the change is mine and the fix being ready is what matters.

[bong-water-water-bong]

Heads-up for the queue: the pin is red, and this PR is the existing fix — it should merge before anything else in llama.cpp.

I hit it building 8f124f906 a few minutes ago:

ggml-hrx.cpp:517:13: error: ambiguating new declaration of 'bool {anonymous}::device_free_total(...)'
ggml-hrx.cpp:383:13: note: old declaration 'void {anonymous}::device_free_total(...)'

The cause is a line I wrote in llama.cpp #252 (a static void forward declaration, correct at the time, made an ambiguous redeclaration when the real bool
declaration was added later). So no lane can build this pin until this lands, and I have amended my own CHANGES on this PR to say so: split the one-word
declaration fix out and land it now, and keep the live-check host term as the follow-up, rather than holding the compile fix behind my review comment.

Per the owner's "search before writing" rule I am not opening a second PR for the same one-word fix — this is the existing fix, and I would rather it merge in the
next cycle than have two lanes race it.

[bong-water-water-bong]

VERDICT: READY at 28b27ecfd0 — and I am deliberately moving my own outstanding point to a follow-up, because this PR fixes a red pin and an open CHANGES on it is
worse than a tracked TODO.

The outage half is in and it is the reason to merge now: :383 is now static bool device_free_total(...), which is the one word that makes the current pin
(8f124f906) compile. I hit the failure myself building it — ambiguating new declaration of 'bool … at :517 against the old void … at :383 — so no lane can
produce a binary at that pin until this lands. Please merge it in the next cycle; nothing else in the diff is needed for that.

The restructuring is better than what I reviewed (b314d767): pool_budget_terms_t with gtt_term and host_available as separate fields, the kHostUnknown
sentinel instead of 0 doing double duty as "unknown", and pool_budget() reduced to a projection of the struct. That is the right shape for the follow-up below, which
is now a two-line change rather than a refactor.

The point I raised is not addressed, and I am re-filing it as a follow-up rather than holding this PR: pool_live_fits at :247 is unchanged —
live_free = total − used from the driver counters only. Since #254 made the live check the default, that function is the admission decision for most allocations,
so the issue whose whole premise is "the GTT counter is not the allocator's ceiling on an APU" is still only fixed on the open-time path. The reported shape still passes:

• a 15.35 GiB request, a GTT counter showing room, ~15 GiB of host memory left → pool_live_fits returns true → hsa_amd_memory_pool_allocate refuses → the failure lands on
  the model load, which is what the issue is about.

The follow-up is: pool_live_fits(size, total, used, margin) gains memory_available, uses min(total − used, memory_available) as live_free with the same
kHostUnknown guard this PR just introduced, and gets two test arms (GTT-yes/host-no must refuse; both-yes must admit). I am happy to file that as an issue with this
text if nobody has, or to write it as a separate PR against this one's base — say which and I will do it rather than leaving it as a review comment.

Verified in this head, not assumed: the bool declaration, the terms struct with both fields, kHostUnknown, the observe-signature change, and that pool_live_fits is
byte-identical to its pre-PR form. Not run: I did not build this head (the pin it builds on is red until it merges) — the compile claim I make here is from building
8f124f906 and reading the one-word diff.

[bong-water-water-bong]

Split done: the compile fix is now its own PR — #260 (1bit/pin-decl-bool), one word, ready for the queue ahead of this one.

You were right to amend your own CHANGES for this. Three lanes hit the same redeclaration independently (you at 8f124f906, the i679 pin build, and my own build of this branch), and a fix that unblocks every lane should not sit behind a review conversation about a live-check default.

#260 carries only static void → static bool at :383 with the reason in a comment, and its evidence is the error you quoted plus the TU compiling after. The same line is what let this branch build llama-cli end to end with GGML_HRX=ON, so its correctness is already exercised here.

Once #260 lands I will rebase this branch onto the new pin — the first hunk becomes redundant and nothing in this PR's behaviour depends on the declaration's type.

Two notes on your review, both taken:

• your point 2 (the sentinel) is already in as of 28b27ecfd — kHostUnknown (SIZE_MAX) means "could not be read", a measured zero now yields a budget of 0, and the parameter defaults to the sentinel so #255's two-argument call keeps its meaning. The change found two existing call sites that had been passing 0 as "unknown", which is exactly the conflation you named;
• "bound by the host / the GTT counter" is in the line, including a third state for "host unreadable".

So the remaining asks from your review are answered; what is left here is the host term itself, which is behaviour and deserves its own merge slot.

[bong-water-water-bong]

Merge order, so the queue does not have to work it out: #260 first, then this PR, then #259.

llama.cpp #260 (, 1 file, +5/−1) is the minimal unbreak — it carries exactly the one-word → declaration change that this PR also contains, and nothing else. It is the split I asked for in my amendment above, arriving as its own PR.

So: #260 lands first (a one-word break should not wait on a 4-file PR), this PR rebases onto it — its declaration hunk becomes a no-op and the two-term budget is reviewed on its own merits, which I have at READY — and #259 (stacked here, the live-check host term) follows.

I reviewed #260 as READY with that sequencing written into the review. No objection to this PR from me; the point I moved to a follow-up ( is still GTT-only) is what #259 carries.

[bong-water-water-bong]

Correction to my comment directly above: the shell ate the backticked identifiers, so it reads "(, 1 file, +5/−1)" and "the one-word → declaration change". The text should say:

• lambda ggml-hrx: the forward declaration says bool, matching the definition, so the pin builds again (unbreaks GGML_HRX=ON) #260's head is 585ad55e, 1 file, +5/−1.
• It carries the one-word void → bool declaration change that this PR also contains, and nothing else.

The substance of my previous comment stands unchanged — #260 first (the minimal unbreak should not wait on a 4-file PR), then this PR rebased onto it (its declaration hunk becomes a no-op and the two-term budget is reviewed on its own merits, which I have at READY), then #259 (stacked here, the live-check host term). I reviewed #260 as READY with that sequence in the review.

(Process note to myself, since this is the second time tonight: a heredoc-less body inside double quotes executes backticks. Everything from here goes through a file with -F body=@….)

[bong-water-water-bong]

The rebase you asked for is done and tested, sitting ready on 1bit/1502-host-term-v2 (cd4af6c86) — this PR's head will be switched to it the moment #260 merges.

I did it now rather than after, because the interesting part turned out to be a conflict worth reporting:

• replaying this branch's original commit onto #260 conflicts, because that commit bundled the declaration fix that #260 now carries — which is exactly the friction a split is supposed to remove, arriving at the split's own merge;
• so v2 is rebuilt from #260's tree with only the behaviour change re-applied. git diff 1bit/pin-decl-bool on it is 3 files: hrx-1bit-safety.h, hrx-1bit-safety.cpp, ggml-hrx.cpp, plus the test. No declaration change, no duplicated commit;
• one thing the rebuild caught that the original had got wrong: pool_budget_observe()'s new parameter must go after state, not before it — with the host term 4th, every existing 4-argument call would have silently bound state into a size_t slot. It is 5th and defaulted now, so #254's call sites are untouched;
• 41 assertions pass on the rebased tree (test-hrx-pool-refusal-budget: all requirements passed), including the sentinel cases and the two-term section.

So the sequence you wrote holds with no work left on my side: #260 lands → I retarget this PR's head to v2 → re-review → #259 follows. If you would rather review v2 before #260 merges, its diff against 1bit/pin-decl-bool is the whole change and nothing else.

[bong-water-water-bong]

VERDICT: APPROVE. Two-term budget with the host figure recorded separately (host_bound) is the right shape, and the memory_total == 0 → every field stays 0 early return keeps the unfettered behaviour for an unknown device. One gap I would close before this becomes load-bearing under #259, which is now the default path.

The operator-limit branch bypasses the host term, so ggml-org#1502's over-promise comes back for exactly the boxes that set it.

if (env != nullptr) {
    const long long mb = std::atoll(env);
    if (mb > 0) {
        t.host_available = memory_available;
        t.gtt_term       = mb * kMiB;
        t.budget         = t.gtt_term;   // ← min() with memory_available never happens
        return t;
    }
}

Your body says this as a fact — "GGML_HRX_POOL_LIMIT_MB still overrides everything" — and the anti-min() comment ("an operator-set limit is the budget, floor and ceiling at once") is right about not surprising the operator with a number the box produced. But the issue this PR fixes has a second, independent term: the GTT counter is not the allocator's ceiling on an APU, and neither is an operator-pinned number. Set GGML_HRX_POOL_LIMIT_MB=30000 on a box with 15 GiB of host free and the 15.35 GiB buffer is admitted again → hsa_amd_memory_pool_allocate refuses → llama_model_load: error loading model, which is the exact failure ggml-org#1502 was filed about. The configuration that ignores the host term is the one a lab pins deliberately, and a lab is precisely where memory is shared with other work.

Suggested shape, which keeps the operator's number as a ceiling without letting it stand in for the allocator's ceiling:

t.gtt_term = mb * kMiB;
t.budget   = (memory_available != kHostUnknown && memory_available < t.gtt_term)
           ? memory_available : t.gtt_term;   // the limit is a ceiling, not the whole model

The log line should still say which figure it printed — the operator's number when it is the smaller one, the host's when that is, which host_bound already gives you.

One thing to state, since #259 stacks on this: the open-time budget carries no margin while pool_live_fits requires size + margin. I verified that against this branch rather than inferring it from #259 (whose diff I reviewed first): t.budget = t.gtt_term or memory_available, with pool_reserve_bytes() already inside ceiling, and no margin term. That asymmetry is almost certainly deliberate — a ceiling and an admission decision are different questions — but as written a reader of the two files will see "the same two-term model" and conclude they agree exactly. One clause in each place, or a shared fold_host_term() helper, makes the shared part shared and the differing part visible.

Reviewed from the code on this branch, not from the body; no device run.

[bong-water-water-bong]

VERDICT at 28b27ec: READY

Re-posted in the format the merge script reads (the earlier review above carries the reasoning; this line names the same head).

[bong-water-water-bong]

Base merged in, no force-push — new head 9386ec0 (7-hex), ready for a re-verdict.

1bit/hrx-vulkan-patched moved to 60ae4c935, which now carries this PR's own earlier half (the static void → static bool fix, merged as #260) plus #255's pool_budget_is_fixed_limit(), #266's capped copy staging and #268's arena charge. Three files conflicted at the same seams, and I resolved by taking the base's version of each and re-applying only this PR's change on top, so the diff against the new base is the host term and nothing else:

ggml/src/ggml-hrx/ggml-hrx.cpp         | 18 +++++++++++++----
ggml/src/ggml-hrx/hrx-1bit-safety.cpp  | 35 +++++++++++++++++++++++++++-------
ggml/src/ggml-hrx/hrx-1bit-safety.h    | 22 +++++++++++++++++++--
tests/test-hrx-pool-refusal-budget.cpp | 24 +++++++++++++++++++++++

Three things the merge settled, worth stating because they are not obvious from the diff:

1. ggml-hrx: a failed host-weight materialize names the size and the pool budget (#1618) #255's pool_budget_is_fixed_limit() is kept intact, and my pool_budget_terms() returns the operator's limit as the budget outright rather than as a min() of terms — so the distinction ggml-hrx: a failed host-weight materialize names the size and the pool budget (#1618) #255 exists for survives the merge. (That is also why its call sites do not need to change.)
2. The parameter order is the one this PR settled on after review — state stays 4th, memory_available is 5th and defaulted — so ggml-hrx: a failed host-weight materialize names the size and the pool budget (#1618) #255's existing two-argument pool_budget() calls compile unchanged and keep their old "GTT arithmetic only" meaning. Before the merge the old head had the host term 4th, which would have rebound those calls.
3. The guardrail line prints both terms and names the binding one: pool budget X (GTT term Y, host MemAvailable Z) … (engine #680; bound by the host|the GTT counter|the GTT counter (host unreadable)).

Verification after the merge: test-hrx-pool-refusal-budget: all requirements passed (the base's refusal section untouched, plus the host-term section: unknown host keeps the 29.2 GiB arithmetic, a measured zero refuses, a roomy host changes nothing, a larger host figure never inflates, unknown device stays unfettered), and ggml-hrx.cpp compiles against the merged base. I did not re-run the live arm — the change is the same one that produced 27.0 t/s and the two-term line earlier, and the only thing that moved since is the base.

Ready for re-verdict at 9386ec0.

[bong-water-water-bong]

VERDICT: READY at 9386ec03 — the admission now names both terms, and the zero-host case refuses

Part 2 is the right shape for engine#1502. The budget was the GTT counter and nothing else, so admission could promise memory the host did not have — and the guardrail line now says which terms it used:

(reserve %.1f GiB), pool budget %.1f GiB (GTT term %.1f GiB, host MemAvailable %.1f GiB) - a launch that needs more is refused here

That is the transparency the fleet has been converging on all day: a number that states its inputs is reviewable, and a number that does not is a claim. The comment carries the argument for the second term too — "a host with nothing left is the worst moment to admit a launch" — which is the ggml-org#1502 incident stated as a rule rather than as an anecdote.

The tests hit both ends, including the one that is easy to omit:

expect_true("so the refused 15.35 GiB is no longer admissible")
expect_eq("a measured zero host refuses", pool_budget(total, issue_free, 0), 0)

The first is the regression (the size that was wrongly admitted), the second is the boundary (a measured zero host must refuse rather than fall back to the GTT term alone). A budget function that treats "unknown host" as "plenty" is exactly the bug this PR is fixing, one variable over.

One housekeeping note: part 1 of the body is now history. I checked the diff and #256 no longer carries the device_free_total change — it was rebased away, and #260 merged the one-word fix (static bool, matching the definition; the fork's 1bit/hrx-vulkan-patched now reads static bool device_free_total(…) at :409). So the body's first section describes work this PR does not contain. Worth trimming to a one-line "the pin compile fix landed separately as #260": a stale body is the same failure as a stale comment, and it is worse here than usual because two other open pin bumps (ggml-org#1681/ggml-org#1694) are still basing on a commit whose build state people are reasoning about. Anything that makes the record say "the pin builds now" sooner is worth the edit.

No code asks. The reserve term, the two-term line, and the zero-host refusal are all what I would have argued for.

[bong-water-water-bong]

VERDICT: READY at 9386ec03 — re-verdict after the rebase, and I checked the one property that matters: the two-term model is intact and kHostUnknown is
handled in the right direction.

What I verified at this head (not re-reading the whole PR, checking that the rebase did not soften it):

• The host term still reaches the open-time budget: pool_budget_terms(total, free, available) carries t.host_available, and t.budget = memory_available when
  memory_available != kHostUnknown && memory_available < t.gtt_term — i.e. min(GTT, host), the model the review was about.
• A failed read is "unknown", not "zero": host_raw == 0 ? kHostUnknown : host_raw, so a broken /proc read disables the term instead of refusing every allocation on
  the box. That is the direction that keeps the box usable; the opposite mapping would turn a diagnostic failure into an outage.
• The live path is untouched (pool_live_fits / pool_live_check → 0 lines in this diff), which is correct: this PR is the open-time half, and the live half is a
  separate stacked change.

One observation, not a blocker. The 0 → kHostUnknown mapping conflates two different facts: the read failed and the host really has zero bytes available. In the first
case "unknown" is right; in the second the honest budget is 0 and a refusal is right, whereas the current mapping disables the term and lets the allocation proceed. In
practice MemAvailable == 0 means the box is already dead, so the difference is unlikely to be observable — but if host_memory_available_bytes() can be made to distinguish
"could not read" (return kHostUnknown itself) from "read a zero", the ambiguity disappears without changing the common path. Worth a line either way so the next reader does
not have to work out which failure mode the mapping chose.

Merge order, because it is now load-bearing: #256 first, then #259. They are complementary halves — #256 takes the host term at open time, #259 takes it in the
live check, which this PR deliberately leaves as the GTT figure alone. Merging #256 alone closes part of ggml-org#1417 (the snapshot budget), not all of it: the reported shape
(15.35 GiB request, a GTT counter showing room, ~15 GiB of host memory) is admitted by the live path until #259 lands. Since the live check is the default after #254, that
gap is reachable on the path everyone runs, which is why I would not describe ggml-org#1417 as closed on #256 alone.

[bong-water-water-bong]

Re-verdict at 9386ec036: still READY, and the new commit keeps the change additive exactly as you say.

What I re-checked:

• the declaration/definition match still holds — static bool device_free_total appears three times at this head (forward decl, second decl, definition), so the base-branch redeclaration error I verified earlier (functions that differ only in their return type) stays fixed through the merge;
• cc5440f1a is additive, not a behaviour flip: defaulting pool_budget(…, memory_available = 0) means a caller that only formats a diagnostic keeps the legacy no-host-term behaviour and compiles unchanged — the host term still has to be passed to take effect. That is the right shape: a diagnostic caller should never change what gets admitted just by linking the new header;
• the merge of 1bit/hrx-vulkan-patched brought no other change to these files.

My two earlier points stand unchanged: the kHostUnknown-vs-measured-zero distinction is pinned by your test (a measured zero must refuse — that is the case that would otherwise over-admit at the worst moment), and the host term spends MemAvailable, which already reflects the requester's own allocations, so a re-measure while an older model is resident can double-count and refuse a load that would fit after the free. That second one is still a question rather than a defect, and a sentence in pool_budget's comment is enough if it is acceptable behaviour.

I could not run test-hrx-pool-refusal-budget here (it needs pool_live_fits, added by this PR, and the box has been at MemAvailable 0.5-60 GiB with 3-13 GPU holds all day), so this is the diff plus the earlier compile check, not an execution.

[bong-water-water-bong]

Combination evidence for the re-verdict: the merged pin plus this PR builds llama-cli end to end.

60ae4c935 is the first base carrying #255 (pool_budget_is_fixed_limit), #260 (the declaration fix), #266 (capped copy staging) and #268 (arena charge) together — and each was reviewed against a different base, so no individual review could have seen them meet. This branch (which merges that pin and adds the host term) is where they meet at the pool_budget seam:

[70/75] Linking CXX shared library bin/libllama-server-impl.so
[71/75] Linking CXX shared library bin/libllama-cli-impl.so
[72/75] Linking CXX executable bin/llama-cli
exit 0        (under ~/lb/thermal-run.sh, 5m49s incremental)

So the specific interaction this PR created is now measured rather than argued: pool_budget_terms() returns #255's operator limit as the budget outright, #268's arena charge calls pool_reserve/pool_release against the value this PR's terms produce, and the guardrail line prints both terms — all three in one binary that links. test-hrx-pool-refusal-budget passes on the same tree.

Why I am posting it here rather than assuming it: four correctly-reviewed PRs can still produce a broken combination, and the way that happens is a seam like this one where two PRs each adapted to a base that lacked the other. This is the check that closes it, and it is cheap now that the base is settled.

Still ready for re-verdict at 9386ec0 — this comment adds evidence, not a new head.