diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 9f482b151..1904eab8b 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -499,6 +499,8 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1471](https://github.com/mudler/vllm.cpp/issues/1471) | `LTX25-DEVICE-RESIDENCY` | **`vt::Conv2d` and `vt::DepthwiseConv1d` compute their output extents with C++ integer division, which truncates toward zero, where torch's shape contract FLOORS.** `src/vt/ops.cpp:2749-2750` (`hout`/`wout`) and `:2883` (`lout`). The two agree for a non-negative numerator and disagree for a negative one whenever `stride > 1`: at `in = 2, k = 3, stride = 2, pad = 0` the numerator is -1, so torch computes `floor(-1/2) + 1 = 0` and raises "Output size is too small" while truncation computes `-1/2 + 1 = 1` and ACCEPTS an extent of 1, convolving over taps the stride skipped. The `VT_CHECK(... > 0)` guard below each line cannot catch it, because truncation has already produced a positive extent. Found by the fresh review of [#1007](https://github.com/mudler/vllm.cpp/issues/1007) (finding F7) against the sibling `vt::Conv3d`, which carried the identical expression and is FIXED on that row: the span is separated from the division and a negative span is refused by name, proven by mutation — reverting the guard reds *conv3d: the shape contract refuses by name* at exit 1 (`CHECK_THROWS` did not throw, `tests/vt/test_ops_conv3d.cpp:446`), restoring it gives 4/4 and 2036 assertions, both arms BUILT=YES with 0 compile errors. NOT FIXED IN FLOW: extending the repair means two more red-first cases and a fresh review over ops #1007 does not own, and **no caller is yet known to reach either path** — the LTX-2.5 `CausalConv3d` geometry that motivated the Conv3d fix materialises a pad of at least the kernel, and the audio-encoder `kConv2d`/`kDepthwiseConv1d` callers are unaudited, which is part of the owed work. `Conv1dOutLength` (`:2886`) and `ConvTranspose1dOutLength` already guard it correctly with an explicit negative-span return, so three of the five conv wrappers use the right shape already. Listed under `## Owed` in [`ltx25-device-residency.md`](specs/ltx25-device-residency.md) | bug | | [#1466](https://github.com/mudler/vllm.cpp/issues/1466) | `MODEL-MUSE-GLIMMER` | **`tests/vllm/models/test_muse_glimmer_text.cpp:532`'s `CHECK(diff <= 5e-4)` is a rounded-up W1 measurement, and one correct kernel change already spent 45 points of its 76-point margin.** `3a54c4b7d`'s own body quotes the number the constant was rounded up from ("max abs diff 1.21e-4 on logits of max 4.88e-2"); there is no derivation beside it. `4712dac40` narrowed `act(gate)` to the input dtype — upstream's polarity, and the only form that reproduces the committed `silu_and_mul_bf16_8x256` oracle golden bit-exactly — and grew the envelope 2.8x to 3.43e-04, which is 0.687 of the bound (measured at `aeba0de6f`, CPU-only Release, x86_64). It is the last member of that class in this file: [#1458](https://github.com/mudler/vllm.cpp/issues/1458) replaces the other one (`bdiff <= 1e-5`) in the same case and leaves this one to its own derivation, because a bigger constant is not a repair. Found in flow while gating #1458 (PR [#1461](https://github.com/mudler/vllm.cpp/pull/1461)); also listed under `## Owed` in [`muse-glimmer.md`](specs/muse-glimmer.md) | bug | | [#1467](https://github.com/mudler/vllm.cpp/issues/1467) | `MODEL-DIFFUSION-ltx-2-5-ltx2-video-transformer-3d-model` | **The ltx2 prompt->conditioning case no longer detects position renumbering, and post-`4712dac40` the instrument is INVERTED — the mutant is closer to the oracle than the correct code.** MEASURED in one build directory, CPU-only Release, x86_64, compile rc 0 on every arm and every source restored `sha256sum`-verified, as ratios to the propagated floor: before `4712dac40`, correct 0.565/0.688 and renumbered 0.831/**1.099** (the 1.10x the case's own note claims, and it reded); at `aeba0de6f`, correct **1.209/1.313** and renumbered 0.683/0.931 — so at the old `1.0x` bound the case reds the port and passes the defect. `4712dac40` is right; what it did here was raise this comparison's noise floor above the defect's signal, and no constant recovers a detection whose ordering has reversed. [#1458](https://github.com/mudler/vllm.cpp/issues/1458) restores a functioning instrument at a derived `2.0x` (its scatter-offset mutation still reds at 14.08x/24.06x) and does not claim this coverage. A repair owes an instrument with no bf16 accumulation between the defect and the assertion — the integer `positions` contract, or the f32 rope table `scripts/gen-ltx2-gemma-tower-goldens.py:363-375` already names as the right one for this class. Found in flow while gating #1458 (PR [#1461](https://github.com/mudler/vllm.cpp/pull/1461)); also listed under `## Owed` in [`ltx-2-5.md`](specs/ltx-2-5.md) | bug | +| [#844](https://github.com/mudler/vllm.cpp/issues/844) | `VT-REFTIER-HOST-ADDRESSABLE` | **The portable CPU reference tier gates on `Backend::UnifiedMemory()`, which is not the property a host kernel needs, so a device tensor reaches a host kernel and the process gets SIGSEGV under a banner reading `correct but slow`.** The eligibility predicate is `ReferenceTierEligible` in `src/vt/op_provider.cpp`; grep the symbol, because this index is append-only and a line number here would be permanent. It asks `Backend::UnifiedMemory()`. The question a host kernel actually asks is `Backend::DeviceMemoryIsHostAddressable()`, whose own comment in `include/vt/backend.h` records the refutation: CUDA on GB10 reports unified memory because host and device address the same physical RAM, yet a plain `cudaMalloc` pointer is still not host-dereferenceable, and its default is `false` because "being wrong here hands a device pointer to a host memcpy and segfaults". `CudaBackend::Alloc` calls `cudaMalloc`, so every CUDA op without a native kernel installed the CPU host kernel and crashed. Measured twice on GB10: `vt::QuantFp8Static` on `sm_110` ([#960](https://github.com/mudler/vllm.cpp/issues/960)), whose spec `## Outcome` relocated the one registration and recorded that the CLASS stayed open; and `vt::MatmulFp8BlockScaled` on a CUTLASS-less CUDA build ([#1435](https://github.com/mudler/vllm.cpp/issues/1435)), which exits 139 with `test cases: 0 assertions: 0`. Fixed by asking the narrow predicate, and by `MetalBackend` and `RocmBackend` answering it truthfully so neither loses the tier: both answers are equal to what those backends report today, so only the CUDA cell moves. The refusal now names why the portable tier did not run, and the banner names its precondition instead of asserting correctness. Reproduced without a GPU in `tests/vt/test_reference_tier.cpp` by a fake backend carrying the GB10 property pair. Spec [`vt-reference-tier-host-addressable.md`](specs/vt-reference-tier-host-addressable.md) | bug | +| [#1482](https://github.com/mudler/vllm.cpp/issues/1482) | `VT-REFTIER-HOST-ADDRESSABLE` | **The remainder of [#844](https://github.com/mudler/vllm.cpp/issues/844), split out so it keeps an open tracker after the crash itself is fixed.** #844 asked for four things. The reference tier no longer hands a device tensor to a host kernel, and the banner no longer claims correctness it cannot hold; those two landed with this row. Two did not. Item 1 also wants the refusal to name the BUILD FEATURE that would have provided the native kernel: the landed message names the op, the device, and which tier precondition the device failed, but not `this build has no cutlass-fp8`, which is the sentence that would have turned #1435's measurement around in one read. It is absent because the op table holds no such mapping -- an `OpId` does not know which CMake feature cell compiles its registering translation unit -- and inferring one by scanning a kernel body for `cutlass` tokens was already argued and rejected in [`vt-fp8-quant-arch-gate.md`](specs/vt-fp8-quant-arch-gate.md) `## Outcome` as transitive through helpers, hence both false-positive and false-negative. A real fix needs a declared, checkable mapping, which is a design question and not a message change. Item 4 -- whether a CUDA build lacking CUTLASS should warn at ENGINE CONSTRUCTION rather than only at configure time -- is untouched. The refusal fires at first dispatch of the missing op, which can be deep inside a model forward, long after the configure log is gone. Filed rather than deferred silently: the fixing pull request carries `Fixes #844`, so without this row the remainder would point at a closed issue and have no owner, which is the ownerless-owed state the protocol forbids. Listed under `## Owed` in [`vt-reference-tier-host-addressable.md`](specs/vt-reference-tier-host-addressable.md) | bug | | [#1489](https://github.com/mudler/vllm.cpp/issues/1489) | `SPEC-DFLASH2` | **`vt::TopKValuesIndices`'s CUDA arm does not order NaN first, and the CPU arm's contract says it does — 12 failed assertions, all one literal row.** FIRST DEVICE RUN of the DFlash2 kernels: an `rc` job on `dgx:gpu0` (GB10, sm_121a, `nvcc` 13.0 matched to the driver) at `SPEC-DFLASH2` W3 head `b29b6f8869a9eeacc451647e859498491ef6bf1e`, 2026-08-20. It settles more than it opens. `BUILD_RC=0` and `COMPILE_ERRORS=0` for all six DFlash2 suites, so `## Owed` O6 is DISCHARGED — W2's grouped convolution and both W3 ops had never been through `nvcc`. The device arms genuinely RAN, which is the precondition that separates this from a green skip: zero `no CUDA backend; skipping` lines, and `test_ops_topk_values_indices` reports **562 assertions on device against 202 on the CPU-only build**. And the tie divergence O10 called the real risk did NOT materialise: the CPU arm sorts under an explicit comparator while the CUDA arm threshold-searches, compacts and fills from the lowest-indexed equals, and on the straddling group, the group larger than k, the ties inside the kept set and the `-inf`-saturated row the two algorithms agree. Five of the six suites pass. THE ONE FAILURE: 7 cases, 5 passed, 2 FAILED; 562 assertions, 550 passed, 12 FAILED; `Status: FAILURE!`, every one of the twelve on the literal row `"NaN sorts first, as torch.topk does"`, including the direct cross-arm pair `CHECK( gpu.indices[i] == cpu.indices[i] )` reading `2 == 1` and `CHECK( std::isnan(gpu.values[i]) )` reading `false` — a genuine backend disagreement, not a wrong expectation on one side. MECHANISM, derived from `src/vt/cuda/cuda_sample.cu:548-698` before the results were read: `TopKValuesIndicesRowKernel` brackets with `fmaxf`/`fminf`, which return the non-NaN operand, and selects survivors with `r[j] > thr`, which is false for a NaN, so the kernel can never select one whatever the threshold converges to. NOT A SHIPPED DEFECT: no production path feeds this op a NaN logit, because the candidate values come from a target LM head. It IS a red suite on any CUDA build, which AGENTS.md `## Gates` does not permit to stand as recorded debt. REPAIRED BY NARROWING in the W3 repair delta: the row is excluded BY NAME from both device cases and kept on the CPU arm where it is the mutation-proven guarantee, `include/vt/ops.h` states the asymmetry beside the contract instead of asserting an ordering no backend delivers, and the exclusion's match count is asserted on the CPU arm so a filter that matched nothing cannot hide. WHAT THIS ISSUE OWNS: reconciling `TopKValuesIndicesRowKernel` to the NaN-first contract, which is what makes `ops.h` true on both arms and lets the row go back into the device cases. It needs a lease to verify and is not attemptable from a host with no `nvcc`. Reproduce with `/mnt/nas_share/rc/dflash2/cuda-gate.sh` at that SHA; per-suite logs at `/workspace/dflash2/cuda-.log`. Two instrument caveats recorded there: `NVCC_INVOCATIONS` read 0 and `CUDA_CONFIGURED` read `unknown`, both defects in the harness rather than facts about the build — the parity assertions ran and disagreed, which is what proves CUDA was active. Listed under `## Owed` O10 in [dflash2-spec-decode.md](specs/dflash2-spec-decode.md) | bug | | [#1493](https://github.com/mudler/vllm.cpp/issues/1493) | `LTX25-DFR-ROUNDS` | **DFR's unclamped `2**round_idx` tile count is mirrored and gated by nothing, because every fixture canvas in this tree has ONE keyframe segment.** `tile_ranges` clamps to `min(num_tiles, n_segments)` (`dfr_layout.py:171`), and the 9-frame fixture pads to a 25-frame canvas with a single segment, so round 1 asks for 2 windows and gets 1 and round 2 asks for 4 and gets 2. A port computing `round_idx + 1`, or `2 * round_idx`, or capping at 2 returns the SAME tile counts on every test here, with every downstream shape, frame count and exit status identical. NOT FIXED IN FLOW and the judgement is recorded rather than reversed: reaching 4 segments needs a materially longer canvas, so round 2 would denoise 4 tiles on a canvas already doubled twice - a new fixture and a substantially longer CPU run in a file that already carries 102 cases, not an assertion added to the existing render. The bound is stated in the test body and in `docs/USAGE.md`, so it was disclosed before it was owned; this row is the ownership. Closing it needs one render whose canvas carries at least 4 segments plus an assertion that `round_tile_counts` reads the unclamped `2**round_idx` for at least one round, which is the only shape that separates `2**round` from every expression agreeing with it at 1 and 2. Listed under `## Owed` in [`ltx25-dfr-rounds.md`](specs/ltx25-dfr-rounds.md) | bug | | [#1494](https://github.com/mudler/vllm.cpp/issues/1494) | `LTX25-DEVICE-RESIDENCY` | **A SECOND LTX-2.5 phase-coverage ratio decides by box load, and it is NOT the one [#1439](https://github.com/mudler/vllm.cpp/issues/1439) tracks.** `ltx2 video: the three carrying phases contain their work and the load keeps its order` asserts `CHECK_MESSAGE(covered >= c.min_coverage * leaf_seconds, ...)` at `tests/vllm/multimodal/test_ltx2_video.cpp:3696`; #1439 is `CHECK(leaves >= 0.95 * wall)` in a DIFFERENT case, now at `:3259`. Closing one does not close the other. Measured 2026-08-20, x86_64 `Release` `VLLM_CPP_CUDA=OFF`, three consecutive full-suite runs of ONE binary (`sha256 8fdbc31d...`) with no source change: loadavg 10.45 gives 94.6039% RED, a quieter run gives 96.8506% green, loadavg 16.53 gives 94.6039% RED. The comparison prints as `CHECK( 0.00414483 >= 0.00416218 )` - `denoise` is 0.00438124 s, its eight named sub-scopes cover 0.00414483 s, so the un-named residue is 0.00023641 s and **the margin is a quarter of a millisecond**. Same scheduling polarity #1439 recorded, which is the tell that this is the instrument and not the code: the run that PASSED is the run where `denoise` took 0.00940481 s, more than twice the failing runs', because the residue grows more slowly than the leaf it is divided by. NOT FIXED IN FLOW: bounding the residue in SECONDS beside the ratio, so the assertion says the same thing at fixture and production scale, changes a gate's semantics and needs its own row, spec and red-first evidence per `AGENTS.md` `## Changing the rules or a checker` - the same conclusion #1439 reached, and the two should be repaired together because one seconds bound would serve both. Found by the fresh implementer repairing the review findings of [#1481](https://github.com/mudler/vllm.cpp/pull/1481); pre-existing and not that PR's defect | bug | diff --git a/.agents/specs/vt-reference-tier-host-addressable.md b/.agents/specs/vt-reference-tier-host-addressable.md new file mode 100644 index 000000000..7af466d75 --- /dev/null +++ b/.agents/specs/vt-reference-tier-host-addressable.md @@ -0,0 +1,289 @@ +# VT-REFTIER-HOST-ADDRESSABLE — the portable reference tier gates on the wrong memory property + +Issues: [#844](https://github.com/mudler/vllm.cpp/issues/844) (the class), +[#1435](https://github.com/mudler/vllm.cpp/issues/1435) (the second measured instance). + +Row: `VT-REFTIER-HOST-ADDRESSABLE`. Seam: `BACKEND-ACCEL-PROVIDER` +(`.agents/backend-matrix.md`), work row `S5` of +[`accelerator-seam-audit.md`](accelerator-seam-audit.md). + +## What is wrong + +`ReferenceTierEligible` (`src/vt/op_provider.cpp`) decides whether a device may +receive the portable CPU reference tier. It asks `Backend::UnifiedMemory()`. +That is the wrong question, and `include/vt/backend.h` already says so beside +`Backend::DeviceMemoryIsHostAddressable()`: + +> CUDA on GB10 reports unified memory because host and device address the same +> physical RAM, yet a plain `cudaMalloc` pointer is still not +> host-dereferenceable. + +`CudaBackend::Alloc` calls `cudaMalloc` and `CudaBackend::UnifiedMemory()` +returns the probed `unified_memory_`, which is true on GB10. So on GB10 every op +that has no native CUDA kernel installs the CPU host kernel as a `vt-cpu-ref` +provider, the host kernel dereferences `cudaMalloc` pointers, and the process +receives SIGSEGV. The banner printed immediately before the crash claims the +opposite: + +```text +[vt reference-tier] op=MatmulFp8BlockScaled device=cuda has NO native kernel; + running the PORTABLE CPU fallback (correct but slow) +``` + +Two measured instances, both on `dgx:gpu0` (GB10, compute capability 12.1): + +1. `vt::QuantFp8Static` on an arch outside the `cutlass-fp8` cell + ([#960](https://github.com/mudler/vllm.cpp/issues/960), Thor `sm_110`). + `.agents/specs/vt-fp8-quant-arch-gate.md` `## Outcome` relocated that one + registration and recorded that it did **not** close the class: "The portable + reference tier still accepts `DeviceType::kCUDA` tensors and still calls + itself 'correct but slow' while dereferencing device pointers." +2. `vt::MatmulFp8BlockScaled` on a CUDA build with no CUTLASS headers + ([#1435](https://github.com/mudler/vllm.cpp/issues/1435), 2026-08-20). + `tests/vt/test_ops_matmul_fp8_block_cuda` exits 139 with + `test cases: 0 assertions: 0`. Evidence logs + `/mnt/nas_share/rc/fp8block-g2/run.log` and `run2.log`. + +The second instance is reachable on a default build: `VLLM_CPP_CUTLASS_FETCH` +defaults `OFF` and CUTLASS is not a submodule, so a plain clone plus +`-DVLLM_CPP_CUDA=ON` reproduces it. + +The class is not FP8 and it is not CUDA. Any op whose native kernel is +feature-gated reproduces it on any backend whose `Alloc` returns memory the host +cannot dereference. #844 states this in its own words: "The crash is the lucky +outcome. The silent pass is the dangerous one." + +## Scope + +In scope: + +1. `ReferenceTierEligible` asks the narrow question — may the host dereference + what `Backend::Alloc` returned — instead of the wide one. +2. The refusal that follows names why the portable tier did not rescue the + caller, so a reader does not have to know that the tier exists. +3. The reference-tier banner stops asserting correctness it cannot hold. It + states the precondition that now gates it. +4. `MetalBackend` and `RocmBackend` answer the narrow question truthfully, so + both keep the tier they have today. Neither answer changes any behaviour; + see [Design](#design). +5. A host-tier test that reproduces the condition without a GPU. + +Out of scope, and why: + +- **The `vt_cuda_report_feature` half of #1435.** `CMakeLists.txt` reports + `CUDA feature cutlass-fp8: ENABLED for [121a]` from the architecture + intersection alone, before the CUTLASS-header detection later in the same + file, so it reported a capability the build did not contain. That is a + configure-time reporting defect on a different surface, with a different + failure mode, and it cannot be exercised at all without a CUDA compiler — the + authoring host has no `nvcc` and no GPU. Bundling it here would put a change + that only a leased box can verify inside a change that a CPU box verifies + completely. Recorded under [`## Owed`](#owed). +- **Copy-in and copy-out instead of refusal.** Rejected under + [Design](#design). +- **Registering a CUDA kernel for `kMatmulFp8BlockScaled` on a CUTLASS-less + build.** That is the FP8 row's question, not this one. This row makes the + missing kernel a named refusal; it does not supply one. + +## Upstream anchors + +vLLM has no mirror for this seam and cannot have one: `CustomOp.forward_native` +(`vllm/model_executor/custom_op.py:138` at the pinned oracle) is pure torch, and +torch tensors carry their device, so torch never dispatches a CPU body against +CUDA storage — it raises. The reference tier is the vllm.cpp original that +stands in for `forward_native` (inventory deviation §9.1), and this row restores +the property torch gets for free: a portable body never runs on storage it +cannot address. + +## Design + +**The predicate already exists.** `Backend::DeviceMemoryIsHostAddressable()` +(`include/vt/backend.h`) is documented as "STRICTLY NARROWER than +`UnifiedMemory()`, and the difference is the whole reason it exists", and its +default is `false` because "being wrong here hands a device pointer to a host +memcpy and segfaults". That is precisely the reference tier's precondition. The +fix is to ask it. No new seam, no new virtual, no `DeviceType` branch — the +audit's rule that eligibility must never key on the device type is kept. + +**What each backend answers, and what changes.** + +| Backend | `Alloc` | `UnifiedMemory()` | host addressable | tier before | tier after | +|---|---|---|---|---|---| +| CPU | host | true | n/a — the tier's source device is never a target | no | no | +| CUDA | `cudaMalloc` | probed; true on GB10 | **false** | **yes** | **no** | +| Vulkan | host-visible and host-coherent, persistently mapped | true only where a combined host-visible, host-coherent, DEVICE_LOCAL type exists | true, unconditionally (already overridden) | yes, **except** where unified is false | yes, always | +| Metal | `MTLResourceStorageModeShared` | `hasUnifiedMemory` | true, once it answers | yes | yes | +| ROCm | `hipMallocManaged` when integrated and managed-capable, else `hipMalloc` | `managed_alloc` or `pageable_memory_access && integrated` | equal to `UnifiedMemory()` | yes | yes | + +Two cells move. CUDA is the crash, and it loses the tier. **Vulkan widens**, and +this is not a side effect worth hiding: `VulkanBackend::DeviceMemoryIsHostAddressable()` +already returns `true` unconditionally, while `VulkanBackend::UnifiedMemory()` +reads `VulkanContext::unified_memory()`, which the context sets from +`FindMemoryType(mem, ~0u, kHostFlags | VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) >= 0`. +On a device with no such combined type that flag is false, the context falls back +to `FindMemoryType(mem, ~0u, kHostFlags)` and then `VT_CHECK`s that one was +found. So every allocation is host-visible and host-coherent regardless, and the +tier that used to be withheld on that device is now installed. The widening is +sound because the `VT_CHECK` is what guarantees it, not because the tier is +harmless. + +Metal and ROCm need the one-line override so that they do not lose the tier as a +side effect; both overrides are by construction equal to what those backends +answer today, so their behaviour is byte-identical. ROCm's answer is its own `unified_memory_` rather than +`managed_alloc_`: the registrar widens `unified_memory_` with +`pageable_memory_access && integrated`, and on that branch `hipMalloc` memory is +host-dereferenceable by the same attribute, which the ROCm backend's own comment +records as measured on gfx1151 and gfx1103. + +**Why refusing, and not copying in and out.** The tier's contract is that it +returns the SAME host kernel pointer the CPU device dispatches +(`ref.fn = src->fn`). A staging fallback cannot live behind that contract: it +would have to wrap every op's argument pack, which is per-op and untyped at this +seam, and it would turn a missing kernel into a silent bandwidth cliff on the +model path. AGENTS.md already settles the polarity — "Refuse an unimplemented +arm with a message that names the missing part" — and #844's "What done looks +like" asks for the refusal. A staging tier is a larger, separate design, and +nothing in the tree asks for it. + +**Why the refusal message grows a clause.** With the gate corrected, the caller +gets `no kernel for op MatmulFp8BlockScaled (id N) on device cuda (type 1)`. +That is already a named refusal, and it is the message #1435 asked for. It does +not say why the portable tier — which the reader may know exists, and which used +to answer — did not run. One clause names the reason and keeps the reader out of +`op_provider.cpp`. + +**Why the banner's wording changes.** After the gate is corrected the banner +only ever prints where the host may address device memory, so "correct" is true +again. It is still the wrong sentence to leave behind: it asserts correctness +without naming what makes it correct, and both #844 and `docs/USAGE.md` quoted +those three words as the thing that misled a reader. `docs/USAGE.md` no longer +quotes them: [#1491](https://github.com/mudler/vllm.cpp/pull/1491) retired the +section that did, which is why this row edits the banner and not that file. #844 +still carries the quote. The replacement names the precondition, which is a fact +the reader can check. + +## Risks + +1. **A backend that is host-addressable but does not say so loses the tier.** + The measured population is five backends and this spec enumerates all five + above. Metal and ROCm gain the override in this change and are byte-identical; + CPU is not a target; CUDA is the defect. **Vulkan is not byte-identical** — it + widens, on a device with no combined host-visible, host-coherent, DEVICE_LOCAL + memory type, for the reason argued under [Design](#design). A backend added + later gets the safe default, which is the polarity `backend.h` chose + deliberately. +2. **Metal and ROCm are not compiled anywhere this change can reach.** CI builds + CPU, CUDA, Vulkan and Windows; there is no macOS or ROCm job, and this host + has neither toolchain. The two edits are therefore one line each, placed + beside the existing `UnifiedMemory()` override in the same class, with the + same signature shape. Their existing tests + (`tests/vt/test_metal_backend.cpp`, `tests/vt/test_rocm_backend.cpp`) assert + tier eligibility and continue to hold, because the override reproduces the + value those tests already compare against. +3. **A CUDA op that relied on the tier starts refusing.** That is the intent, + and it cannot regress a working path: the tier on CUDA either crashed or, on + an op whose operands happened to be host tensors, silently compared the CPU + against itself. #844 names that second outcome as the dangerous one. No op is + known to depend on it; `RefTierHits()` is documented as MUST-be-zero in any + performance arm. +4. **This does not make the block-wise FP8 arm more finished.** It has no token + gate, no speed claim, and correctness only on the seven shapes run under + #1437. This row changes a crash into a refusal and claims nothing else. + +## Tests and gates + +1. **Red first.** `tests/vt/test_reference_tier.cpp` gains a fake backend that + reports `UnifiedMemory() == true` and + `DeviceMemoryIsHostAddressable() == false` — the GB10 CUDA shape, expressed + without a GPU. The new case requires that the tier is refused, that eager + registration installs nothing, that `GetOp` throws, and that the message + names the op and the device. On the unfixed tree the tier installs and + `GetOp` returns the host kernel, so the case fails on the assertion that + states the invariant. +2. **Focused green.** `ctest --test-dir build -R test_reference_tier -V`, read + with ANSI stripped, checking `Status:` and `assertions:` together. +3. **Mutation.** Revert the predicate in a scratch copy and rerun the focused + gate. The new case must go red. Print `git diff --stat` and any compiler + error with the result, because a mutation that never applied and a mutation + that failed to build both read as a passing test. +4. **Reachability, mutated at the production call site.** The new refusal clause + is not proved by calling `ReferenceTierRefusalReason` directly. Delete the + clause from the `VT_CHECK` inside `Resolve` — the production path every + `vt::GetOp` takes — and `tests/vt/test_reference_tier.cpp` goes red on the + `host-addressable` assertion. Run by the fresh reviewer, recorded here because + "nothing lands dead" asks for exactly this and the implementer did not run it. + +5. **No regression in the seam's own suites.** + `ctest --test-dir build -R 'test_op_provider|test_reference_tier|test_backend_cross_device'`. +6. **Record gates.** `scripts/check-agent-record.py`, + `scripts/check-doc-checkpoint.py`, `scripts/check-public-doc-tables.py`, + `scripts/check-gate-commands.py --check`, + `scripts/check-commit-style.py --range origin/main..HEAD`, + `scripts/check-commit-trailers.py --range origin/main..HEAD`. +7. **Not run here, and named as such.** No CUDA build, no GPU. The defect this + row fixes was measured on GB10 and the fix is verified on the host tier + against a fake backend that reproduces the exact property pair. A CUDA + re-measurement — the block-wise FP8 test refusing instead of exiting 139 — + is owed and is listed under `## Owed`. + +## Stop conditions + +- Stop and return `NEEDS_DECISION` if a backend in the tree is found to be + host-addressable and cannot say so through + `Backend::DeviceMemoryIsHostAddressable()`. +- Stop if the focused gate cannot be made red before the fix. A test that is + green on the unfixed tree is measuring something else. +- Do not widen what the reference tier accepts to keep a test green. + +## Evidence + +Red, fixed, and mutation evidence are recorded in the pull request body, which +is the landed commit message. The two GB10 logs that motivated the row are +`/mnt/nas_share/rc/fp8block-g2/run.log` and `run2.log`. + +## Owed + +- [#1435](https://github.com/mudler/vllm.cpp/issues/1435) stays OPEN after this + row. Its second defect — `vt_cuda_report_feature` reporting + `CUDA feature cutlass-fp8: ENABLED` from the architecture intersection alone, + before the CUTLASS-header detection later in `CMakeLists.txt` — is not fixed + here. It needs a CUDA configure to exercise, which this host cannot run. + `cmake/CudaArchFeatures.cmake` opens by naming this exact class as what the + feature table exists to prevent, so the diagnostic asserts the opposite of the + truth. + + REPRODUCED AGAIN, on a second box, while this row's `nvcc` check ran. Job + `645bf395-23fc-408f-a9ad-b9823885622c` on `thor:gpu0` (NVIDIA Thor, CUDA 13.0 + V13.0.88, aarch64) configured this branch at `817e1769c` with + `-DVLLM_CPP_CUDA=ON -DVLLM_CPP_CUDA_ARCHITECTURES=121a` and NO CUTLASS at all, + and the configure printed: + + ```text + CUDA feature cutlass-nvfp4: ENABLED for [121a] + CUDA feature cutlass-fp8: ENABLED for [121a] + ``` + + So the defect is not specific to `dgx:gpu0` and not specific to `cutlass-fp8`. + Every CUTLASS-dependent feature cell reports ENABLED on a build that contains + no CUTLASS. Evidence: `/mnt/nas_share/rc/reftier-1435/nvcc-check.sh` and + `/workspace/reftier-1435/nvcc-configure.log` on that worker. +- [#1482](https://github.com/mudler/vllm.cpp/issues/1482) carries the remainder + of [#844](https://github.com/mudler/vllm.cpp/issues/844), and this row owns it. + Two of #844's four "what done looks like" items are not done here: item 1 also + wants the refusal to name the BUILD FEATURE that would have provided the + kernel, and item 4 wants a warning at engine construction rather than only at + configure time. The pull request carries `Fixes #844`, because the defect in + that issue's title — the SIGSEGV, and the banner that claimed correctness — is + fixed, and leaving the issue open would misreport that. The alternative, + `Refs #844` with the issue left open, was rejected for that reason. Splitting + the remainder is what keeps it owned by an OPEN issue instead of a closed one. +- A CUDA re-run of `tests/vt/test_ops_matmul_fp8_block_cuda` on a CUTLASS-less + build, to record the refusal replacing exit 139. Owned by this row. + +## Now + +`ACTIVE`. The spec is committed before the implementation, in the same pull +request, so the commit order proves the order of work. The row is deliberately +not added to an area matrix: it is a bug fix to the `BACKEND-ACCEL-PROVIDER` +seam that already carries an `ACTIVE` matrix row, and adding a second row for +one predicate would put a shared-file edit in the path of a one-line fix. diff --git a/include/vt/backend.h b/include/vt/backend.h index 462a5d5bf..2dd019422 100644 --- a/include/vt/backend.h +++ b/include/vt/backend.h @@ -43,9 +43,10 @@ class Backend { // Drains any deferred submission WITHOUT a Queue in hand. Needed because the // portable CPU reference tier (op_provider.cpp) runs a HOST kernel directly - // over device memory on a unified-memory backend, and must not observe bytes - // a batched-but-uncommitted GPU submission has not written yet. Default no-op - // suits every backend that submits eagerly; Metal overrides it (M3c-1). + // over device memory on a backend that reports DeviceMemoryIsHostAddressable(), + // and must not observe bytes a batched-but-uncommitted GPU submission has not + // written yet. Default no-op suits every backend that submits eagerly; Metal + // overrides it (M3c-1). virtual void FlushPending() {} // True when host and device share one memory space (CPU, GB10, Apple). @@ -249,7 +250,8 @@ Backend& GetBackend(DeviceType type); // registered. `GetBackend` throws for the unregistered case, which forces every // "is this device present?" caller into a try/catch; this is the answer without // one. Used by the portable reference tier (op_provider.cpp) to read a device's -// UnifiedMemory() property without assuming the device exists in this build. +// DeviceMemoryIsHostAddressable() property without assuming the device exists in +// this build. Backend* TryGetBackend(DeviceType type); // Threading contract: all registration must complete before main() runs // (backends register via static initializers). After that, GetBackend is diff --git a/include/vt/op_provider.h b/include/vt/op_provider.h index 3446b465c..76983ab44 100644 --- a/include/vt/op_provider.h +++ b/include/vt/op_provider.h @@ -203,11 +203,19 @@ bool OpProviderDisabled(const char* name); // provider on a UNIFIED-MEMORY device, so an op the device lacks a native kernel // for falls back to the CPU reference instead of throwing. // -// SAFETY (the load-bearing invariant). A CPU kernel dereferences host pointers. -// That is correct ONLY where host and device memory alias (Metal StorageMode- -// Shared, GB10 / integrated Vulkan, CPU) — `Backend::UnifiedMemory()`. On a -// DISCRETE GPU a CPU kernel reading a device pointer is memory corruption, so the -// tier is gated on the unified-memory property, never on DeviceType blindly. +// SAFETY (the load-bearing invariant). A CPU kernel dereferences the pointers it +// is given. That is correct ONLY where the HOST MAY DEREFERENCE what +// `Backend::Alloc` returned — `Backend::DeviceMemoryIsHostAddressable()`. The +// tier is gated on that property, never on DeviceType blindly. +// +// IT IS NOT `Backend::UnifiedMemory()`, and the difference is two crashes +// (#844, #1435). Unified memory says host and device address the same physical +// RAM; it does NOT say a device allocation is host-dereferenceable. CUDA on GB10 +// reports unified memory and allocates with `cudaMalloc`, which the host may not +// touch, so gating on the wide property ran host kernels over device pointers +// and the process took SIGSEGV under a banner claiming a correct fallback. +// include/vt/backend.h carries the same distinction beside the narrow predicate, +// including why its default is `false`. // // DETERMINISM / no-change-on-native. The fallback registers at // kReferenceTierPriority (strictly below every native kernel's priority >= 0), so @@ -220,8 +228,9 @@ inline constexpr int kReferenceTierPriority = -1000; // strictly below any nati // THE SAFETY GATE. True iff `device` may host the CPU reference tier: it is not // the CPU source device itself, a backend is registered for it, and that backend -// reports UnifiedMemory() == true. Consulted at registration time; a device that -// answers false NEVER gets a CPU fallback installed. +// reports DeviceMemoryIsHostAddressable() == true. Consulted at registration +// time; a device that answers false NEVER gets a CPU fallback installed, and +// GetOp refuses it by name instead, naming which precondition it failed. bool ReferenceTierEligible(DeviceType device); // Eagerly install the reference tier for `target`: for every op that has a CPU diff --git a/src/vt/cuda/cuda_attention_cross.cu b/src/vt/cuda/cuda_attention_cross.cu index 04b26bee2..47b3a8bb1 100644 --- a/src/vt/cuda/cuda_attention_cross.cu +++ b/src/vt/cuda/cuda_attention_cross.cu @@ -4,14 +4,21 @@ // that kernel. // // WHY IT HAD TO BE WRITTEN NOW, and not deferred again. Before this TU the op had -// a CPU kernel only. On a DISCRETE CUDA device `GetOp` refuses, which is at least -// loud. On GB10 it is worse and quieter: `Backend::UnifiedMemory()` is true, so -// `RegisterReferenceTier` installs the CPU kernel for the accelerator and every -// cross-attention in the DiT would have executed on the HOST over unified memory -// — running, correct, and making "the forward ran on the GPU" false. LTX-2.5's -// block reaches this op six times per layer (two text cross-attentions, two -// audio<->video cross-attentions, and both self-attentions whenever a mask -// supplies a score bias), so that is most of the attention in the model. +// a CPU kernel only, and `GetOp` on a CUDA device would have installed the +// portable CPU tier: the gate read `Backend::UnifiedMemory()`, which GB10 +// reports true. Every cross-attention in the DiT would have left the GPU. +// LTX-2.5's block reaches this op six times per layer (two text +// cross-attentions, two audio<->video cross-attentions, and both self-attentions +// whenever a mask supplies a score bias), so that is most of the attention in +// the model. +// +// That gate now reads `Backend::DeviceMemoryIsHostAddressable()`, which CUDA +// answers false because it allocates with `cudaMalloc` (#844, #1435), so the +// same absence would be a named refusal today rather than a silent host run. +// The original note called the host run "correct", and it was not: the host +// kernel dereferences device pointers and the process gets SIGSEGV. Either way +// the conclusion is unchanged — this kernel had to exist — and the refusal makes +// the reason legible instead of leaving it to a crash. // // PORTED FROM, in structure: `AttentionDenseFlashKernel` // (src/vt/cuda/cuda_ops.cu:3229-3318), itself a 1:1 structural port of the diff --git a/src/vt/metal/metal_backend.mm b/src/vt/metal/metal_backend.mm index a2dac9d79..0c92b265c 100644 --- a/src/vt/metal/metal_backend.mm +++ b/src/vt/metal/metal_backend.mm @@ -112,6 +112,18 @@ Queue CreateQueue() override { bool UnifiedMemory() const override { return MetalContext::Get().unified_memory(); } + // Every buffer this backend allocates is MTLResourceStorageModeShared, whose + // `contents` pointer IS host memory the GPU also reads (src/vt/metal/ + // metal_buffers.h). So the answer is the same one UnifiedMemory() gives, and + // stating it separately is not redundant: `DeviceMemoryIsHostAddressable()` is + // the STRICTLY NARROWER question, it defaults to false, and it is what gates + // the portable CPU reference tier this backend depends on for the ops it has + // no native kernel for. Metal answered it only through the wide property until + // #844 / #1435 measured that the two differ on CUDA. + bool DeviceMemoryIsHostAddressable() const override { + return MetalContext::Get().unified_memory(); + } + int DeviceCapabilityMajor() const override { return MetalContext::Get().gpu_family_apple(); } diff --git a/src/vt/op_provider.cpp b/src/vt/op_provider.cpp index 2d476a8ec..8a79e12fb 100644 --- a/src/vt/op_provider.cpp +++ b/src/vt/op_provider.cpp @@ -224,6 +224,38 @@ bool MaybeInstallReferenceTier(OpId op, DeviceType device) { return true; } +// Why the portable CPU reference tier did not answer for (op, device). Called +// only on the refusal path, so it may take the slow, readable route. The clauses +// name the tier's preconditions in the order MaybeInstallReferenceTier and +// ReferenceTierEligible test them; the last one asks OpRegistered on the CPU +// slot rather than repeating that function's Choose call, which is the same +// question for the CPU device, where no reference provider ever exists. +std::string ReferenceTierRefusalReason(OpId op, DeviceType device) { + if (device == DeviceType::kCPU) { + return "the portable CPU reference tier is the SOURCE of that kernel, not a " + "fallback for it"; + } + Backend* b = TryGetBackend(device); + if (b == nullptr) { + return "no backend is registered for that device in this build, so the " + "portable CPU reference tier has nothing to install onto"; + } + if (!b->DeviceMemoryIsHostAddressable()) { + return std::string("the portable CPU reference tier is NOT eligible: this " + "backend does not report its device memory " + "host-addressable, so a host kernel may not dereference " + "what it allocated (unified memory is ") + + (b->UnifiedMemory() ? "true" : "false") + + ", which is a DIFFERENT property). Build a native kernel for this op " + "or run it on the CPU device"; + } + if (!OpRegistered(op, DeviceType::kCPU)) { + return "the portable CPU reference tier is eligible but has no CPU kernel " + "for this op to install"; + } + return "the portable CPU reference tier declined it"; +} + void Announce(OpId op, DeviceType device, Slot& slot, const OpProvider* chosen, const ProviderCaps& caps) { if (!AnnounceEnabled()) return; @@ -522,11 +554,15 @@ void* Resolve(OpId op, DeviceType device, Slot& slot) { slot.fallbacks.fetch_add(1, std::memory_order_relaxed); slot.resolved_none.store(true, std::memory_order_relaxed); // Refuse BY NAME. The integers stay for grep-ability, but a reader must not - // have to count enumerators in include/vt/ops.h to learn what was refused. + // have to count enumerators in include/vt/ops.h to learn what was refused — + // nor open this file to learn that a portable fallback exists and was + // withheld. The trailing clause says which of the tier's preconditions the + // device failed, which is the sentence #844 asked for. VT_CHECK(false, std::string("no kernel for op ") + OpNameImpl(op) + " (id " + std::to_string(static_cast(op)) + ") on device " + DeviceTypeName(device) + " (type " + - std::to_string(static_cast(device)) + ")"); + std::to_string(static_cast(device)) + "), and " + + ReferenceTierRefusalReason(op, device)); return nullptr; } // Reference-tier accounting: count it, and warn LOUDLY exactly once per @@ -535,9 +571,17 @@ void* Resolve(OpId op, DeviceType device, Slot& slot) { slot.ref_selected.store(true, std::memory_order_relaxed); RefTierHits().fetch_add(1, std::memory_order_relaxed); if (!slot.ref_announced.exchange(true, std::memory_order_relaxed)) { + // NOT "correct but slow". Those three words asserted a property instead of + // naming it, and #844 / docs/USAGE.md both quote them as what misled a + // reader past a SIGSEGV. What makes this dispatch valid is one checkable + // fact — the backend reports its device memory host-addressable — so the + // line states that fact and lets the reader verify it. std::fprintf(stderr, "[vt reference-tier] op=%s device=%s has NO native kernel; " - "running the PORTABLE CPU fallback (correct but slow)\n", + "running the PORTABLE CPU host kernel, which this backend " + "permits because it reports its device memory " + "host-addressable. It is SLOW: this run is not a performance " + "measurement\n", OpNameImpl(op), DeviceTypeName(device)); } } @@ -791,13 +835,34 @@ bool ReferenceTierEligible(DeviceType device) { // The CPU is the SOURCE of the reference kernels, never a fallback target // (falling back to itself is a no-op at best and self-reference at worst). if (device == DeviceType::kCPU) return false; - // THE SAFETY GATE. A CPU kernel dereferences host pointers, which is correct - // ONLY where host and device memory alias. Gate on the unified-memory property - // of the ACTUAL registered backend, not on DeviceType: a discrete GPU (CUDA or - // Vulkan) answers false and never receives a CPU fallback. A device with no - // backend in this build is trivially ineligible. + // THE SAFETY GATE. A CPU kernel dereferences pointers that came out of + // `Backend::Alloc`, so the question is whether the HOST MAY DEREFERENCE THEM + // — `DeviceMemoryIsHostAddressable()` — and not whether the two address spaces + // happen to sit on the same physical RAM. Gate on the property of the ACTUAL + // registered backend, not on DeviceType. A device with no backend in this + // build is trivially ineligible. + // + // THIS USED TO ASK `UnifiedMemory()`, AND THAT COST TWO CRASHES (#844, #1435). + // The two properties are not the same, and include/vt/backend.h says so beside + // the narrow one: CUDA on GB10 reports unified memory because host and device + // address the same physical RAM, yet a plain `cudaMalloc` pointer is still not + // host-dereferenceable. `CudaBackend::Alloc` calls exactly that, so every CUDA + // op without a native kernel installed the CPU host kernel over device + // pointers and the process took SIGSEGV — `vt::QuantFp8Static` on sm_110 + // (#960) and `vt::MatmulFp8BlockScaled` on a CUTLASS-less build (#1435). + // + // Metal (StorageModeShared) and ROCm (managed or pageable-access integrated + // allocations) answer the narrow predicate with the value they already report + // for unified memory, so neither changes. VULKAN WIDENS, and safely: it + // answers the narrow predicate `true` unconditionally, while its + // `UnifiedMemory()` is false when no HOST_VISIBLE|HOST_COHERENT|DEVICE_LOCAL + // memory type exists. On such a device the tier was WITHHELD before and is + // installed now. That is sound rather than lucky — `VulkanContext` falls back + // to the host flags alone and then VT_CHECKs that a host-visible, host-coherent + // type was found, so every allocation it hands out is host memory the GPU also + // reads. A backend added later gets the safe default, which is `false`. Backend* b = TryGetBackend(device); - return b != nullptr && b->UnifiedMemory(); + return b != nullptr && b->DeviceMemoryIsHostAddressable(); } int RegisterReferenceTier(DeviceType target) { diff --git a/src/vt/rocm/rocm_backend.hip b/src/vt/rocm/rocm_backend.hip index d2bb658eb..121aacf6d 100644 --- a/src/vt/rocm/rocm_backend.hip +++ b/src/vt/rocm/rocm_backend.hip @@ -355,6 +355,21 @@ class RocmBackend final : public Backend { bool UnifiedMemory() const override { return unified_memory_; } + // The SAME value, and deliberately not `managed_alloc_`. The registrar sets + // `unified_memory_ = managed_alloc || (pageable_memory_access && integrated)`, + // and BOTH branches give host code a pointer it may dereference: the managed + // branch by the hipMallocManaged API contract, the other by the + // PageableMemoryAccess attribute on an integrated device. So every allocation + // this backend hands out on a `unified_memory_` device is host-addressable. + // + // Answered explicitly because `DeviceMemoryIsHostAddressable()` defaults to + // false and is what gates the portable CPU reference tier (#844, #1435). That + // gate used to read `UnifiedMemory()`, which is a WIDER property — true on + // CUDA/GB10, where a cudaMalloc pointer is still not host-dereferenceable. + // Without this override ROCm would lose the tier that makes an APU run a model + // with one registered kernel. + bool DeviceMemoryIsHostAddressable() const override { return unified_memory_; } + bool DeviceMemoryInfo(size_t* free_bytes, size_t* total_bytes) const override { size_t free_b = 0, tot_b = 0; if (hipSetDevice(device_) != hipSuccess) return false; diff --git a/tests/vt/test_ops_mamba2_gated_norm.cpp b/tests/vt/test_ops_mamba2_gated_norm.cpp index a90ef15d2..a5ff46e59 100644 --- a/tests/vt/test_ops_mamba2_gated_norm.cpp +++ b/tests/vt/test_ops_mamba2_gated_norm.cpp @@ -581,14 +581,23 @@ Backend* MaybeCuda() { } // A GREEN TEST DOES NOT PROVE THE DEVICE RAN IT. GB10 is -// `integrated && pageable_memory_access`, so `Backend::UnifiedMemory()` is TRUE -// and `ReferenceTierEligible(kCUDA)` with it: absent a native kernel, `GetOp` -// does not throw — it installs the CPU HOST kernel as a `kReferenceProviderName` -// provider and runs THAT over the device pointers (op_provider.h, "portable +// `integrated && pageable_memory_access`, so `Backend::UnifiedMemory()` is TRUE, +// and the reference tier USED TO gate on that: absent a native kernel, `GetOp` +// did not throw — it installed the CPU HOST kernel as a `kReferenceProviderName` +// provider and ran THAT over the device pointers (op_provider.h, "portable // reference tier"), so every assertion below would pass while nothing ran on the // GPU. Every CUDA case therefore asserts the SELECTED provider is native. These // are EAGER dispatches, so the counters are populated // ([[graph-replay-does-no-host-dispatch-counters-read-zero]]). +// +// SINCE #844 / #1435 that specific false-green is CLOSED, and the guard stays. +// `ReferenceTierEligible` now reads `Backend::DeviceMemoryIsHostAddressable()`, +// which CUDA answers FALSE because `CudaBackend::Alloc` calls `cudaMalloc`, so a +// missing kernel is a named refusal rather than a silent host run. The paragraph +// above is kept as the REASON this guard exists, not as current behaviour. Do not +// delete the guard on the strength of the fix: it also catches a provider that +// DECLINES at run time, and a future backend that answers the narrow predicate +// true would restore the original hazard exactly. void RequireNativeCudaProvider(vt::OpId op, const std::string& what) { const vt::OpProviderStats st = vt::GetOpProviderStats(op, DeviceType::kCUDA); INFO(what << ": selected CUDA provider = " diff --git a/tests/vt/test_ops_mamba2_ssd.cpp b/tests/vt/test_ops_mamba2_ssd.cpp index 37add73fd..f4c04fbc1 100644 --- a/tests/vt/test_ops_mamba2_ssd.cpp +++ b/tests/vt/test_ops_mamba2_ssd.cpp @@ -1065,16 +1065,25 @@ Backend* MaybeCuda() { } // A GREEN TEST DOES NOT PROVE THE DEVICE RAN IT — and on THIS box it very nearly -// proves the opposite. GB10 is `integrated && pageable_memory_access`, so -// `Backend::UnifiedMemory()` is TRUE (cuda_backend.cu Registrar) and therefore -// `ReferenceTierEligible(kCUDA)` is TRUE. Absent a native kernel, `GetOp` does -// not throw: it installs the CPU HOST kernel as a `kReferenceProviderName` -// provider and runs THAT over the device pointers (op_provider.h, "portable -// reference tier"). Every numeric assertion below would then pass — the device -// arm would be gated by running the host arm twice, the exact false-green shape -// of [[absent-hook-looks-like-armed-instrument]] and +// proved the opposite. GB10 is `integrated && pageable_memory_access`, so +// `Backend::UnifiedMemory()` is TRUE (cuda_backend.cu Registrar), and the +// reference tier USED TO gate on that: absent a native kernel `GetOp` did not +// throw, it installed the CPU HOST kernel as a `kReferenceProviderName` provider +// and ran THAT over the device pointers (op_provider.h, "portable reference +// tier"). Every numeric assertion below would then pass — the device arm gated by +// running the host arm twice, the exact false-green shape of +// [[absent-hook-looks-like-armed-instrument]] and // [[gate-comparing-shared-helper-proves-consistency-not-correctness]]. // +// SINCE #844 / #1435 that specific false-green is CLOSED, and the guard stays. +// `ReferenceTierEligible` now reads `Backend::DeviceMemoryIsHostAddressable()`, +// which CUDA answers FALSE because `CudaBackend::Alloc` calls `cudaMalloc`, so a +// missing kernel is a named refusal rather than a silent host run. The paragraph +// above is kept as the REASON this guard exists, not as current behaviour. Do not +// delete the guard on the strength of the fix: it also catches a provider that +// DECLINES at run time, and a future backend that answers the narrow predicate +// true would restore the original hazard exactly. +// // So every CUDA case asserts the SELECTED provider is native. These are EAGER // dispatches rather than a captured graph, so the counters are genuinely // populated ([[graph-replay-does-no-host-dispatch-counters-read-zero]]). diff --git a/tests/vt/test_ops_mamba2_state_update.cpp b/tests/vt/test_ops_mamba2_state_update.cpp index d4e6f9bf1..1ed798c0c 100644 --- a/tests/vt/test_ops_mamba2_state_update.cpp +++ b/tests/vt/test_ops_mamba2_state_update.cpp @@ -759,14 +759,23 @@ Backend* MaybeCuda() { } // A GREEN TEST DOES NOT PROVE THE DEVICE RAN IT. GB10 is -// `integrated && pageable_memory_access`, so `Backend::UnifiedMemory()` is TRUE -// and `ReferenceTierEligible(kCUDA)` with it: absent a native kernel, `GetOp` -// does not throw — it installs the CPU HOST kernel as a `kReferenceProviderName` -// provider and runs THAT over the device pointers (op_provider.h, "portable +// `integrated && pageable_memory_access`, so `Backend::UnifiedMemory()` is TRUE, +// and the reference tier USED TO gate on that: absent a native kernel, `GetOp` +// did not throw — it installed the CPU HOST kernel as a `kReferenceProviderName` +// provider and ran THAT over the device pointers (op_provider.h, "portable // reference tier"), so every assertion below would pass while nothing ran on the // GPU. Every CUDA case therefore asserts the SELECTED provider is native. These // are EAGER dispatches, so the counters are populated // ([[graph-replay-does-no-host-dispatch-counters-read-zero]]). +// +// SINCE #844 / #1435 that specific false-green is CLOSED, and the guard stays. +// `ReferenceTierEligible` now reads `Backend::DeviceMemoryIsHostAddressable()`, +// which CUDA answers FALSE because `CudaBackend::Alloc` calls `cudaMalloc`, so a +// missing kernel is a named refusal rather than a silent host run. The paragraph +// above is kept as the REASON this guard exists, not as current behaviour. Do not +// delete the guard on the strength of the fix: it also catches a provider that +// DECLINES at run time, and a future backend that answers the narrow predicate +// true would restore the original hazard exactly. void RequireNativeCudaProvider(vt::OpId op, const std::string& what) { const vt::OpProviderStats st = vt::GetOpProviderStats(op, DeviceType::kCUDA); INFO(what << ": selected CUDA provider = " diff --git a/tests/vt/test_reference_tier.cpp b/tests/vt/test_reference_tier.cpp index 701626fe0..e440452da 100644 --- a/tests/vt/test_reference_tier.cpp +++ b/tests/vt/test_reference_tier.cpp @@ -6,16 +6,21 @@ // three properties the tier must have, WITHOUT needing Metal or Vulkan hardware: // // 1. SAFETY (the load-bearing invariant) — the CPU fallback is installed ONLY -// where host and device memory alias (Backend::UnifiedMemory()). A device -// that reports DISCRETE memory NEVER receives a CPU fallback; GetOp still -// throws there, exactly as before, because a CPU kernel against true device -// memory is corruption. This is asserted against a fake DISCRETE backend. +// where the HOST MAY DEREFERENCE what Backend::Alloc returned +// (Backend::DeviceMemoryIsHostAddressable()). A device that cannot promise +// that NEVER receives a CPU fallback; GetOp refuses by name instead, +// because a CPU kernel against device memory is corruption. Asserted twice: +// against a fake DISCRETE backend, and against a fake backend carrying the +// GB10 CUDA pair — unified memory TRUE, host-addressable FALSE. The gate +// read UnifiedMemory() until #844 / #1435, and the second fake is the shape +// that made that a SIGSEGV rather than a refusal. // // 2. CORRECTNESS-WITH-ZERO-KERNELS — a device with NO native kernel for an op // still produces the right answer through the reference tier. Asserted -// against a fake UNIFIED backend (host-memory allocator standing in for -// Metal StorageModeShared / GB10 / integrated Vulkan): vt::Relu dispatched on -// that device falls back to the CPU kernel and returns bit-identical output. +// against a fake UNIFIED, host-addressable backend (host-memory allocator +// standing in for Metal StorageModeShared and integrated Vulkan): vt::Relu +// dispatched on that device falls back to the CPU kernel and returns +// bit-identical output. // // 3. NATIVE ALWAYS WINS + OBSERVABILITY — a registered native kernel outranks // the tier (priority), the tier is never silent (GetReferenceTierHits, and a @@ -23,12 +28,15 @@ // so the fused-recipe ladder is unchanged. // // This file is its own executable (tests/CMakeLists.txt: one add_executable per -// test), so registering a backend on the otherwise-unused kXPU slot cannot leak -// into any other test binary. +// test), so registering a backend on the otherwise-unused kXPU and kTENSTORRENT +// slots cannot leak into any other test binary. The two slots are separate +// because a reference-tier provider cannot be uninstalled once registered, so a +// case that must observe an EMPTY table needs a table nothing else has touched. #include #include #include +#include #include #include @@ -53,7 +61,15 @@ using vt::Tensor; // host allocator so a fallback kernel dispatched here actually runs. class FakeBackend final : public Backend { public: - explicit FakeBackend(bool unified) : unified_(unified) {} + // `unified` and `host_addressable` are the two properties under test, and they + // are SEPARATE because a real backend separates them. `UnifiedMemory()` says + // host and device address the same physical RAM; `DeviceMemoryIsHostAddressable()` + // says a pointer from `Alloc()` may be dereferenced by the host. CUDA on GB10 + // answers true then false (include/vt/backend.h), which is the pair the third + // instance below carries. Everything else is a plain host allocator so a + // fallback kernel dispatched here actually runs. + FakeBackend(bool unified, bool host_addressable) + : unified_(unified), host_addressable_(host_addressable) {} void* Alloc(size_t bytes) override { return std::malloc(bytes == 0 ? 1 : bytes); } void Free(void* p) override { std::free(p); } void Memset(Queue&, void* p, int v, size_t bytes) override { std::memset(p, v, bytes); } @@ -62,17 +78,25 @@ class FakeBackend final : public Backend { } Queue CreateQueue() override { return Queue{Device{DeviceType::kXPU, 0}, nullptr}; } bool UnifiedMemory() const override { return unified_; } + bool DeviceMemoryIsHostAddressable() const override { return host_addressable_; } private: bool unified_; + bool host_addressable_; }; FakeBackend& Unified() { - static FakeBackend b(true); + static FakeBackend b(true, true); return b; } FakeBackend& Discrete() { - static FakeBackend b(false); + static FakeBackend b(false, false); + return b; +} +// The GB10 CUDA shape, expressed without a GPU: host and device share the RAM, +// and a `cudaMalloc` pointer is still not host-dereferenceable. +FakeBackend& UnifiedNotHostAddressable() { + static FakeBackend b(true, false); return b; } @@ -103,6 +127,80 @@ TEST_CASE("reference tier: the CPU source device is never eligible") { CHECK_FALSE(vt::ReferenceTierEligible(DeviceType::kCPU)); } +// --------------------------------------------------------------------------- +// 1b. THE SAFETY GATE IS HOST-ADDRESSABILITY, NOT UNIFIED MEMORY (#844, #1435). +// +// This is the shape that crashed twice on GB10. `Backend::UnifiedMemory()` is +// true there, because host and device address the same physical RAM. It is NOT +// the property a host kernel needs: `CudaBackend::Alloc` calls `cudaMalloc`, and +// `include/vt/backend.h` records beside `DeviceMemoryIsHostAddressable()` that +// such a pointer "is still not host-dereferenceable". Gating the tier on the +// wide property handed the CPU kernel device pointers, and the process took +// SIGSEGV under a banner reading "correct but slow" -- `vt::QuantFp8Static` on +// sm_110 (#960) and `vt::MatmulFp8BlockScaled` on a CUTLASS-less build (#1435, +// exit 139 with `test cases: 0 assertions: 0`). +// +// A SEPARATE device slot from the cases around it, deliberately: the tier cannot +// be uninstalled once a provider is registered, so sharing kXPU with the unified +// case below would make this assertion depend on doctest's execution order. +// +// AND THAT SLOT IS SKIPPED, NOT ASSERTED EMPTY. kTENSTORRENT is not an unused +// enumerator: src/vt/tenstorrent/tenstorrent_ops.cpp registers 21 op sites on it, +// `OpId::kRelu` among them, and `vllm` carries an INTERFACE `--whole-archive` +// (CMakeLists.txt), so on a `-DVLLM_CPP_TENSTORRENT=ON` build that registrar runs +// inside THIS binary. An earlier draft REQUIREd the table empty and called that +// failing loudly. It is not: the failure is unconditional on a legitimate +// configuration nobody's CI builds, so it would land as a permanent red for +// whoever next turns that backend on, and RegisterBackend would displace a live +// backend in-process on the way. Skipping is the honest answer, because the +// property under test needs a slot no other registrar has claimed and this build +// then has none. +// --------------------------------------------------------------------------- +TEST_CASE("reference tier: unified memory the host cannot address is refused, by name") { + constexpr DeviceType kSlot = DeviceType::kTENSTORRENT; + if (vt::TryGetBackend(kSlot) != nullptr || + vt::OpProviderCount(OpId::kRelu, kSlot) != 0) { + MESSAGE("SKIP: DeviceType::kTENSTORRENT carries a backend or providers in " + "this build, so it is not a free slot; this case needs one no other " + "registrar has claimed"); + return; + } + vt::RegisterBackend(kSlot, &UnifiedNotHostAddressable()); + + // The property pair under test: the OLD gate says yes, the right one says no. + Backend& dev = vt::GetBackend(kSlot); + REQUIRE(dev.UnifiedMemory()); + REQUIRE_FALSE(dev.DeviceMemoryIsHostAddressable()); + + // THE INVARIANT. A host kernel is never installed for a device whose memory + // the host cannot dereference. + CHECK_FALSE(vt::ReferenceTierEligible(kSlot)); + CHECK(vt::RegisterReferenceTier(kSlot) == 0); + CHECK(vt::OpProviderCount(OpId::kRelu, kSlot) == 0); + + // And the dispatch REFUSES rather than returning the host kernel. Comparing + // against the CPU fn pointer is what separates a refusal from a fallback: the + // tier installs `src->fn`, the very same pointer, so an equal answer here is + // the defect even when nothing throws. + REQUIRE(vt::OpRegistered(OpId::kRelu, DeviceType::kCPU)); + void* thrown_fn = nullptr; + std::string message; + try { + thrown_fn = vt::GetOp(OpId::kRelu, kSlot); + } catch (const std::exception& e) { + message = e.what(); + } + CHECK(thrown_fn == nullptr); + CHECK(thrown_fn != vt::GetOp(OpId::kRelu, DeviceType::kCPU)); + + // Refused BY NAME: the op, the device, and why the portable tier did not run. + // A reader who sees this must not have to open op_provider.cpp to learn that a + // fallback exists and was withheld. + CHECK(message.find("Relu") != std::string::npos); + CHECK(message.find("tenstorrent") != std::string::npos); + CHECK(message.find("host-addressable") != std::string::npos); +} + // --------------------------------------------------------------------------- // 2. CORRECTNESS WITH ZERO NATIVE KERNELS — a unified device runs via the tier. // ---------------------------------------------------------------------------