Pull requests / #2106

#2106 sycl verify: complete STRATA_VERIFY_EAGER for the commit path

open · @demetree · 0 commentaires · Sur GitHub

BenchmarksServer & APIAMD / HIPNVIDIA / CUDAModels & quantsWindowsLinux

Description

Completes the `STRATA_VERIFY_EAGER` path for the verify window's commit.

## The gap

The eager path is already half-built for the window: `capture()` returns early under `STRATA_VERIFY_EAGER`, `run()` replays the body through `record_window`, the stage table branches on the flag, and the stepped-mode check excludes it. `prepare_chain()` in `mtp.cpp` documents the state of the rest out loud:

```
err = "mtp: chains need command graphs (STRATA_VERIFY_EAGER=1 has none)";
```

The commit is the missing half: `capture_commit()` records a command graph and `commit()` launches it, so a backend with no graph extension cannot get through a window's commit at all. That is not a hypothetical - the OpenCL UR adapter has no command-graph backend, which is why `run()` already carries the window replay.

## The change

`capture_commit`'s body is now `Verifier::record_commit`, and the two callers are:

```cpp
capture_commit()   records it between begin_recording/end_recording and finalizes commit_exec_,
                   or answers "captured" under eager
commit()           STRATA_VERIFY_EAGER ? record_commit() : ext_oneapi_graph(*commit_exec_)
```

The body refreshes everything it reads - the mapped `commit_` upload first, then the layer snapshots - so a replay sees the same inputs the graph would have, and it is safe to run once per window. `h_commit_` is written by `commit()` before the launch, exactly as before.

## What it leaves alone

The graph path records the same kernels in the same order and finalizes them the same way - only the body moved into a function the eager path can call. `record_window`, the MTP drafter's replays and the one-token self-commit are untouched.

## On "is this a bug?"

On `main` today the same state is refused, not broken: `run()` calls `if (!capture(T, err) || !capture_commit(err)) return false;`, so `capture_commit()` fails at `begin_recording()` and `run()` bails with "verify: begin commit capture failed". I hit the null-`commit_exec_` case only after a Windows/OpenCL port of mine had started answering "captured" without building a graph. So this is a **completion, not a fix for a crash reachable on main** - and I would rather say that plainly than dress it up as a crash report.

## Measurements

Arc Pro B70, 32 GB, Windows 11 24H2, engine 0.1.43, coder-IQ1_M, 2047-token prompt, `--spec 4 --spec-min-p 0.5`, `--max-context 131072`, `--kv int8 --kv-resident 32768`, `--stream-experts --vram-reserve-mib 2048`, greedy, with `STRATA_VERIFY_DEVICE_PLAN=1 STRATA_VERIFY_NO_HOST=1 STRATA_VERIFY_EAGER=1 STRATA_MIRROR_MIB=4096`:

```
with the commit replay:  exit 0, prefill 2047 tokens in 25252 ms -> 81.1 tok/s
                         decode 128 tokens in 1921 ms -> 66.6 tok/s
                         speculation 3.91 tokens per round (33 rounds of 6, 96/96 drafts)
through serve:          220 tokens in 3679 ms -> 59.8 tok/s, 166/168 drafts accepted
```

## What I could not verify

- **Built and run on SYCL/Windows only.** I have no CUDA, HIP or Linux toolchain here, so I cannot show the graph path is untouched on those, although the change sits inside a branch that is false unless `STRATA_VERIFY_EAGER` is set.
- **This tree's SYCL port does not compile on Windows as it stands.** A clean checkout of the release I based this on fails twice on Windows (`pinned.dp.cpp` uses the CUDA runtime API under an unguarded `#ifdef _WIN32`; `verify_kernels.dp.cpp` hits the `OUT` macro). My numbers come from a tree that also carries those repairs. If you want a patch that builds standalone here, that is a separate change.

## Suggested review order

1. `capture_commit()` - is answering "captured" under eager the right contract, and does it belong here rather than in the caller?
2. `record_commit()` - the body refresh is unchanged, but it is now called per-window instead of once at capture.
3. `commit()` - the `else` I added around the existing graph launch.

Sur le site

Liens install, modèles, releases.