From 164805fd5fb2456ca12143fa1c37d9507e9ba0d3 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 09:19:00 +0000 Subject: [PATCH 1/2] spec(VT-REFTIER-HOST-ADDRESSABLE): pin CUDA's host-addressability answer where it can be checked, not where it can only be skipped #1635 offered two ways to discharge its remaining owed item: give the real `CudaBackend` a pinning test, or state in the record that the default holds unpinned. This spec takes the first, in the only shape that runs. A runtime test was rejected on a CI fact rather than a preference. No job in `.github/workflows/ci.yml` has a GPU, `cuda-fat-build` configures `-DVLLM_CPP_BUILD_TESTS=OFF` and builds the `vllm` target alone, and the CUDA registrar leaves `kCUDA` unregistered when `cudaGetDeviceCount` finds no device. A `TEST_CASE` reading the real backend would therefore skip on every lane forever, and a skip reads as a pass, which is the shape of evidence #1635 was filed about. The record's claim is instead split into the two halves it actually has, each checked where it executes: a `static_assert` in `src/vt/cuda/cuda_backend.cu` that `CudaBackend` declares no override, compiled by `cuda-fat-build`; and a case in `tests/vt/test_backend.cpp` that the inherited default is `false`, run by every host lane. What stays unobserved -- a live `CudaBackend` answering on hardware -- is named as such rather than left to be inferred. The implementation follows in this branch, so the commit order proves the spec came first. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code] --- .../vt-reference-tier-host-addressable.md | 69 +++++++++++++++---- 1 file changed, 54 insertions(+), 15 deletions(-) diff --git a/.agents/specs/vt-reference-tier-host-addressable.md b/.agents/specs/vt-reference-tier-host-addressable.md index c79749499..d8936f94b 100644 --- a/.agents/specs/vt-reference-tier-host-addressable.md +++ b/.agents/specs/vt-reference-tier-host-addressable.md @@ -296,21 +296,60 @@ is the landed commit message. The two GB10 logs that motivated the row are the backends that merely satisfy the predicate. What stays owed is the MEASUREMENT itself on the two new arms, which needs an Apple-silicon box or an integrated AMD part and cannot be taken here. Owned by this row. -- **Nothing in the tree pins the real `CudaBackend`'s - `DeviceMemoryIsHostAddressable()`, and - [#1635](https://github.com/mudler/vllm.cpp/issues/1635) is OPEN and owned - here.** `tests/vllm/platforms/test_platform.cpp` was cited as that pin and is - not one: `FakeUnifiedAddressablePlatform` reports `device_type() == kCUDA` - while its `backend()` returns `vt::GetBackend(DeviceType::kCPU)`, so the - `CHECK_FALSE` reads the CPU backend and the fixture's own comment says so. The - conclusion survives by ABSENCE of an override — `CudaBackend` declares none, so - it inherits the base `false` in `include/vt/backend.h` — which is a weaker - claim than a pin and must not read as one. `docs/ENVIRONMENT.md` is corrected - here; the `#1502` row in `.agents/issue-index.md` keeps the wrong citation, - because that index is append-only and can never be edited. What stays owed is - the pin itself: either exercise the real `CudaBackend`, which needs a CUDA - device, or state in the record that the default holds unpinned. Owned by this - row. +- **`CudaBackend`'s `DeviceMemoryIsHostAddressable()` is now CHECKED, structurally + rather than by observation, and + [#1635](https://github.com/mudler/vllm.cpp/issues/1635) is discharged by that.** + `tests/vllm/platforms/test_platform.cpp` was cited as the pin and is not one: + `FakeUnifiedAddressablePlatform` reports `device_type() == kCUDA` while its + `backend()` returns `vt::GetBackend(DeviceType::kCPU)`, so the `CHECK_FALSE` + reads the CPU backend. That fixture's comment now says so + ([#1639](https://github.com/mudler/vllm.cpp/pull/1639)), and the `#1502` row in + `.agents/issue-index.md` keeps the wrong citation, because that index is + append-only and can never be edited. + + **A runtime pin was rejected, and the reason is a CI fact rather than a + preference.** No job in `.github/workflows/ci.yml` has a GPU. `cuda-fat-build` + is the only job with a CUDA toolchain; it runs the `nvidia/cuda:13.3.0-devel` + container on `ubuntu-latest`, it configures `-DVLLM_CPP_BUILD_TESTS=OFF`, and + it builds the `vllm` target alone. `CudaBackend`'s registrar returns early when + `cudaGetDeviceCount` reports no device, so `vt::GetBackend(kCUDA)` throws on + every machine this project's CI owns. A `TEST_CASE` that reads the real backend + would therefore report a skip on every lane forever, and a skip reads as a + pass — the same shape of evidence #1635 was filed about. + + **What landed instead is one claim split into two halves, each checked on a + surface that executes it.** + + 1. *`CudaBackend`'s answer IS the base default.* A `static_assert` beside the + class in `src/vt/cuda/cuda_backend.cu` requires + `decltype(&CudaBackend::DeviceMemoryIsHostAddressable)` to be + `bool (Backend::*)() const`. Taking the address of an INHERITED member + through a derived class yields a pointer-to-member of the class that + DECLARES it, so that type holds exactly while `CudaBackend` declares no + override, and it becomes `bool (CudaBackend::*)() const` the moment somebody + adds one. `cuda-fat-build` compiles this translation unit on every push, so + the check runs where the type exists. It fires on ANY override, including + one that returns `false`, because an override invalidates the record's + reasoning whatever it returns. + 2. *The base default is `false`.* `tests/vt/test_backend.cpp` gains a `Backend` + subclass that implements the pure virtuals and deliberately declares no + `DeviceMemoryIsHostAddressable`, and requires `false`. Every other fake in + the tree overrides that method and therefore measures its own override, so + this subclass is the tree's only reader of the default itself. It runs on + every host lane. + + Neither half restates the thing it measures, and both were mutation-proven: + adding an override to `CudaBackend` fails the compile, and flipping the base + default in `include/vt/backend.h` fails the test. The evidence is in the pull + request body, which is the landed commit message. + + **What is still NOT pinned, stated precisely.** No CI surface observes a live + `CudaBackend` object answering the question, because no CI surface has a + device. `tests/vt/test_cuda_backend.cpp` gains that observation, following the + skip convention every case in that file already uses, and it asserts nothing + until somebody runs it on a leased GPU. It is the empirical belt to the two + structural braces above. It is not what holds the answer on a push, and citing + it as though it were would repeat #1635 exactly. ## Now From 58cebc62edf8c3c9da9f86103ba857cc2b2e07e1 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 09:32:39 +0000 Subject: [PATCH 2/2] test(VT-REFTIER-HOST-ADDRESSABLE): check CUDA's host-addressability answer in the two halves it actually has `CudaBackend::DeviceMemoryIsHostAddressable()` gates the portable reference tier, the `VT_ADOPT_DEVICE_BYTES` adoption and the logits-processor bounce, and nothing in the tree held it. The test cited as the pin reads a different backend: `FakeUnifiedAddressablePlatform` reports `device_type() == kCUDA` while its `backend()` returns `vt::GetBackend(DeviceType::kCPU)`. A runtime test cannot replace it, and the reason is a CI fact. No job has a GPU; `cuda-fat-build` is the only job with a CUDA toolchain and it builds with `-DVLLM_CPP_BUILD_TESTS=OFF`; and the CUDA registrar leaves `kCUDA` unregistered when `cudaGetDeviceCount` finds no device. A case reading the real backend would skip on every lane forever, and a skip reads as a pass -- the same shape of evidence #1635 was filed about. The claim has two independent halves, and each is now checked where it executes. `src/vt/cuda/cuda_backend.cu` gains a `static_assert` that `decltype(&CudaBackend::DeviceMemoryIsHostAddressable)` is `bool (Backend::*)() const`: taking the address of an inherited member through a derived class yields a pointer-to-member of the DECLARING class, so that type holds exactly while `CudaBackend` declares no override. `tests/vt/test_backend.cpp` gains a `Backend` subclass that declares no override and requires the inherited default to be `false` -- every other fake in the tree overrides the method and so measures its own override, never the default. `tests/vt/test_cuda_backend.cpp` gains the observation on a real device. It asserts nothing without a GPU and says so in its skip message, so it is the belt and not the braces. Mutation-proven on both halves. Adding an override returning `true` to `CudaBackend` fails `nvcc` 13.3.33 with the intended message (rc=2), an override returning `false` fails it identically, and the restored file recompiles clean at the same sha256. Flipping the base default in `include/vt/backend.h` to `true` turns the new case red (2 of 3 assertions, rc=1), and the restored header is byte-identical. `docs/ENVIRONMENT.md` said nothing pinned the real `CudaBackend`, which this change makes false, so it now names both halves and warns off the wrong citation. Refs #1635 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code] --- docs/ENVIRONMENT.md | 2 +- src/vt/cuda/cuda_backend.cu | 41 +++++++++++++++++++++++++++++ tests/vt/test_backend.cpp | 48 ++++++++++++++++++++++++++++++++++ tests/vt/test_cuda_backend.cpp | 34 ++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 1 deletion(-) diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 18227de30..7a9991f81 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -208,7 +208,7 @@ portable/reference path. In normal operation leave them unset. | `VT_DFLASH_ATTN_WARP` | off (CUDA) | `=1` falls back to the older per-key warp-reduction block-attention kernel instead of the default chunked reduce-scatter form. Kept for the same-binary A/B that recorded the verdict | | `VT_DFLASH_ATTN_KEYLANE` | off (CUDA) | `=1` selects the one-key-per-lane block-attention form. **MEASURED NEGATIVE and not a tuning knob:** 28.90 s/step against the per-key warp kernel's 18.73 on the same binary (sm_110, MiniMax-H3 512x512/33f, seq 3224), 54% slower, because giving each lane a whole K row makes every K load 32-way scattered. Kept only because it is the experiment that located the real constraint | -| `VT_ADOPT_DEVICE_BYTES` | on (acts only where the backend advertises host-addressable device memory: Vulkan always, Metal and ROCm where the part reports unified memory. MEASURED on Vulkan only) | After a weight is uploaded, re-point its host buffer AT the device allocation instead of keeping a second copy. On a unified box the two copies come out of the same RAM. MEASURED on GB10, Qwen3.6-27B bf16 (50.89 GiB): with the mirror the process reaches **VmRSS 100.759 GiB** and drives the machine to MemAvailable 13.85 / MemFree 1.13 GiB of 119.6 GiB before it has even finished allocating; without it the same load completes at **VmHWM 53.413 GiB**. Qwen3-4B: **16.392 -> 9.607 GiB**. Vulkan allocation is byte-identical either way. `0` is the same-binary A/B back to the two-copy behaviour, and it moves ONLY this lever: the `VT_LOAD_DIRECT_UPLOAD` post-upload source-page release still runs at `0` and on a backend without host-addressable device memory, so the two knobs stay independent. It is an adoption, not a release — the bytes survive at the device address and every reader sees them — so tokens are unchanged either way (`test_opt_paged_engine` on Vulkan is 6/6 token-exact, 96/96, both arms). **REACH AND MEASUREMENT ARE NOT THE SAME SET**, and this row used to read as if they were. The lever is gated on `vt::Backend::DeviceMemoryIsHostAddressable()`, read at both `AdoptDeviceBytesAsHost` branches in `src/vllm/model_executor/models/qwen3_5_weights.cpp`. That predicate defaults to `false` (`include/vt/backend.h`), so a backend has to opt in, and THREE now do. `VulkanBackend` (`src/vt/vulkan/vulkan_backend.cpp`) returns `true` unconditionally, because every allocation it makes is `HOST_VISIBLE|HOST_COHERENT` and persistently mapped. Since [`cffe59b02`](https://github.com/mudler/vllm.cpp/commit/cffe59b02) ([#1477](https://github.com/mudler/vllm.cpp/issues/1477), which moved the portable reference tier off the WIDER `UnifiedMemory()`) `MetalBackend` (`src/vt/metal/metal_backend.mm`) answers `MetalContext::unified_memory()`, i.e. `dev.hasUnifiedMemory`, and `RocmBackend` (`src/vt/rocm/rocm_backend.hip`) answers its own `unified_memory_`, i.e. a managed allocator or an integrated part with `PageableMemoryAccess`. **So the lever ACTS on Apple silicon and on an integrated ROCm part, and nobody has measured it on either** — every number above is GB10 through Vulkan. Read those two as reachable-and-unmeasured rather than as covered by this row's numbers ([#1502](https://github.com/mudler/vllm.cpp/issues/1502)). It is still inert on CUDA and CPU, neither of which overrides the default — and that holds by ABSENCE of an override in `CudaBackend` (`include/vt/backend.h` defaults it `false`), NOT by a test. `tests/vllm/platforms/test_platform.cpp` looks like it pins the CUDA answer and does not: its `FakeUnifiedAddressablePlatform` reports `device_type() == kCUDA` but its `backend()` returns `vt::GetBackend(DeviceType::kCPU)`, so the `CHECK_FALSE(gb10.backend().DeviceMemoryIsHostAddressable())` reads the CPU backend. Nothing in the tree pins the real `CudaBackend` ([#1635](https://github.com/mudler/vllm.cpp/issues/1635)). The CPU backend reports `UnifiedMemory() == true` while this narrower predicate stays `false` — the two properties disagreeing is the whole reason the second one exists | +| `VT_ADOPT_DEVICE_BYTES` | on (acts only where the backend advertises host-addressable device memory: Vulkan always, Metal and ROCm where the part reports unified memory. MEASURED on Vulkan only) | After a weight is uploaded, re-point its host buffer AT the device allocation instead of keeping a second copy. On a unified box the two copies come out of the same RAM. MEASURED on GB10, Qwen3.6-27B bf16 (50.89 GiB): with the mirror the process reaches **VmRSS 100.759 GiB** and drives the machine to MemAvailable 13.85 / MemFree 1.13 GiB of 119.6 GiB before it has even finished allocating; without it the same load completes at **VmHWM 53.413 GiB**. Qwen3-4B: **16.392 -> 9.607 GiB**. Vulkan allocation is byte-identical either way. `0` is the same-binary A/B back to the two-copy behaviour, and it moves ONLY this lever: the `VT_LOAD_DIRECT_UPLOAD` post-upload source-page release still runs at `0` and on a backend without host-addressable device memory, so the two knobs stay independent. It is an adoption, not a release — the bytes survive at the device address and every reader sees them — so tokens are unchanged either way (`test_opt_paged_engine` on Vulkan is 6/6 token-exact, 96/96, both arms). **REACH AND MEASUREMENT ARE NOT THE SAME SET**, and this row used to read as if they were. The lever is gated on `vt::Backend::DeviceMemoryIsHostAddressable()`, read at both `AdoptDeviceBytesAsHost` branches in `src/vllm/model_executor/models/qwen3_5_weights.cpp`. That predicate defaults to `false` (`include/vt/backend.h`), so a backend has to opt in, and THREE now do. `VulkanBackend` (`src/vt/vulkan/vulkan_backend.cpp`) returns `true` unconditionally, because every allocation it makes is `HOST_VISIBLE|HOST_COHERENT` and persistently mapped. Since [`cffe59b02`](https://github.com/mudler/vllm.cpp/commit/cffe59b02) ([#1477](https://github.com/mudler/vllm.cpp/issues/1477), which moved the portable reference tier off the WIDER `UnifiedMemory()`) `MetalBackend` (`src/vt/metal/metal_backend.mm`) answers `MetalContext::unified_memory()`, i.e. `dev.hasUnifiedMemory`, and `RocmBackend` (`src/vt/rocm/rocm_backend.hip`) answers its own `unified_memory_`, i.e. a managed allocator or an integrated part with `PageableMemoryAccess`. **So the lever ACTS on Apple silicon and on an integrated ROCm part, and nobody has measured it on either** — every number above is GB10 through Vulkan. Read those two as reachable-and-unmeasured rather than as covered by this row's numbers ([#1502](https://github.com/mudler/vllm.cpp/issues/1502)). It is still inert on CUDA and CPU, neither of which overrides the default. That is now CHECKED rather than merely observed ([#1635](https://github.com/mudler/vllm.cpp/issues/1635)), and in two halves, because no CI job has a GPU on which a live `CudaBackend` could answer: a `static_assert` beside the class in `src/vt/cuda/cuda_backend.cu` fails the `cuda-fat-build` compile if `CudaBackend` ever declares its own `DeviceMemoryIsHostAddressable`, so its answer IS the inherited one; and a case in `tests/vt/test_backend.cpp` pins that inherited default to `false` on a subclass that declares no override, on every host lane. Do NOT cite `tests/vllm/platforms/test_platform.cpp` for this: its `FakeUnifiedAddressablePlatform` reports `device_type() == kCUDA` but its `backend()` returns `vt::GetBackend(DeviceType::kCPU)`, so its `CHECK_FALSE(gb10.backend().DeviceMemoryIsHostAddressable())` reads the CPU backend. `tests/vt/test_cuda_backend.cpp` observes the real backend but skips wherever no GPU is present, so it asserts nothing in CI. The CPU backend reports `UnifiedMemory() == true` while this narrower predicate stays `false` — the two properties disagreeing is the whole reason the second one exists | | `VT_QWEN35_ALIAS_HOST_WEIGHTS` | on (Qwen3.5/3.6 family; acts only where the platform advertises `host_memory_is_device_addressable()` — a GB10-class integrated part today) | Hand a dense weight's HOST bytes straight to the device kernel instead of allocating a device copy and uploading into it. On a part whose kernels can dereference host storage that copy buys nothing and costs a second full resident copy of every dense weight out of the same RAM. MEASURED on GB10, `Qwen3.8-2.4T-A95B UD-Q1_0` (369.97 GiB) on `--device cuda`: with the second copy the load completes at 61.20 GiB resident and then exhausts the 119.631 GiB box inside its FIRST forward, **zero decode steps over seven attempts** (issue #1299); with the alias the same run reaches **32/32 decode steps at peak RSS 97.75 GiB**. The instrument counts 60.793 GiB of dense weight aliased rather than duplicated (first-forward totals, at call 1361, where re-homing plateaus) against ~9.2 GiB of misaligned GGUF borrows that decline and still stage. `0` is the same-binary A/B back to the staging behaviour, and it is not only a bisect lever: `src/vllm/model_executor/models/laguna.cpp` records a MEASURED GB10 penalty for reading system-allocated memory from the GPU rather than a `cudaMalloc` allocation, worst on a long-K low-parallelism GEMV, so a decode regression has to be separable from the workload. The substitution is otherwise indistinguishable — the aliased pointer is re-homed to 256 bytes, cuBLASLt's own `CUBLASLT_MATMUL_PREF_MIN_ALIGNMENT_A_BYTES` default, which dominates every explicit pointer gate in the CUDA kernels (the strictest asks 32). A DISCRETE device answers the predicate false and is byte-for-byte unchanged. No effect on CPU, Vulkan, Metal or XPU, whose platforms do not advertise the property | | `VT_LOAD_DIRECT_UPLOAD` | on | Load a weight the device consumes VERBATIM by VIEWING the safetensors mmap (`OwnedBytes::Borrow`, keep-alive on the mapping) instead of copying it into an owned host buffer first, so the device upload reads the file mapping and the load moves those bytes ONCE rather than twice. Only whole-range same-size copies qualify — a transpose, a dtype conversion, a dequant, a concatenation or a load-time repack always takes the copy path, and the helper re-checks `numel * sizeof(dtype) == span` and fails closed to the copy on any mismatch. `0` is the same-binary A/B back to copy-then-upload. Bytes are identical either way, so tokens are unchanged. MEASURED on GB10, Qwen3.6-27B bf16 (50.098 GiB), Vulkan, same binary both arms: the weight-load phase goes **19.27 -> 12.48 s warm** (1.54x) and **52.62 -> 32.75 s cold** (1.61x), load-and-one-token **30.39 -> 22.47 s** warm and **62.98 -> 55.60 s** cold, with every ON leg beating every OFF leg. Total bytes MOVED **100.196 -> 81.260 GiB**: the host materialization pass drops **50.098 -> 31.162 GiB** while the 50.098 GiB device upload is unchanged (the model still has to be uploaded once). 37.8% of this checkpoint qualifies; the rest is merged (qkv, gate_up) or transposed (lm_head) at load and correctly still copies | | `VT_LOAD_STATS` | off | `=1` prints one line per load phase (mmap+header, weights) with its wall time, plus the bytes the load MOVED: `host_copy` (source bytes materialized into an owned host buffer), `borrowed` (source bytes viewed in place by the direct-upload path) and `device_upload` (bytes copied host to device). Diagnostic only; it changes no numerics. Issue #150 | diff --git a/src/vt/cuda/cuda_backend.cu b/src/vt/cuda/cuda_backend.cu index d654cfa65..27c2a69a7 100644 --- a/src/vt/cuda/cuda_backend.cu +++ b/src/vt/cuda/cuda_backend.cu @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "vt/backend.h" @@ -325,6 +326,46 @@ class CudaBackend final : public Backend { std::unique_ptr dedup_; }; +// ISSUE #1635, and the only thing in this tree that holds `CudaBackend`'s answer +// to `Backend::DeviceMemoryIsHostAddressable()`. +// +// That predicate gates the portable CPU reference tier (`src/vt/op_provider.cpp`), +// the weight loader's `VT_ADOPT_DEVICE_BYTES` adoption +// (`src/vllm/model_executor/models/qwen3_5_weights.cpp`) and the logits-processor +// bounce (`src/vllm/v1/sample/logits_processor/builtin.cpp`). CUDA must answer +// `false`: `Alloc` above is `cudaMalloc`, and a `cudaMalloc` pointer is not +// host-dereferenceable even on GB10, where `UnifiedMemory()` answers true because +// host and device address one physical RAM. Reading the WIDE predicate instead is +// what SIGSEGV'd the reference tier (#844, #1435). +// +// It is checked HERE, at compile time, because no CI job has a GPU. `cuda-fat-build` +// is the only job with a CUDA toolchain, it builds with `-DVLLM_CPP_BUILD_TESTS=OFF`, +// and the registrar below leaves `kCUDA` unregistered when `cudaGetDeviceCount` +// finds no device -- so a runtime `TEST_CASE` reading this backend would report a +// skip on every lane forever, and a skip reads as a pass. That is the shape of +// evidence #1635 was filed about, and repeating it here would be the same defect. +// +// Taking the address of an INHERITED member through a derived class yields a +// pointer-to-member of the class that DECLARES the member. So this type is +// `bool (Backend::*)() const` exactly while `CudaBackend` declares no override of +// its own, and it becomes `bool (CudaBackend::*)() const` the moment somebody adds +// one. The assertion therefore fires on ANY override, including one that returns +// `false`: an override invalidates the reasoning that CUDA inherits the base +// answer, whatever value it happens to return. +// +// This is HALF the claim. It says CUDA's answer IS the base default; it cannot say +// what that default is. The other half is the `Backend::DeviceMemoryIsHostAddressable +// defaults to false` case in `tests/vt/test_backend.cpp`, which reads the default on +// a subclass that declares no override and runs on every host lane. Change either +// half and re-derive the record in +// `.agents/specs/vt-reference-tier-host-addressable.md` before you change this one. +static_assert(std::is_same_v, + "CudaBackend now declares its own DeviceMemoryIsHostAddressable. The " + "reference tier, VT_ADOPT_DEVICE_BYTES and the logits-processor bounce " + "all read that predicate, and the record (#1635) states that CUDA " + "answers the inherited false. Re-derive the record before changing this."); + // Registers kCUDA during static init (registration must complete before // main() per the backend.h contract). The probe must stay silent on machines // that have the CUDA toolkit but no usable GPU: no throw, no print — it just diff --git a/tests/vt/test_backend.cpp b/tests/vt/test_backend.cpp index 7944eff19..cefe7529c 100644 --- a/tests/vt/test_backend.cpp +++ b/tests/vt/test_backend.cpp @@ -173,3 +173,51 @@ TEST_CASE("CPU events carry a null handle (synchronous degeneration)") { cpu.DestroyEvent(e); cpu.DestroyEvent(e); } + +// ISSUE #1635. `Backend::DeviceMemoryIsHostAddressable()` decides whether a HOST +// kernel may dereference a DEVICE allocation. Three landed levers read it: the +// portable CPU reference tier (`src/vt/op_provider.cpp`), the weight loader's +// `VT_ADOPT_DEVICE_BYTES` adoption (both `AdoptDeviceBytesAsHost` branches in +// `src/vllm/model_executor/models/qwen3_5_weights.cpp`) and the logits-processor +// bounce (`src/vllm/v1/sample/logits_processor/builtin.cpp`). Being wrong here +// hands a device pointer to a host memcpy, which is what #844 and #1435 measured +// as a SIGSEGV, so the polarity `include/vt/backend.h` chose is that a backend +// must OPT IN and everything else inherits `false`. +// +// Nothing read that inherited value. Every fake in this tree overrides the method +// -- `tests/vt/test_reference_tier.cpp`, `tests/vllm/test_load_direct_upload.cpp`, +// `tests/vllm/test_qwen36_weights.cpp` and the rest all take the answer as a +// constructor argument -- so each of them measures its own override and none of +// them measures the default. This subclass deliberately declares no override, +// which is the only way to read the default itself. +// +// It is also the half of the CUDA answer that a host lane can hold. `CudaBackend` +// (`src/vt/cuda/cuda_backend.cu`) declares no override either, and a +// `static_assert` beside that class fails the build if one ever appears -- so +// CUDA's answer IS this value, and flipping this default would flip CUDA's answer +// with no CUDA source changed and no CUDA lane able to notice. +namespace { +class DefaultsOnlyBackend final : public Backend { + public: + void* Alloc(size_t) override { return nullptr; } + void Free(void*) override {} + void Memset(Queue&, void*, int, size_t) override {} + void Copy(Queue&, void*, const void*, size_t) override {} + Queue CreateQueue() override { return Queue{Device{DeviceType::kXPU, 0}, nullptr}; } + // The WIDE predicate answers true on purpose, so the case below cannot pass by + // the two questions happening to agree: this is the GB10 CUDA shape, where host + // and device address one physical RAM and a `cudaMalloc` pointer is still not + // host-dereferenceable. + bool UnifiedMemory() const override { return true; } + // NO DeviceMemoryIsHostAddressable override. That absence is the subject. +}; +} // namespace + +TEST_CASE("Backend::DeviceMemoryIsHostAddressable defaults to false") { + DefaultsOnlyBackend b; + REQUIRE(b.UnifiedMemory()); + CHECK_FALSE(b.DeviceMemoryIsHostAddressable()); + // Not one bit read twice: the narrow predicate disagrees with the wide one on + // exactly the device shape that motivated splitting them. + CHECK(b.UnifiedMemory() != b.DeviceMemoryIsHostAddressable()); +} diff --git a/tests/vt/test_cuda_backend.cpp b/tests/vt/test_cuda_backend.cpp index 1f8a360f8..6bdc31338 100644 --- a/tests/vt/test_cuda_backend.cpp +++ b/tests/vt/test_cuda_backend.cpp @@ -202,3 +202,37 @@ TEST_CASE("CUDA backend reports the device compute capability") { CHECK(cuda.DeviceCapabilityMinor() == minor); #endif } + +// ISSUE #1635, the empirical half. This case asserts NOTHING on any CI lane: no +// job in this repository's CI has a GPU, so `HasCuda()` is false everywhere CI +// runs and the case reports a skip. It is here because a leased GPU box can run +// it, and because the answer it observes was once cited to a test that read a +// different backend entirely -- which is the defect #1635 exists for. +// +// What holds this answer on every push needs no device, and is deliberately NOT +// this case: +// +// * `src/vt/cuda/cuda_backend.cu` carries a `static_assert` that `CudaBackend` +// declares no `DeviceMemoryIsHostAddressable` override, so its answer IS the +// inherited one. `cuda-fat-build` compiles that translation unit. +// * `tests/vt/test_backend.cpp` pins the inherited default to `false` on a +// subclass that declares no override. Every host lane runs it. +// +// Cite those two, never this one, for the claim that CUDA answers false. +TEST_CASE("CUDA backend refuses host dereference of device memory (GPU box only)") { + if (!HasCuda()) { + MESSAGE("no CUDA backend registered; NOTHING was observed here (#1635) — the " + "answer is held by the static_assert in src/vt/cuda/cuda_backend.cu " + "and by tests/vt/test_backend.cpp"); + return; + } + Backend& cuda = vt::GetBackend(DeviceType::kCUDA); + // `Alloc` is `cudaMalloc`, whose pointers the host may not dereference. On GB10 + // `UnifiedMemory()` answers TRUE at the same time, because host and device + // address one physical RAM — the two questions are different, and the reference + // tier reading the wide one is what produced the SIGSEGV in #844 and #1435. + MESSAGE("CUDA UnifiedMemory=" << cuda.UnifiedMemory() + << " DeviceMemoryIsHostAddressable=" + << cuda.DeviceMemoryIsHostAddressable()); + CHECK_FALSE(cuda.DeviceMemoryIsHostAddressable()); +}