diff --git a/ggml/src/ggml-hrx/runtime/failure-status.cpp b/ggml/src/ggml-hrx/runtime/failure-status.cpp index 427d61883b45..b3876f0f6066 100644 --- a/ggml/src/ggml-hrx/runtime/failure-status.cpp +++ b/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; diff --git a/ggml/src/ggml-hrx/runtime/failure-status.h b/ggml/src/ggml-hrx/runtime/failure-status.h index 1b1f8c41abb3..d50d96be34a6 100644 --- a/ggml/src/ggml-hrx/runtime/failure-status.h +++ b/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 & 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 diff --git a/ggml/src/ggml-hrx/runtime/host-memory.cpp b/ggml/src/ggml-hrx/runtime/host-memory.cpp index e0b1d70d60b1..024d29e26021 100644 --- a/ggml/src/ggml-hrx/runtime/host-memory.cpp +++ b/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; diff --git a/ggml/src/ggml-hrx/runtime/transient-arena.cpp b/ggml/src/ggml-hrx/runtime/transient-arena.cpp index b4a3099bb349..1c01a35018f7 100644 --- a/ggml/src/ggml-hrx/runtime/transient-arena.cpp +++ b/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; diff --git a/tests/test-hrx-failure-status.cpp b/tests/test-hrx-failure-status.cpp index 4e019cff68ee..bcc425139c31 100644 --- a/tests/test-hrx-failure-status.cpp +++ b/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 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 kernel_fault = {