From 0307fb2c02100a2bd6536e0b8e57c076b71f708e Mon Sep 17 00:00:00 2001 From: bong-water-water-bong <277547417+bong-water-water-bong@users.noreply.github.com> Date: Sat, 7 Nov 2026 21:32:22 -0400 Subject: [PATCH 1/2] hrx: say which pool state produced the graph split when strict placement refuses a model (strict-placement ticket) `GGML_HRX_STRICT=1` aborts on the first node that is not placed on HRX. On this APU the split depends on the *shared* GTT pool - other processes move it - so the same binary and the same model bytes serve on a quiet box and are refused on a busy one, and the abort line names a tensor without saying why. Measured here (strict-placement ticket, one variable at a time): `Qwen3.8-27B-UD-Q4_K_XL` serves with 37.5 GiB in use and is refused with 47.5 GiB in use, same sidecar hashes and mtimes, same model hash; the single node behind it is a 2-row f32 `GET_ROWS` (the token-embedding gather), 1 of 4087. That reads as a verdict on the model, and it has already turned two release windows' payload checks into "the shipped payload cannot serve these models". So the abort now prints the pool it was taken in: GTT in use / total and the HRX pool budget, plus the two ways out (`--no-strict-device`, or a retry on a quieter box). Numbers only, computed at the point of refusal; nothing about placement changes, no new decision, no new flag. Deliberately not done here: the two semantic fixes in the ticket (whitelist the benign embedding gather, or judge strictness by policy rather than by observation of a pool-dependent split). Both change what `--strict-device` promises, which is an owner call, and the second needs the split to be independent of the pool before it can be trusted. Refs #1259 --- ggml/src/ggml-hrx/ggml-hrx.cpp | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/ggml/src/ggml-hrx/ggml-hrx.cpp b/ggml/src/ggml-hrx/ggml-hrx.cpp index b32d7c341a50..a501260b1f7f 100644 --- a/ggml/src/ggml-hrx/ggml-hrx.cpp +++ b/ggml/src/ggml-hrx/ggml-hrx.cpp @@ -490,6 +490,9 @@ static bool placement_fallback_quiet() { return end != value && *end == '\0' && parsed <= 0; } +// defined below device registration; the strict-placement report needs the live pool state +static void device_free_total(ggml_backend_hrx_device_context * device_ctx, size_t & free_bytes, size_t & total_bytes); + static void placement_observer(ggml_backend_sched_t sched, ggml_cgraph * graph, void * user_data) { const bool strict = environment_flag_enabled("GGML_HRX_STRICT"); const int level = placement_report_level(); @@ -569,10 +572,26 @@ static void placement_observer(ggml_backend_sched_t sched, ggml_cgraph * graph, } if (strict && first_off != nullptr) { - GGML_ABORT("%s: GGML_HRX_STRICT=1: \"%s\" is %s %s ne=[%" PRId64 ",%" PRId64 ",%" PRId64 ",%" PRId64 "] and is not placed on HRX; %" PRId64 " of %d nodes off HRX (%s), %d splits\n", + // The split is a function of the *shared* GTT pool, which other processes on this APU move, so a strict refusal is + // not a property of the model or of the build. Without the pool numbers an operator reads "X is not placed on HRX" + // as a model problem; with them the same line says which state produced the split and that a fallback exists + // (strict-placement ticket, measured on this box: 37.5 GiB in use serves, 47.5 GiB in use refuses). + double in_use_gib = 0.0, total_gib = 0.0, budget_gib = 0.0; + { + auto * hrx_context = hrx != nullptr ? static_cast(hrx->context) : nullptr; + if (hrx_context != nullptr && hrx_context->device != nullptr) { + size_t free_bytes = 0, total_bytes = 0; + device_free_total(hrx_context->device, free_bytes, total_bytes); + const double gib = 1024.0 * 1024.0 * 1024.0; + total_gib = (double) total_bytes / gib; + in_use_gib = (double) (total_bytes > free_bytes ? total_bytes - free_bytes : 0) / gib; + budget_gib = (double) hrx_context->device->memory_budget / gib; + } + } + GGML_ABORT("%s: GGML_HRX_STRICT=1: \"%s\" is %s %s ne=[%" PRId64 ",%" PRId64 ",%" PRId64 ",%" PRId64 "] and is not placed on HRX; %" PRId64 " of %d nodes off HRX (%s), %d splits. Shared GTT pool: %.1f of %.1f GiB in use, HRX pool budget %.1f GiB - the split follows that pool, so --no-strict-device (or a retry on a quieter box) runs those nodes on the CPU\n", __func__, first_off->name, ggml_op_name(first_off->op), ggml_type_name(first_off->type), first_off->ne[0], first_off->ne[1], first_off->ne[2], first_off->ne[3], off_nodes, graph->n_nodes, - off_detail.c_str(), ggml_backend_sched_get_n_splits(sched)); + off_detail.c_str(), ggml_backend_sched_get_n_splits(sched), in_use_gib, total_gib, budget_gib); } if (level >= 1) { From 7a6cf42863c001272567dd5e817a60e6a191be09 Mon Sep 17 00:00:00 2001 From: bcloud Date: Sun, 8 Nov 2026 20:14:36 -0400 Subject: [PATCH 2/2] hrx: say "pool state unavailable" instead of 0.0 GiB in use when no source answered `device_free_total`'s last-resort branch sets free = total = memory_total when both `hrx_device_memory_info` and `ggml::hrx::onebit::gtt_counters` fail. The strict-placement line computes `in_use = total - free` from those, so on that path it would print Shared GTT pool: 0.0 of 120.0 GiB in use, HRX pool budget 98.0 GiB i.e. the most reassuring possible number, at the exact moment the operator is asking whether the box or the model caused the refusal, and indistinguishable from a genuinely idle pool. That is the class this lane keeps fixing: a plausible number manufactured out of no measurement. `device_free_total` now returns whether either source answered (false only on the fallback it already distinguishes internally; the other three call sites ignore the result, unchanged). The message builds its pool segment as a string, so the unavailable case can say so: Shared GTT pool: state unavailable (neither the driver nor the GTT counters answered) Compile-checked: the exact Release command line for `ggml-hrx.dir/ggml-hrx.cpp.o` from a configured HRX build (clang 24, -Werror), object produced with no diagnostics. Not run against a device: the fallback branch needs both memory sources to fail, and this is a message path. Refs the review at 0307fb2c0 (the CHANGES in the fallback path of the number this PR prints). --- ggml/src/ggml-hrx/ggml-hrx.cpp | 35 +++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/ggml/src/ggml-hrx/ggml-hrx.cpp b/ggml/src/ggml-hrx/ggml-hrx.cpp index a501260b1f7f..73fe1fee66ff 100644 --- a/ggml/src/ggml-hrx/ggml-hrx.cpp +++ b/ggml/src/ggml-hrx/ggml-hrx.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -490,8 +491,10 @@ static bool placement_fallback_quiet() { return end != value && *end == '\0' && parsed <= 0; } -// defined below device registration; the strict-placement report needs the live pool state -static void device_free_total(ggml_backend_hrx_device_context * device_ctx, size_t & free_bytes, size_t & total_bytes); +// defined below device registration; the strict-placement report needs the live pool state. Returns false when NEITHER +// source answered and the out-params are a fallback, so a caller can say "unavailable" instead of printing a number it +// did not measure (a manufactured 0.0 GiB in use is the same lie the ceiling exists to prevent). +static bool device_free_total(ggml_backend_hrx_device_context * device_ctx, size_t & free_bytes, size_t & total_bytes); static void placement_observer(ggml_backend_sched_t sched, ggml_cgraph * graph, void * user_data) { const bool strict = environment_flag_enabled("GGML_HRX_STRICT"); @@ -576,22 +579,27 @@ static void placement_observer(ggml_backend_sched_t sched, ggml_cgraph * graph, // not a property of the model or of the build. Without the pool numbers an operator reads "X is not placed on HRX" // as a model problem; with them the same line says which state produced the split and that a fallback exists // (strict-placement ticket, measured on this box: 37.5 GiB in use serves, 47.5 GiB in use refuses). - double in_use_gib = 0.0, total_gib = 0.0, budget_gib = 0.0; + // The pool segment is a STRING, not three %.1f arguments: when neither source answered, printing "0.0 of 120.0 GiB + // in use" would manufacture the most reassuring possible number at the exact moment the operator is asking whether + // the box or the model caused the refusal. Unavailable has to be sayable. + char pool_detail[160] = "state unavailable (neither the driver nor the GTT counters answered)"; { auto * hrx_context = hrx != nullptr ? static_cast(hrx->context) : nullptr; if (hrx_context != nullptr && hrx_context->device != nullptr) { size_t free_bytes = 0, total_bytes = 0; - device_free_total(hrx_context->device, free_bytes, total_bytes); - const double gib = 1024.0 * 1024.0 * 1024.0; - total_gib = (double) total_bytes / gib; - in_use_gib = (double) (total_bytes > free_bytes ? total_bytes - free_bytes : 0) / gib; - budget_gib = (double) hrx_context->device->memory_budget / gib; + if (device_free_total(hrx_context->device, free_bytes, total_bytes)) { + const double gib = 1024.0 * 1024.0 * 1024.0; + const double use = (double) (total_bytes > free_bytes ? total_bytes - free_bytes : 0) / gib; + const double tot = (double) total_bytes / gib; + const double budg = (double) hrx_context->device->memory_budget / gib; + std::snprintf(pool_detail, sizeof(pool_detail), "%.1f of %.1f GiB in use, HRX pool budget %.1f GiB", use, tot, budg); + } } } - GGML_ABORT("%s: GGML_HRX_STRICT=1: \"%s\" is %s %s ne=[%" PRId64 ",%" PRId64 ",%" PRId64 ",%" PRId64 "] and is not placed on HRX; %" PRId64 " of %d nodes off HRX (%s), %d splits. Shared GTT pool: %.1f of %.1f GiB in use, HRX pool budget %.1f GiB - the split follows that pool, so --no-strict-device (or a retry on a quieter box) runs those nodes on the CPU\n", + GGML_ABORT("%s: GGML_HRX_STRICT=1: \"%s\" is %s %s ne=[%" PRId64 ",%" PRId64 ",%" PRId64 ",%" PRId64 "] and is not placed on HRX; %" PRId64 " of %d nodes off HRX (%s), %d splits. Shared GTT pool: %s - the split follows that pool, so --no-strict-device (or a retry on a quieter box) runs those nodes on the CPU\n", __func__, first_off->name, ggml_op_name(first_off->op), ggml_type_name(first_off->type), first_off->ne[0], first_off->ne[1], first_off->ne[2], first_off->ne[3], off_nodes, graph->n_nodes, - off_detail.c_str(), ggml_backend_sched_get_n_splits(sched), in_use_gib, total_gib, budget_gib); + off_detail.c_str(), ggml_backend_sched_get_n_splits(sched), pool_detail); } if (level >= 1) { @@ -823,23 +831,24 @@ static const char * device_description(ggml_backend_dev_t device) { // GTT free and total as the driver sees them. hrx_device_memory_info is the proper source but the AMDGPU HAL does // not report availability (libhrx: "did not provide available memory in its observation"), so the driver's own // counters are the fallback - and they are the only cross-process view, which is what the #680 ceiling needs. -static void device_free_total(ggml_backend_hrx_device_context * device_ctx, size_t & free_bytes, size_t & total_bytes) { +static bool device_free_total(ggml_backend_hrx_device_context * device_ctx, size_t & free_bytes, size_t & total_bytes) { size_t hal_free = 0; size_t hal_total = 0; if (hrx_status_is_ok(hrx_device_memory_info(device_ctx->device, &hal_free, &hal_total)) && hal_total != 0) { free_bytes = hal_free; total_bytes = hal_total; - return; + return true; } size_t gtt_total = 0; size_t gtt_used = 0; if (ggml::hrx::onebit::gtt_counters(gtt_total, gtt_used) && gtt_total != 0) { total_bytes = gtt_total; free_bytes = gtt_total > gtt_used ? gtt_total - gtt_used : 0; - return; + return true; } free_bytes = device_ctx->memory_total; total_bytes = device_ctx->memory_total; + return false; } static void device_memory(ggml_backend_dev_t device, size_t * free, size_t * total) {