fix(BACKEND-ROCM): a narrow predicate makes the device-fit refusal reachable on ROCm

include/vllm/platforms/interface.h

@@ -372,6 +372,47 @@ class Platform {
   // memory in place) — the decoupling a bare `kCUDA` cannot express.
   virtual bool needs_weight_staging() const { return false; }
 
+  // BACKEND-ROCM, issue #1934. Does a load on this platform allocate BOUNDED,
+  // budget-checkable device memory for resident weights — the narrower
+  // PHYSICAL question `needs_weight_staging()` deliberately does NOT answer.
+  //
+  // `needs_weight_staging()`'s own doc above says it "governs the model's
+  // device-resident forward as a whole", listing `ResidentWeight` among what it
+  // gates — that listing is STALE. `qwen3_5.cpp::ResidentWeight` was fixed
+  // under issue #125 to key its alias-vs-upload branch on `is_cpu()`, not
+  // `needs_weight_staging()`, precisely because the old predicate answered
+  // false for every non-CUDA device (Vulkan, Metal, XPU, ROCm) and each of
+  // them aliased a HOST pointer into a DEVICE kernel. So today, on ANY
+  // non-CPU platform, `ResidentWeight::Alloc` genuinely allocates bounded
+  // device memory regardless of `needs_weight_staging()` — confirmed on ROCm
+  // by issue #1870's own reproduction, a real `hipMalloc: out of memory`.
+  //
+  // What `needs_weight_staging()` correctly still gates is a DIFFERENT
+  // question: should the OPTIMIZED device-resident forward run — the indexed
+  // GDN state-I/O kernels (though `IndexedGdnStateIoEnabled` already
+  // special-cases a non-staging, non-CPU device by checking op registration
+  // directly, so ROCm already takes the fast arm there without this method),
+  // the merged/packed GDN projections, and the fp8/bf16 GDN resident-prep
+  // passes. Those default to the ROW-COPY REFERENCE path today on ROCm, and
+  // this method changes NONE of them: flipping `needs_weight_staging()`
+  // itself was considered and rejected (see the row's spec) because at least
+  // one of those consumers has no op-registration fallback and would silently
+  // assume kernels a device might not have, exactly the failure mode
+  // `IndexedGdnStateIoEnabled` was written to avoid for the ONE consumer that
+  // already checks.
+  //
+  // Consumed by the ONE production call site of `CheckDeviceWeightFit`
+  // (`gguf_device_fit.h`, issue #1123): the load-time refusal only needs to
+  // know "will this load draw from a bounded device memory pool, and do we
+  // know its size" — not "should the fully-optimized forward run".
+  //
+  // Default DELEGATES to `needs_weight_staging()`, so CUDA's answer (true) is
+  // unchanged and every platform that overrides neither method reads exactly
+  // as it did before this method existed — a pure additive seam.
+  virtual bool allocates_bounded_device_memory() const {
+    return needs_weight_staging();
+  }
+
   // Does this platform have the fused flash-attention-2 (native-bf16) attention
   // fast path? The FA2 dispatch (qwen3_5.cpp GdnBlockPaged / full-attn preamble)
   // emits bf16 q/k and a bf16 attention output — the combo the vendored CUDA

include/vt/rocm/rocm_runtime.h

@@ -10,6 +10,7 @@
 // at load time on a machine that merely happens to have HIP installed.
 #pragma once
 
+#include <cstddef>
 #include <string>
 
 namespace vt::rocm {
@@ -55,4 +56,17 @@ bool HostMemoryIsDeviceAddressable(int index) noexcept;
 // path their silicon took without reading driver internals.
 bool ManagedAllocActive(int index) noexcept;
 
+// BACKEND-ROCM, issue #1934. `hipMemGetInfo`'s `total` for device `index`, in
+// bytes; 0 when the device is absent or the probe fails. HIP-free so the
+// PLATFORM registrar (static-init time, unspecified cross-TU order — the same
+// reasoning `DeviceAvailable()` above states) can read it without depending on
+// `RocmBackend`'s own registrar having already run. Mirrors
+// `platforms/cuda.cpp`'s own `cudaMemGetInfo` probe at registration, which
+// this project's ResidencyPolicy::device_memory_total_bytes doc already
+// specifies as "TOTAL rather than FREE, because free at load time carries the
+// page cache and whatever else the box is doing" — same reasoning applies to
+// HIP's allocator. 0 == UNKNOWN, which `gguf_device_fit.h`'s load-time
+// refusal already reads as "do not decide", never as "nothing fits".
+size_t DeviceMemoryTotalBytes(int index) noexcept;
+
 }  // namespace vt::rocm

src/vllm/entrypoints/model_loader.cpp

@@ -2402,9 +2402,18 @@ std::unique_ptr<LoadedEngine> LoadedEngine::FromModelDir(
       // already excludes every load it could apply to.
       const bool policy_forces_full_expand =
           GgufPolicyForcesFullExpand(gguf_load_policy);
+      // BACKEND-ROCM (#1934): `allocates_bounded_device_memory()`, not
+      // `needs_weight_staging()`. The two questions differ (see the interface
+      // doc): this one asks whether `ResidentWeight` draws from a bounded
+      // device pool at all -- true on every non-CPU platform since issue
+      // #125's `is_cpu()` fix -- while `needs_weight_staging()` asks whether
+      // the FULLY-OPTIMIZED device-resident forward (several GDN kernel
+      // defaults) should run. Using the narrower predicate here is what makes
+      // this refusal reachable on ROCm without moving any of the other one's
+      // consumers; the row's spec records why that flag stays untouched.
       const DeviceWeightFit fit = CheckDeviceWeightFit(
           gguf, vt::DeviceTypeName(target.device_type()),
-          target.needs_weight_staging(),
+          target.allocates_bounded_device_memory(),
           DeviceWeightBudgetBytes(
               target.residency_policy().device_memory_total_bytes),
           /*model_dtype_bytes=*/2, lane, policy_forces_full_expand);

src/vllm/platforms/rocm.cpp

@@ -18,6 +18,7 @@
 // value here is a guess dressed as a decision.
 #include "vllm/platforms/interface.h"
 
+#include <cstddef>
 #include <vector>
 
 #include "vt/backend.h"
@@ -28,6 +29,15 @@ namespace {
 
 class RocmPlatform final : public Platform {
  public:
+  // Issue #1934. `device_memory_total_bytes` is `vt::rocm::DeviceMemoryTotalBytes(0)`,
+  // probed once by the registrar below at static init, exactly mirroring how
+  // `CudaPlatform` threads its own `cudaMemGetInfo` probe through its
+  // constructor (`platforms/cuda.cpp`). 0 means the probe failed or no device
+  // is present; `residency_policy()` passes it through unexamined, and
+  // `gguf_device_fit.h` already reads 0 as UNKNOWN rather than "nothing fits".
+  explicit RocmPlatform(size_t device_memory_total_bytes)
+      : device_memory_total_bytes_(device_memory_total_bytes) {}
+
   DeviceType device_type() const override { return DeviceType::kROCM; }
   Backend& backend() const override { return vt::GetBackend(DeviceType::kROCM); }
 
@@ -86,24 +96,46 @@ class RocmPlatform final : public Platform {
   //   snapshot. This flag still stays false because flipping it to engage a
   //   real model's decode-graph path is W2, not W1.
   // needs_weight_staging() stays false: this is the memory-model POLICY that
-  //   selects the device-resident forward over the host-resident reference path.
-  //   HIP's programming model does stage (hipMalloc hands back a distinct
-  //   address), so a discrete AMD card will eventually answer true — but in W0
-  //   there is one registered op, so the only path that can run at all is the
-  //   host-resident one the reference tier serves on a unified part. Answering
-  //   true today would route a model into a path with no kernels. Revisit at M2.
+  //   selects the FULLY-OPTIMIZED device-resident forward (indexed GDN state
+  //   I/O with no op-registration fallback for a couple of its consumers,
+  //   merged/packed GDN projections, fp8/bf16 GDN resident prep) over the
+  //   host-resident reference path for THOSE specific kernels. Issue #1934
+  //   measured that several of those consumers have no per-op fallback and
+  //   would silently assume kernels this device might not register, so
+  //   flipping this blindly was rejected — see that issue and
+  //   `allocates_bounded_device_memory()` below, which answers the NARROWER
+  //   question the load-time device-fit check actually needs without moving
+  //   any of this flag's other consumers. `IndexedGdnStateIoEnabled` already
+  //   takes ROCm's fast arm today regardless of this flag, by checking op
+  //   registration directly rather than trusting this policy bit — the same
+  //   move #1934 makes for the device-fit check.
   // supports_fa2_attention() / opaque_attention_op() stay false: no ROCm
   //   attention kernel exists yet. See get_attn_backend_priority below.
 
-  // The residency/memory-model policy. DEFAULTS in W0, and the reason is not
-  // laziness: on a unified part (780M, Strix Halo) freeing the host copy after
-  // "upload" would free the ONLY copy, which is the same answer the CPU, Metal
-  // and Vulkan platforms give for the same reason. A DISCRETE card genuinely
-  // wants release_host_weights_after_upload=true and a pooled allocator, and
-  // that is a per-DEVICE answer this per-DEVICE-TYPE seam cannot express yet
-  // (the CUDA leg has the same shape and has not needed to). Flip it when a
-  // discrete board actually loads weights, with the measurement in the record.
-  ResidencyPolicy residency_policy() const override { return {}; }
+  // BACKEND-ROCM, issue #1934. The ONE narrow question the load-time
+  // device-fit refusal (`gguf_device_fit.h`, issue #1123) needs answered:
+  // does a load here allocate device memory `ResidentWeight` cannot exceed
+  // unnoticed? Yes, unconditionally, on any non-CPU platform since issue
+  // #125's fix (see the interface doc on this method) — independent of
+  // `needs_weight_staging()`, which this method deliberately does not touch.
+  bool allocates_bounded_device_memory() const override { return true; }
+
+  // The residency/memory-model policy. `device_memory_total_bytes` is now a
+  // REAL probe (issue #1934, mirrors `CudaPlatform`'s own `cudaMemGetInfo`
+  // probe) — the ONE field the device-fit check reads. The other two fields
+  // stay DEFAULT/false, unlike CUDA's: `release_host_weights_after_upload`
+  // and `uses_device_memory_pool` are separate policy questions (a discrete
+  // card's host-copy release and DevicePool reuse) this row does not touch,
+  // because on a unified part (780M, Strix Halo) freeing the host copy after
+  // "upload" would free the ONLY copy — the same answer CPU, Metal and Vulkan
+  // give for the same reason, and per-DEVICE (not per-DEVICE-TYPE) besides.
+  // Flip those when a discrete board's release/pool behavior is actually
+  // measured, not as a side effect of making the budget check reachable.
+  ResidencyPolicy residency_policy() const override {
+    ResidencyPolicy p;
+    p.device_memory_total_bytes = device_memory_total_bytes_;
+    return p;
+  }
 
   // Attention-backend priority — M3 (issue #41). Mirrors rocm.py:407-441
   // `_get_backend_priorities` at pin 555967922. The dense branch is
@@ -130,6 +162,9 @@ class RocmPlatform final : public Platform {
     return {"ROCM_ATTN", "ROCM_AITER_FA", "ROCM_AITER_UNIFIED_ATTN",
             "TRITON_ATTN", "TURBOQUANT"};
   }
+
+ private:
+  size_t device_memory_total_bytes_ = 0;
 };
 
 // Registers kROCM during static init (registration completes before main() per
@@ -143,7 +178,13 @@ class RocmPlatform final : public Platform {
 struct Registrar {
   Registrar() noexcept {
     if (!vt::rocm::DeviceAvailable()) return;
-    static RocmPlatform platform;
+    // Issue #1934. Device 0, matching this leg's other single-device probes
+    // (`host_memory_is_device_addressable()` above states the same choice).
+    // HIP-free free function, not `Backend::DeviceMemoryInfo`: the backend's
+    // OWN registrar (`rocm_backend.hip`) may not have run yet at this point —
+    // static-init order across TUs is unspecified, the same reason this
+    // registrar probes the device itself rather than trusting one.
+    static RocmPlatform platform(vt::rocm::DeviceMemoryTotalBytes(0));
     RegisterPlatform(DeviceType::kROCM, &platform);
   }
 } registrar;

src/vt/rocm/rocm_backend.hip

@@ -460,4 +460,15 @@ bool ManagedAllocActive(int index) noexcept {
   return caps.valid && UseManagedAlloc(caps);
 }
 
+size_t DeviceMemoryTotalBytes(int index) noexcept {
+  // Mirrors `Backend::DeviceMemoryInfo` (above) byte-for-byte, deliberately
+  // NOT calling it: this is a HIP-free, backend-registration-independent
+  // free function meant for the platform registrar (see the declaration),
+  // where `GetBackend(kROCM)` may not have run yet.
+  if (hipSetDevice(index) != hipSuccess) return 0;
+  size_t free_b = 0, tot_b = 0;
+  if (hipMemGetInfo(&free_b, &tot_b) != hipSuccess) return 0;
+  return tot_b;
+}
+
 }  // namespace vt::rocm

tests/vllm/entrypoints/test_gguf_device_fit_reach.cpp

@@ -100,7 +100,16 @@ class StagingPlatform final : public vllm::platforms::Platform {
   std::vector<vt::DType> supported_dtypes() const override {
     return {vt::DType::kBF16};
   }
-  bool needs_weight_staging() const override { return true; }
+  bool needs_weight_staging() const override { return needs_weight_staging_flag; }
+  // BACKEND-ROCM (#1934). Overridden explicitly, NOT left to the default
+  // delegation to `needs_weight_staging()`, and independently settable: the
+  // two are proven-independent inputs to `CheckDeviceWeightFit`'s call site
+  // by a case that sets them to DIFFERENT values, mirroring ROCm's own real
+  // production state (`needs_weight_staging()=false`,
+  // `allocates_bounded_device_memory()=true`).
+  bool allocates_bounded_device_memory() const override {
+    return allocates_bounded_device_memory_flag;
+  }
   // ENG-EXPERT-STREAM-DEVICE W0d (#1124). The second half of the loader's lane
   // condition. A settable field for the same reason `create_queue_throws` is
   // one: the platform registry is process-global, so a second registration would
@@ -115,6 +124,8 @@ class StagingPlatform final : public vllm::platforms::Platform {
   }
 
   bool host_addressable = false;
+  bool needs_weight_staging_flag = true;
+  bool allocates_bounded_device_memory_flag = true;
 
  private:
   HostBackend& backend_;
@@ -882,3 +893,63 @@ TEST_CASE("device fit: the VARIABLE beats the config key, through the loader") {
   CHECK(message.find("cannot serve this GGUF") != std::string::npos);
   CHECK(message.find(std::to_string(kStagedLowerBound - 1)) != std::string::npos);
 }
+
+// --- BACKEND-ROCM (#1934): the refusal is gated on --------------------------
+// --- `allocates_bounded_device_memory()`, never on `needs_weight_staging()` -
+//
+// Issue #1934: `RocmPlatform::needs_weight_staging()` is stale-false, so
+// before this row the ONE production call site of `CheckDeviceWeightFit`
+// never ran on ROCm regardless of budget. The fix is `model_loader.cpp`
+// reading `target.allocates_bounded_device_memory()` instead. These two cases
+// pin that the call site reads the NEW predicate and NOT the old one, in both
+// directions, so a regression that reverted the call site to
+// `needs_weight_staging()` — or one that read a `||` instead of the plain
+// predicate — goes red here.
+
+TEST_CASE(
+    "device fit: ROCm's own state (staging=false, bounded-memory=true) "
+    "still refuses") {
+  RegisterFakeStagingPlatform();
+  Platform().needs_weight_staging_flag = false;
+  Platform().allocates_bounded_device_memory_flag = true;
+  TempFile f(BuildSyntheticMoeGguf());
+
+  vllm_test::SetEnv("VT_DEVICE_WEIGHT_BUDGET_BYTES",
+                    std::to_string(kStagedLowerBound - 1));
+  const std::string message = ThrownMessage(f.path(), vllm::Device::kNamedPlatform);
+  vllm_test::UnsetEnv("VT_DEVICE_WEIGHT_BUDGET_BYTES");
+  Platform().needs_weight_staging_flag = true;
+  Platform().allocates_bounded_device_memory_flag = true;
+
+  REQUIRE_FALSE(message.empty());
+  CAPTURE(message);
+  CHECK(message.find("cannot serve this GGUF") != std::string::npos);
+  CHECK(message.find(std::to_string(kStagedLowerBound)) != std::string::npos);
+  CHECK(message.find(std::to_string(kStagedLowerBound - 1)) != std::string::npos);
+  CHECK(message.find("tokenizer") == std::string::npos);
+}
+
+TEST_CASE(
+    "device fit: a platform that stages but reports no bounded memory is "
+    "NEVER refused") {
+  RegisterFakeStagingPlatform();
+  Platform().needs_weight_staging_flag = true;
+  Platform().allocates_bounded_device_memory_flag = false;
+  TempFile f(BuildSyntheticMoeGguf());
+
+  // One byte under the footprint, exactly the budget the positive-control case
+  // above refuses at. The ONLY thing that moved is
+  // `allocates_bounded_device_memory_flag`.
+  vllm_test::SetEnv("VT_DEVICE_WEIGHT_BUDGET_BYTES",
+                    std::to_string(kStagedLowerBound - 1));
+  const std::string message = ThrownMessage(f.path(), vllm::Device::kNamedPlatform);
+  vllm_test::UnsetEnv("VT_DEVICE_WEIGHT_BUDGET_BYTES");
+  Platform().allocates_bounded_device_memory_flag = true;
+
+  REQUIRE_FALSE(message.empty());
+  CAPTURE(message);
+  CHECK(message.find("cannot serve this GGUF") == std::string::npos);
+  // The LATER error, asserted positively: without it, "no refusal" would also
+  // be true of a load that died earlier for an unrelated reason.
+  CHECK(message.find("tokenizer: GGUF missing kv") != std::string::npos);
+}