hrx: a pool refusal at compute time is ALLOC_FAILED, so the server says out of memory instead of "Compute error." (engine #1618)

ggml/src/ggml-hrx/runtime/failure-status.cpp

@@ -10,12 +10,14 @@ namespace {
 // iree's status name for a pool that cannot serve the request, and HSA's for the same condition one layer down. Both appear
 // verbatim in the failures this exists for:
 //   al...: RESOURCE_EXHAUSTED; [hsa_amd_memory_pool_allocate] HSA_STATUS_ERROR_OUT_OF_RESOURCES
-constexpr const char * kResourceNeedles[] = { "RESOURCE_EXHAUSTED", "OUT_OF_RESOURCES" };
+// kPoolRefusalToken is the same condition one layer UP: the pool ledger refused before the allocator was asked.
+constexpr const char * kResourceNeedles[] = { "RESOURCE_EXHAUSTED", "OUT_OF_RESOURCES", kPoolRefusalToken };
 
 // Case-insensitive on purpose: the *classification* needles are exact because they are identifiers as iree and HSA render
 // them, but the *hint* exists to catch spellings we have not seen -- `hipErrorOutOfMemory` is real hip and mixed case, and a
 // case-sensitive match would have missed exactly the case this probe was added for (the test caught that).
-constexpr const char * kAllocationHints[] = { "memory", "alloc" };
+// "pool" is here for our own refusals: one written without kPoolRefusalToken is then at least named in the log.
+constexpr const char * kAllocationHints[] = { "memory", "alloc", "pool" };
 
 bool contains_any_of_ci(const std::string & message, const char * const * needles, size_t count) {
     std::string lowered;

ggml/src/ggml-hrx/runtime/failure-status.h

@@ -20,7 +20,16 @@ namespace ggml::hrx {
 // survive rewording of the prose around them.
 bool failure_looks_like_allocation(const std::vector<std::string> & messages);
 
-// A message that *looks* like an allocation failure (it mentions memory or allocation) but matched neither needle, so it is
+// The third identifier, and the only one this backend writes itself: the token a compute-time refusal by the pool LEDGER
+// carries in its message. The two names above come from the allocator, i.e. from a launch the guardrail admitted and the
+// device then could not serve. Since engine #1711/#1823 the ledger refuses first (the transient arena, a host staging buffer,
+// a materialized host weight), and those refusals are this backend's own prose: "refused: a 121.7 MiB host weight does not
+// fit the pool" names neither RESOURCE_EXHAUSTED nor OUT_OF_RESOURCES, so the better the accounting got, the more often an
+// out-of-memory condition went back to being "Compute error." for the client. A refusal site appends this token instead of
+// relying on its wording, so rewording a sentence cannot change the status.
+inline constexpr char kPoolRefusalToken[] = "POOL_EXHAUSTED";
+
+// A message that *looks* like an allocation failure (it mentions memory, allocation or the pool) but matched no needle, so it is
 // about to be classified as a compute failure (engine #1618 review). Returns nullptr when nothing looks suspicious. This exists
 // so the next spelling of an out-of-memory status -- `hipErrorOutOfMemory`, `HSA_STATUS_ERROR_INVALID_ALLOCATION`, whatever the
 // runtime renders tomorrow -- costs a log line rather than another day of misdiagnosis: the fallback stays the safe direction

ggml/src/ggml-hrx/runtime/host-memory.cpp

@@ -5,6 +5,7 @@
 #include "hrx-interop-utils.h"
 #include "hrx_runtime.h"
 #include "hrx-1bit-safety.h"            // engine#1711: pool_reserve/pool_release
+#include "runtime/failure-status.h"     // engine#1618: kPoolRefusalToken, so a pool refusal is not a "Compute error."
 #include "transient-arena-charge.h"     // engine#1711: the same charge decision as the transient arena
 #include "runtime/host-memory-available.h"
 #include "runtime/host-memory-ternary.h"
@@ -2021,8 +2022,8 @@ HostWeightAcquireResult HostWeightCache::acquire(hrx_device_t             device
         // pool that cannot hold it is a refusal naming the weight rather than an over-commit that faults later.
         if (pool_budget > 0 && materialized_length > 0) {
             if (!ggml::hrx::onebit::pool_reserve(pool_budget, materialized_length, "a materialized host weight")) {
-                result.status.log("refused: a %.1f MiB host weight does not fit the pool",
-                                  materialized_length / (1024.0 * 1024.0));
+                result.status.log("refused: a %.1f MiB host weight does not fit the pool (%s)",
+                                  materialized_length / (1024.0 * 1024.0), kPoolRefusalToken);
                 return result;
             }
             entry->charged_bytes_ = materialized_length;
@@ -2174,7 +2175,7 @@ Status allocate_host_staging_buffer(hrx_device_t device, size_t size, HostStagin
     if (size > 0 && pool_budget > 0) {
         if (!ggml::hrx::onebit::pool_reserve(pool_budget, size, "a host staging buffer")) {
             Status refused;
-            refused.log("refused: a %zu-byte host staging buffer does not fit the pool", size);
+            refused.log("refused: a %zu-byte host staging buffer does not fit the pool (%s)", size, kPoolRefusalToken);
             return refused;
         }
         charge_taken = true;

ggml/src/ggml-hrx/runtime/transient-arena.cpp

@@ -2,6 +2,7 @@
 
 #include "hrx-1bit-safety.h"   // engine#1711: the pool ledger (pool_reserve/pool_release)
 #include "transient-arena-charge.h"
+#include "runtime/failure-status.h"   // engine#1618: kPoolRefusalToken
 
 #include "hrx-interop-utils.h"
 #include "hrx_runtime.h"
@@ -96,7 +97,8 @@ Status TransientArena::ensure_capacity_locked(hrx_device_t device, hrx_stream_t
     bool                  charge_taken = false;
     if (charge.reserve_bytes > 0 && pool_budget > 0) {
         if (!ggml::hrx::onebit::pool_reserve(pool_budget, charge.reserve_bytes, "the transient arena")) {
-            status.log("refused: the transient arena needs %zu more bytes and the pool has no room", charge.reserve_bytes);
+            status.log("refused: the transient arena needs %zu more bytes and the pool has no room (%s)", charge.reserve_bytes,
+                       kPoolRefusalToken);
             return status;
         }
         charge_taken   = true;

tests/test-hrx-failure-status.cpp

@@ -51,6 +51,28 @@ int main() {
     REQUIRE(failure_looks_like_allocation({ "materialize host weight value 3 failed: HSA_STATUS_ERROR_OUT_OF_RESOURCES" }));
     REQUIRE(failure_status({ "x: RESOURCE_EXHAUSTED" }) == GGML_STATUS_ALLOC_FAILED);
 
+    // The pool LEDGER's own refusals (engine #1711/#1823) are the same condition one layer up, and they are this backend's
+    // prose rather than an iree or HSA name. Before kPoolRefusalToken all three were GGML_STATUS_FAILED, i.e. "Compute
+    // error." for the client -- #1618's defect again, on the path that replaced the allocator failure. The sentences are
+    // the three refusal sites' (host-memory.cpp twice, transient-arena.cpp), with the token each now appends.
+    using ggml::hrx::kPoolRefusalToken;
+    const std::string token = kPoolRefusalToken;
+    for (const std::string & refusal : {
+             "refused: a 121.7 MiB host weight does not fit the pool (" + token + ")",
+             "refused: a 8388608-byte host staging buffer does not fit the pool (" + token + ")",
+             "refused: the transient arena needs 67108864 more bytes and the pool has no room (" + token + ")",
+         }) {
+        REQUIRE(failure_looks_like_allocation({ refusal }));
+        REQUIRE(failure_status({ refusal }) == GGML_STATUS_ALLOC_FAILED);
+        REQUIRE(ggml::hrx::failure_unmatched_allocation({ refusal }) == nullptr);
+    }
+    // The token decides, not the sentence: the old wording without it stays a compute failure (so a refusal site that
+    // forgets the token is the bug, and rewording one cannot change a status) -- but it is NAMED by the warning, because
+    // "pool" is an allocation hint.
+    const std::vector<std::string> untagged = { "refused: a 121.7 MiB host weight does not fit the pool" };
+    REQUIRE(failure_status(untagged) == GGML_STATUS_FAILED);
+    REQUIRE(ggml::hrx::failure_unmatched_allocation(untagged) != nullptr);
+
     // A real compute failure must stay a compute failure: this is the direction that would be a regression, because a
     // kernel error reported as "out of memory" sends the reader to free memory that was never the problem.
     const std::vector<std::string> kernel_fault = {