hrx: read the host term before classifying it, so a clang build opens the device again (engine #1942)

ggml/src/ggml-hrx/ggml-hrx.cpp

@@ -1647,7 +1647,7 @@ static std::unique_ptr<ggml_backend_hrx_reg_context> create_registry_context() {
         // refuse - a host with nothing left is the worst moment to admit a launch (engine #1502).
         size_t       host_raw       = 0;
         const size_t host_available =
-            ggml::hrx::onebit::pool_host_term(host_raw, ggml::hrx::host_memory_available_bytes(host_raw));
+            ggml::hrx::onebit::pool_host_term_read(ggml::hrx::host_memory_available_bytes, host_raw);
         const ggml::hrx::onebit::pool_budget_terms_t terms =
             ggml::hrx::onebit::pool_budget_terms(device_ctx->memory_total, device_free, host_available);
         device_ctx->memory_budget = terms.budget;

ggml/src/ggml-hrx/hrx-1bit-safety.cpp

@@ -88,6 +88,16 @@ size_t pool_host_term(size_t raw, bool readable) {
     return readable ? raw : kHostUnknown;
 }
 
+size_t pool_host_term_read(bool (*reader)(size_t &), size_t & raw) {
+    raw = 0;
+    // A full expression of its own: the read is complete before `raw` is looked at (see the header).
+    const bool readable = reader != nullptr && reader(raw);
+    if (!readable) {
+        raw = 0;  // a reader that failed may have written to its out parameter; that is not a reading
+    }
+    return pool_host_term(raw, readable);
+}
+
 bool pool_budget_is_fixed_limit() {
     const char * env = std::getenv("GGML_HRX_POOL_LIMIT_MB");
     return env != nullptr && std::atoll(env) > 0;
@@ -272,7 +282,7 @@ bool pool_reserve(size_t budget, size_t size, const char * what) {
         if (pool_live_counters(total, in_use)) {
             const size_t margin = pool_live_margin_bytes();
             size_t       host_raw     = 0;
-            const size_t host_available = pool_host_term(host_raw, host_memory_available_bytes(host_raw));
+            const size_t host_available = pool_host_term_read(host_memory_available_bytes, host_raw);
             if (!pool_live_fits(size, total, in_use, margin, host_available)) {
                 const size_t live_free = in_use >= total ? 0 : total - in_use;
                 GGML_LOG_ERROR("%s: HRX pool limit: %s of %.1f MiB does not fit the live device budget "

ggml/src/ggml-hrx/hrx-1bit-safety.h

@@ -56,6 +56,18 @@ bool   pool_budget_is_fixed_limit();
 // `raw == 0 ? kHostUnknown : raw` collapsed those two cases - see tests/test-hrx-pool-refusal-budget.cpp.
 size_t pool_host_term(size_t raw, bool readable);
 
+// The same term, READ AND CLASSIFIED IN ONE SEQUENCED STEP (engine #1942). 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) and the live check refused every allocation. g++ evaluates right to
+// left, so the same line was correct there and the fault only showed on the pinned toolchain's own compiler.
+// |reader| is host_memory_available_bytes in production; a parameter so the test can drive the three cases without
+// a live /proc. |raw| receives the figure the reader took (0 when it took none), for the caller's log line.
+size_t pool_host_term_read(bool (*reader)(size_t &), size_t & raw);
+
 // The budget after an allocation the allocator refused, given the size it refused (engine #1502).
 //
 // The guardrail can only estimate what the pool will serve - ROCr exposes no "available" query, so pool_budget() comes from the

tests/test-hrx-pool-refusal-budget.cpp

@@ -32,6 +32,20 @@ static void expect_true(const char * what, bool cond) {
     failures++;
 }
 
+// Readers for pool_host_term_read(): what /proc/meminfo can say, without a live /proc.
+static bool fake_reader_64gib(size_t & out) {
+    out = static_cast<size_t>(64) * 1024 * 1024 * 1024;
+    return true;
+}
+static bool fake_reader_zero(size_t & out) {
+    out = 0;
+    return true;
+}
+static bool fake_reader_unreadable(size_t & out) {
+    out = 777;   // a reader that failed may have scribbled on its out parameter; the figure must not be trusted
+    return false;
+}
+
 static void expect_eq(const char * what, size_t got, size_t want) {
     if (got == want) {
         printf("  PASS  %-58s %zu\n", what, got);
@@ -194,6 +208,30 @@ int main() {
     expect_true("an unreadable /proc still admits on the GTT term alone",
                 pool_live_fits(1 * MiB, gtt, gtt - 45 * GiB, margin_256, pool_host_term(0, false)));
 
+    // ---- the host term is READ BEFORE it is classified (engine #1942) ---------------------------------------------
+    // Both call sites wrote pool_host_term(raw, reader(raw)): `raw` goes in by value while a sibling argument fills
+    // it, and the order of argument evaluation is unspecified. clang (the pinned toolchain's compiler) copies `raw`
+    // first, so a host with 64 GiB available arrived as (0, readable) - a measured zero - and the guardrail refused
+    // the device with "leaves nothing under the ceiling" while the live check refused every allocation.
+    // pool_host_term_read() sequences the read; these arms fail on the old expression under clang and pass under g++,
+    // which is exactly why a g++ run of this suite never saw it.
+    using ggml::hrx::onebit::pool_host_term_read;
+    size_t raw = 12345;
+    expect_eq("a readable host figure reaches the term (not a measured zero)",
+              pool_host_term_read(fake_reader_64gib, raw), 64 * GiB);
+    expect_eq("and the caller's log figure is the one that was read", raw, 64 * GiB);
+    expect_true("so a roomy host does not zero the open-time budget",
+                pool_budget(total, issue_free, pool_host_term_read(fake_reader_64gib, raw)) == issue_budget);
+    expect_true("nor refuse a 1 MiB allocation through the live check",
+                pool_live_fits(1 * MiB, gtt, gtt - 45 * GiB, margin_256, pool_host_term_read(fake_reader_64gib, raw)));
+    raw = 12345;
+    expect_eq("a MEASURED zero still refuses", pool_host_term_read(fake_reader_zero, raw), 0);
+    expect_eq("with a zero log figure", raw, 0);
+    raw = 12345;
+    expect_eq("an unreadable /proc is unknown", pool_host_term_read(fake_reader_unreadable, raw), kHostUnknown);
+    expect_eq("and a stale caller value is not reported as a reading", raw, 0);
+    expect_eq("no reader at all is unknown too", pool_host_term_read(nullptr, raw), kHostUnknown);
+
     printf(failures == 0 ? "test-hrx-pool-refusal-budget: all requirements passed\n"
                          : "test-hrx-pool-refusal-budget: %d FAILED\n", failures);
     return failures == 0 ? 0 : 1;