Bring the user-facing docs up to v0.6.18 by demandal25 · Pull Request #357 · AMD-Ecosystem/flashinfer

demandal25 · 2026-09-03T15:55:10Z

Summary

Brings the user-facing documentation up to v0.6.18 for the GitHub release, narrows the support statement to the one configuration docker/Dockerfile.rocm builds, and fixes the places where the docs described behaviour the code did not have.

What changed

Install story

pip install amd-flashinfer --index-url https://pypi.amd.com/simple/ has never worked. That index lists 20 projects and amd-flashinfer is not among them — /simple/amd-flashinfer/ returns HTTP 403, and the one FlashInfer artifact published there is flashinfer-0.2.5.post10+rocm, under the project name flashinfer. amd-flashinfer-jit-cache is not published either. Quick start is now build-from-source. The Docker Hub row is gone: those tags exist but are 0.5.3-era and nothing in this repo produces them. The dead CHANGELOG.md link (deleted by the v0.6.18 sync) now points at Releases.

One supported configuration

ROCm 10.0, PyTorch 2.12.0, Python 3.12, Ubuntu 24.04, amd-aiter 0.1.20 — what docker/Dockerfile.rocm builds. The old table advertised five ROCm and three torch versions as "Supported", which nothing backed, and the pins are not independent anyway (0.1.20 is cp312-only; torch must stay below 2.13 for a c10 symbol AITER's prebuilt kernels need; repo.radeon.com publishes no rocm-rel- directory for 10.0 at all, which is why the supported configuration is an image rather than a version list).

Consequently docker/Dockerfile.rocm_ci is deleted — it was the last thing pinning ROCm 7.1.1 / torch 2.8.0 / amd-aiter 0.1.10, the sub-floor pin that made backends.md admit CI exercised no AITER path. No workflow in this repo built it; the out-of-tree CI entrypoint that invokes it by name needs the same removal. Every other ROCm 7.x reference is gone too, including the KnownBad row for the gfx950 ROCm 7.2 causal miscompile and the stored notebook outputs from a torch 2.9.1+rocm7.2.0 run.

`-n auto` was CPU-derived outside `tests/rocm`

conftest.py (new, repo root) — holds both the xdist worker count and the per-worker GPU pinning.

xdist resolves -n auto from pytest_cmdline_main, when only the initial args' conftests are loaded. testpaths is expanded before that, so a bare pytest -n auto picked up tests/rocm/conftest.py; any explicit path outside it did not:

tests/rocm/test_arch_caps.py       ->   4 workers
tests/utils/test_quantization.py   -> 192 workers    <-- CPU count

tests/utils/test_quantization.py is a configured testpath, so this was reachable from a documented command. Moving both halves to a root conftest fixes it, extends the same treatment to benchmarks/, and shrinks this fork's diff against upstream's tests/conftest.py.

Support matrix

One row per (op, backend) pair, so the Backend column now also says whether auto takes that row. The ✅/◻️ split is gone — it rendered VALIDATED if entry.evidence else DECLARED, i.e. whether someone had pasted a measurement string, which read as a support tier and was not one.

cascade was wrong, not just ambiguous. Its row says hip, true of the merge kernels, but every cascade wrapper builds BatchPrefillWithPagedKVCacheWrapper internally at backend="auto" (cascade.py:350,374,674,939). A two-level plan() at page_size 128 resolves both levels to aiter. No cascade wrapper takes a backend=, and cascade.py is upstream, so this is documented rather than given a new parameter.

Other

pyproject.toml, flashinfer/rocm/hip_utils.py — package metadata named github.com/rocm/flashinfer, a real but different repository, which is what a wheel's METADATA shows.
docs/rocm/backends.md — a "Not available on ROCm" table built from CUDA_ONLY_MODULES, stating explicitly that gating flashinfer.fused_moe does not mean no MoE; per-op notes for the architecture-dependent MoE fp8 encoding and both fp8 shape constraints, MLA rejecting use_cuda_graph/return_lse, and decode rejecting q_len_per_req > 1.
MANIFEST.in — excludes the new root conftest.py, which setuptools-scm would otherwise ship in the sdist.

Architecture / design notes

No upstream-pristine file is edited, confirmed against upstream-base/v0.6.18 (69ff11fc4). That is why the CUDA-only surface is documented once in the fork-owned backends.md rather than by annotating 30 upstream .rst files, and why cascade's routing is documented rather than fixed with a new parameter.

Test plan

Notes for the release

v0.6.18+amd.1 currently points at 76132f779 (#348), behind this branch. The plan is to force-move it to the post-merge commit so the tag contains #349 onwards and this change; anyone who has fetched that tag will need to re-fetch.

`pytest -n auto` is meant to spawn half as many workers as physical AMD cards. It only did so when the invocation named `tests/rocm`. xdist resolves `-n auto` from `pytest_cmdline_main`, by which point pytest has loaded conftests only for the *initial* args. `testpaths` is expanded before that (pytest 9.1.1, `Config._decide_args`), so a bare `pytest -n auto` did pick up `tests/rocm/conftest.py` and behaved. Any explicit path outside that directory did not, and fell through to xdist's CPU-count default. Measured on an 8-card gfx950 host, pytest 9.1.1 / pytest-xdist 3.8.0: tests/rocm/test_arch_caps.py -> 4 workers tests/utils/test_quantization.py -> 192 workers <-- CPU count `tests/utils/test_quantization.py` is one of the configured `testpaths`, so this was reachable from a documented command. 192 workers on 8 cards is 24 processes per card. Both halves of the mechanism now live in a root `conftest.py`, which is loaded for every invocation. Keeping the count and the pinning in separate conftests was the worse split, and moving the pinning out of `tests/conftest.py` shrinks this fork's diff against that upstream file. `benchmarks/` gains the same treatment, having had none before. After: tests/rocm/test_arch_caps.py -> 4 workers tests/utils/test_quantization.py -> 4 workers benchmarks/test_flashinfer_benchmark.py -> 4 workers Also notes that the hook is `firstresult` and therefore shadows xdist's `PYTEST_XDIST_AUTO_NUM_WORKERS`, which is otherwise a dead escape hatch.

`[project.urls]` named `github.com/rocm/flashinfer` as Homepage, Repository and Bug Tracker. That is a real but different repository, and it is what a wheel's METADATA and any index page display -- so a release would ship users to the wrong issue tracker. `hip_utils.py`'s CPU-only-torch error message pointed at the same place, and suggested torch 2.7.1 / ROCm 6.4, neither of which is in the supported table any more. Also drops `exclude CHANGELOG.md` from MANIFEST.in; the file was deleted by the v0.6.18 sync.

The docs still described the v0.5.3 era. Three claims were not merely stale but wrong: - `pip install amd-flashinfer --index-url https://pypi.amd.com/simple/` cannot work. That index lists 20 projects and amd-flashinfer is not among them (/simple/amd-flashinfer/ -> HTTP 403); the only FlashInfer artifact published there is flashinfer-0.2.5.post10+rocm. - README linked CHANGELOG.md for the release history. The v0.6.18 sync (de79634) deleted that file. - The Docker Hub row advertised flashinfer-0.5.3.amd1_rocm7.2_*. Those tags exist but are 0.5.3-era, and nothing in this repo produces them. Quick start is now build-from-source: build the development image from docker/Dockerfile.rocm, then the editable install, delegating the full recipe to CONTRIBUTING.md rather than duplicating commands that would drift. "the devcontainer" is no longer cited as a ROCm environment -- .devcontainer/ holds only cu129 and cu130. The `amd-aiter >= 0.1.10` page-size qualifier appeared in four places while AITER_MIN_VERSION has been 0.1.20 for some time, so every supported install clears it. One of the four was inside README's generated block, so it is fixed at its source in arch_caps.py and the block regenerated. Also in this change: - backends.md gains a "Not available on ROCm" table built from CUDA_ONLY_MODULES, replacing the idea of annotating 30 upstream docs/api/*.rst files -- which the additive-only policy rules out. It says explicitly that gating `flashinfer.fused_moe` does not mean no MoE, and that find_spec still reports these modules present. - Per-op notes for what the code enforces but the docs did not: the architecture-dependent MoE fp8 encoding (e4m3fnuz on gfx942, e4m3fn on gfx950; the wrong one is silently wrong), MLA rejecting use_cuda_graph and return_lse, and decode rejecting q_len_per_req > 1. - The Benchmarking section is un-commented; benchmarks/rocm/ and the testlist have existed for a while. Adds the --refcheck the prose already credited. - README documents the v0.6.18 surface: block-sparse, POD, cascade, sampling, fp8 MoE, and the max_seq_len graph-capture hint. - Env-var table gains FLASHINFER_EXTRA_CFLAGS/CUDAFLAGS, FLASHINFER_DISABLE_AOT_ARCH_CHECK, AITER_JIT_DIR and GPU_ARCHS. The build-time pointer moves from CLAUDE.md (agent instructions) to CONTRIBUTING.md, which now carries that table and the per-arch jit-cache wheel build. - A "Running the tests" note stating that `-n auto` is derived from the GPU count, not the CPU count: half the physical cards, minimum one, so a single-GPU host runs one worker. - The tutorial notebook's setup cell (dead install command, Docker Hub claim, placeholder image tag), its page-size claim, and the RuntimeError string pointing at a README section that does not exist.

'a few minutes' understated it; docs/rocm/backends.md already budgets 20+ minutes for a cold AITER variant, and a single-prefill fp16 variant took 62.9s to build during wheel verification.

Self-review of the changelist before push. The first item is a real break introduced by the conftest move earlier in this branch. test_worker_pinning.py loads the conftest by path via parents[1], which pointed at tests/conftest.py -- where _worker_gpu_index no longer lives. The full suite on gfx942 (MI300X, 8 cards) confirmed it: 7 failed, 32913 passed, 3892 skipped, and all 7 failures were this file. parents[2] now names the root conftest; the same 7 pass in 0.96s. The worker-count hook raised RuntimeError when rocminfo reported no supported GPU. That was survivable while it only ran for tests/rocm; at the root it makes a GPU-free checkout unable to run even the GPU-free tests, and it contradicted the pinning block 30 lines below, which degrades to torch.cuda.device_count(). Both halves now degrade the same way. Documentation claims that did not survive checking against the code: - README and the notebook told the reader to `docker run` and then `pip install -ve .`, with no bind mount. Dockerfile.rocm never COPYs the repo, so that installs from an empty home directory. Both now carry -v "$PWD":/workspace and say why. - The --refcheck added to the benchmark command is a no-op: in testlist mode flashinfer_benchmark.py parses only --testlist/--output_path and discards the rest, and every testlist line already carries its own --refcheck. Removed, and the prose now says where it comes from. - "the wrong fp8 encoding is silently wrong, not an error" is false -- fused_moe.py:289 raises ValueError. The silent-corruption risk is what the guard exists to prevent, not what a user experiences. - FLASHINFER_DISABLE_AOT_ARCH_CHECK read inverted: "a mismatch is ignored" describes the variable being set, not unset. - GPU_ARCHS does not simply defer to an operator value; a shim build overwrites it from FLASHINFER_ROCM_ARCH_LIST and does not restore it. - The notebook still sourced amd-aiter from "AMD's PyPI", which backends.md documents as not carrying it above the 0.1.20 floor. - Two stale cross-references to tests/conftest.py.

Copilot

🟡 Changes recommended

The root pytest hook breaks CPU-only runs, and the notebook setup lacks required render-group access.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates ROCm documentation for v0.6.18 and makes pytest-xdist worker selection GPU-derived repository-wide.

Changes:

Centralizes xdist worker counting and GPU mapping.
Refreshes installation, backend, testing, and benchmarking guidance.
Corrects project metadata, compatibility guidance, and AITER notes.

File summaries

File	Review
`tests/rocm/test_worker_pinning.py`	Updates tests to load the root conftest.
`tests/rocm/conftest.py`	Removes relocated worker-count logic.
`tests/conftest.py`	Removes relocated GPU mapping logic.
`README.md`	Updates v0.6.18 guidance. Nit (1 vote): GPU pinning wording overstates worker isolation.
`pyproject.toml`	Corrects project URLs.
`MANIFEST.in`	Removes the obsolete changelog exclusion.
`flashinfer/rocm/hip_utils.py`	Updates installation guidance. Nit (1 vote): Version-mismatch warnings still recommend unsupported PyTorch 2.7.1.
`flashinfer/rocm/arch_caps.py`	Refreshes AITER paging notes.
`examples/run_jupyter_server.sh`	References the current development image.
`examples/amd_flashinfer_rocm_tutorial.ipynb`	Replaces obsolete setup instructions. Moderate (1 vote): Docker command omits the required host render-group GID.
`docs/rocm/backends.md`	Documents backend limitations. Nit (2 votes): Incorrectly suggests fp8 activations are supported by `aiter_fused_moe`.
`CONTRIBUTING.md`	Expands build and testing documentation. Nit (2 votes): Coverage recipe contradicts the authoritative `upstream-base` scheme.
`conftest.py`	Adds repository-wide xdist configuration. Critical (1 vote): CPU-only PyTorch fails during FlashInfer import before the zero-GPU fallback. Nit (1 vote): Worker-count and fallback branches lack automated coverage.
`benchmarks/rocm/testlist_rocm.txt`	Updates paging guidance.

Review details

Suppressed comments (4)

README.md:229

“Each worker is pinned” is stronger than the implementation: the new root conftest explicitly notes that importing flashinfer initializes HIP before HIP_VISIBLE_DEVICES is changed, so only child processes spawned afterward are scoped to that card. Clarify this distinction so users do not assume tests running in the xdist worker see only one GPU.

worker. Each worker is pinned to one card. Pass an explicit `-n N` to

conftest.py:82

The worker-side path has the same CPU-only failure independently of the auto-count hook: every xdist worker enters this branch and importing the FlashInfer submodule executes the package's CPU-only RuntimeError. Skip AMD pinning when PyTorch is absent or is not a ROCm build so GPU-free xdist workers can start.

if _xdist_worker.startswith("gw"):
    import torch

    from flashinfer.rocm.hip_utils import get_physical_card_device_indices

conftest.py:64

The central new behavior is not covered by the moved worker tests: test_worker_pinning.py exercises only _worker_gpu_index, not this hook's physical-card count, torch fallback, or zero-GPU minimum. Add automated cases for those branches so a future conftest move or fallback change cannot silently restore CPU-derived worker counts or break GPU-free test runs.

        n_physical = len(get_physical_card_device_indices()) or (
            torch.cuda.device_count() if torch.cuda.is_available() else 0
        )
        return max(1, n_physical // 2)

flashinfer/rocm/hip_utils.py:727

Only the CPU-only error was updated: the version-mismatch warning later in this same function still tells users to install unsupported torch==2.7.1 (lines 749 and 753), while the documented v0.6.18 matrix starts at 2.8. Users who already have ROCm PyTorch but hit a runtime-version mismatch therefore still receive stale remediation. Update both warning commands, preferably by directing users to the supported matrix because the correct wheel depends on the ROCm release.

            "     pip install torch==2.9.1 -f https://repo.radeon.com/rocm/manylinux/rocm-rel-7.2/\n\n"
            "See https://github.com/AMD-Ecosystem/flashinfer for detailed installation instructions.\n"

Files reviewed: 14/14 changed files
Comments generated: 4
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Accepted: - CONTRIBUTING's coverage paragraph described the pre-#348 scheme. The base is the recorded upstream-base sha, not a lookup from this fork's +amd. tag; _resolve_base_detail prefers it precisely because a squash-merged sync leaves no merge parent, and merge-base would walk to the previous fork point (2468 files owned instead of 290). The earlier commit only swapped the version in the stale sentence. - backends.md read as if aiter_fused_moe accepts fp8 activations. It does not -- fused_moe.py:272 rejects any hidden_states outside fp16/bf16; only the weights may be fp8. Activation and weight dtype requirements are now separate bullets. - hip_utils.py: only the CPU-only error was updated, leaving the version-mismatch warning recommending torch 2.7.1 at two more sites. Hard-coding any version is what made this stale, since the right one depends on the ROCm release, so both now point at the matrix. - The notebook's docker run omitted the host render GID that the README and CONTRIBUTING both carry; without it the container cannot open /dev/dri/renderD* and reports "No CUDA GPUs are available". - "Each worker is pinned to one card" overstated it: HIP is initialized by the time HIP_VISIBLE_DEVICES is set, so it scopes the subprocesses a test spawns, not the worker. - The worker-count hook had no test -- the gap that let the conftest move break test_worker_pinning silently. Adds coverage for the halving, the one-card floor, the torch fallback and where the hook lives. A/B: reverting the fallback fails test_worker_count_falls_back_to_torch and nothing else. Declined: the CPU-only-torch import guard, both inline and suppressed. The premise is real -- importing flashinfer raises on a CPU-only wheel -- but it is pre-existing, not introduced here. On the base commit both conftests already imported flashinfer.rocm.hip_utils at module scope: $ git show origin/amd-integration:tests/rocm/conftest.py | sed -n 18,23p from flashinfer.rocm.arch_caps import normalize_arch from flashinfer.rocm.hip_utils import ( get_physical_card_device_indices, so `pytest -n auto` failed identically before this PR. The RuntimeError is check_torch_rocm_compatibility's deliberate, actionable message, and suppressing it to run a GPU test suite on CPU-only torch trades a clear error for an obscure one. The GPU-free CI lane passes --noconftest.

The commit that answered Copilot introduced or left these. MANIFEST.in did not exclude the new root conftest.py. setuptools-scm's finder adds every tracked file, which is why tests/, ci/ and .github/ are pruned explicitly; without a rule the release sdist shipped a top-level conftest that imports xdist and rewrites HIP_VISIBLE_DEVICES in any xdist worker of whatever project unpacked it. Verified after the fix: $ tar tzf dist/amd_flashinfer-0.6.18+amd.1.dev12.tar.gz \ | grep -cE '/conftest.py$' 0 Dropping the hook's RuntimeError traded a loud abort for a silent over-subscription. On a CPX host without rocminfo on PATH, get_physical_card_device_indices() returns empty while torch reports 32 logical XCDs, so the fallback picked 16 workers and pinned each to a logical device -- precisely the one-worker-per-XCD configuration the halving exists to avoid, and the source of the intermittent HSA failures that motivated it. It still falls back, since raising would stop a GPU-free checkout running GPU-free tests, but now warns and says to pass -n N. The docstring also records that no GPU means a single worker. The README's Quick start passed no --build-arg, so the image it told the reader to build runs as UID 1003 while the bind-mounted tree is theirs; the very next command, the editable install, then fails on Permission denied writing _version.py and the generated include/csrc trees. The notebook had the same gap. Both now carry the trio and say why. The mismatch warning rewritten in the previous commit still emitted a repo.radeon.com URL, interpolating the three-component system_rocm (rocm-rel-7.1.0/) where the published directories are two-component (rocm-rel-7.1.1/, rocm-rel-7.2/), and no such directory exists at all for ROCm 7.14 or 10.0 -- the stack this release is built against. Since the point was to stop hard-coding a command, the URL goes too.

demandal25 · 2026-09-03T16:54:42Z

Disposition of the four suppressed comments in review 5104079202 (they have no threads).

README.md:229 — "Each worker is pinned" overstates it. Fixed in 3a0430d. Now says each worker sets HIP_VISIBLE_DEVICES, which scopes the subprocesses a test spawns, not the worker itself.

conftest.py:64 — the hook's branches have no test coverage. Fixed in 3a0430d. This was the gap that let the conftest move break test_worker_pinning silently, so it earned the tests: halving, the one-card floor, the torch fallback, and where the hook lives. A/B — reverting the fallback fails test_worker_count_falls_back_to_torch and nothing else.

hip_utils.py:727 — the version-mismatch warning still recommends torch 2.7.1. Fixed in 3a0430d, and taken further in 4d222d6: the remaining repo.radeon.com URL interpolated the three-component system_rocm (rocm-rel-7.1.0/) where the published directories are two-component, and none exists at all for ROCm 7.14 or 10.0 — the stack this release targets. Hard-coding any command is what made this stale, so it now points at the matrix.

conftest.py:82 — the worker-side pinning has the same CPU-only import. Declined, same reason as the inline thread: pre-existing, not introduced here. tests/conftest.py on the base commit already did this import inside the same gw branch, so a CPU-only wheel failed identically before this PR.

Self-review after the above found five more, fixed in 4d222d6 — the most consequential being that the new root conftest.py was shipping in the release sdist (no MANIFEST.in rule), and that dropping the hook's RuntimeError traded a loud abort for silent CPX over-subscription. Both are described in that commit message.

Copilot

🟡 Changes recommended

GPU pinning can escape inherited visibility restrictions, and installation diagnostics contain incorrect ROCm guidance.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

README.md:276

This describes the opposite of the implemented restoration behavior. The shim saves a pre-existing GPU_ARCHS and restores it in finally; it only leaves the derived architecture set when the variable was initially absent (flashinfer/jit/rocm/aiter_source.py:351-441). Document that distinction so users do not expect their explicit setting to be permanently overwritten.

| `GPU_ARCHS` | autodetected | AITER's own JIT architecture. Respected when AITER is imported directly, but a shim build overwrites it from `FLASHINFER_ROCM_ARCH_LIST` and does not restore it. |

examples/amd_flashinfer_rocm_tutorial.ipynb:63

The option recommends aligning with the named ROCm 10.0 environment, but substituting 10.0 into this template fails because that rocm-rel- directory does not exist (the updated README and docker/Dockerfile.rocm:10-12 explicitly call this out). Use a concrete available pair here and direct ROCm 7.14/10.0 users to the development image.

    "pip install torch==<version> -f https://repo.radeon.com/rocm/manylinux/rocm-rel-<rocm-version>/\n",

flashinfer/rocm/hip_utils.py:752

The referenced table does not say which PyTorch version pairs with each ROCm version; it only lists independent supported-version rows. Moreover, wheel users may not have a local README.md. Link the actual online installation section without claiming it contains a version mapping.

                f"The supported-toolchain table in README.md says which torch\n"
                f"pairs with it and where to get it -- repo.radeon.com publishes\n"
                f"no rocm-rel- directory for every ROCm release, so there is no\n"
                f"one command that is right for all of them.\n"

Files reviewed: 14/14 changed files
Comments generated: 2
Review effort level: Balanced

The GPU_ARCHS row was my own error, and worth naming: I took it from a review claim in the previous round without checking it. aiter_source.py saves GPU_ARCHS into `prev` and restores it in a `finally`, so an explicit value *is* preserved; the derived architecture is left set only when the variable was absent to begin with, and the code says exactly why. The row now describes that instead of "does not restore it". The other three are the same defect in three places: a hard-coded repo.radeon.com command presented as the fix for an error that can occur on any ROCm release. The CPU-only-torch error still named torch==2.9.1 / rocm-rel-7.2 unconditionally, and the notebook's Option 2 told the reader to substitute their ROCm version into a rocm-rel-<v> template -- which fails on exactly the 10.0 the option above it recommends. Both now say the wheel depends on the release, point at the Quick start, and the notebook shows a concrete working pair and sends 7.14/10.0 users to the development image. Declined: treating an inherited HIP_VISIBLE_DEVICES as an upper bound on worker counting and pinning. The concern is real -- rocminfo enumerates the host, so under an allocation of GPUs 4,5 a worker can be pinned to host index 0 and its children escape. But it is pre-existing behaviour this PR moved verbatim, not something it introduces: $ diff <(git show origin/amd-integration:tests/conftest.py | sed -n '/^_xdist_worker = /,/HIP_VISIBLE_DEVICES.. = str/p') \ <(sed -n '/^_xdist_worker = /,/HIP_VISIBLE_DEVICES.. = str/p' \ conftest.py) (no output) Making the pinning allocation-aware changes how every existing CI and cluster run selects devices, which is its own change with its own blast radius -- not a rider on a docs PR.

demandal25 · 2026-09-03T17:13:55Z

Disposition of the three suppressed comments in review 5104630909 (no threads).

README.md:276 — the GPU_ARCHS row describes the opposite of the implemented behaviour. Fixed in 1efc74f. Correct, and my error: I took "does not restore it" from a review claim in the previous round without checking it. aiter_source.py saves GPU_ARCHS into prev and restores it in a finally, so an explicit value is preserved; the derived architecture is left set only when the variable was absent to begin with.

examples/…tutorial.ipynb:63 — the rocm-rel-<rocm-version> template fails for the environment the option above recommends. Fixed in 1efc74f. Shows a concrete working pair (7.2 / torch 2.9.1) and sends ROCm 7.14 and 10.0 users to the development image, which carries a matching torch.

hip_utils.py:752 — README has no ROCm→torch mapping, and a wheel user has no local copy. Fixed in 1efc74f. Both diagnostics now link https://github.com/AMD-Ecosystem/flashinfer#quick-start and say the wheel depends on the ROCm release, rather than claiming a mapping exists.

That closes the loop from my side: the two remaining items are declines with evidence (CPU-only-torch import and the HIP_VISIBLE_DEVICES upper bound, both pre-existing and both recorded in their threads). Later rounds re-raising them should be read against those replies. What this needs next is a human reviewer.

Copilot

🔵 Needs a closer look

Benchmark coverage, reference-check wording, and ROCm import-gating documentation still need human confirmation.

Review details

Suppressed comments (3)

README.md:244

This test list currently contains only batch decode plus paged/ragged batch prefill, so it does not drive the single-prefill, MLA, POD, block-sparse, or cascade paths advertised above. Calling it “every ROCm attention path” overstates the benchmark's coverage; narrow the claim to the paths actually listed.

The unified runner drives every ROCm attention path from one testlist:

README.md:254

--refcheck only compares the backends when both survive capability filtering. The Llama-3.1-405B rows explicitly drop fa2 and run with no reference (benchmarks/rocm/testlist_rocm.txt:12-15,59-63), so the unconditional side-by-side claim can make users treat unverified rows as checked.

Each line requests both `fa2` and `auto` and carries its own `--refcheck`,
so the two are compared side by side. **Read the `backend_resolved` column** — `auto` is a
request, not a result, and `backend_fallback_reason` says why AITER was

docs/rocm/backends.md:29

The trtllm_* wildcard is broader than CUDA_ONLY_MODULES: it also matches flashinfer.comm.trtllm_moe_alltoall, which is not directly gated and fails transitively while naming flashinfer.comm.mnnvl, contrary to the import behavior promised above. List the three directly gated TensorRT-LLM modules explicitly so this table remains an accurate rendering of the gate.

| `flashinfer.comm.*` (`cuda_ipc`, `mnnvl`, `nvshmem*`, `trtllm_*`, `vllm_ar`, `mixed_comm`) | NVLink / NVSHMEM transports |

Files reviewed: 14/14 changed files
Comments generated: 0 new
Review effort level: Balanced

All three from Copilot round 3, all confirmed against the files. "every ROCm attention path" was wrong about the testlist, which holds only batch decode and paged/ragged batch prefill: $ grep -oE '\--routine [A-Za-z]+' benchmarks/rocm/testlist_rocm.txt | sort | uniq -c 12 --routine BatchDecodeWithPagedKVCacheWrapper 6 --routine BatchPrefillWithPagedKVCacheWrapper 6 --routine BatchPrefillWithRaggedKVCacheWrapper Single prefill, MLA, POD, block-sparse and cascade are not driven by it. --refcheck compares the backends only where both survive capability filtering. The testlist's own header says the Llama-3.1-405B rows lose fa2 -- GQA group size 16 is outside DISPATCH_GQA_GROUP_SIZE {1,2,3,4,8} -- and are therefore unverified. Presenting the comparison as unconditional invites treating those rows as checked. The `trtllm_*` wildcard in the CUDA-only table was broader than the gate. CUDA_ONLY_MODULES names trtllm_alltoall, trtllm_ar and trtllm_mnnvl_ar; flashinfer/comm/trtllm_moe_alltoall.py exists and is not gated, so it fails transitively while naming its dependency. The nine comm modules are now listed explicitly, with a note on what a transitive failure looks like -- the point of that table is that it renders the gate exactly.

demandal25 · 2026-09-03T17:53:46Z

Disposition of the three suppressed comments in the round-3 review (no threads). All three confirmed against the files; all three fixed in df589ec.

README.md:244 — "every ROCm attention path" overstates the testlist. Right. It holds batch decode and paged/ragged batch prefill only; single-prefill, MLA, POD, block-sparse and cascade are not driven by it. Claim narrowed to what is there.

README.md:254 — --refcheck does not compare unconditionally. Right, and the testlist's own header already said so: the Llama-3.1-405B rows lose fa2 because GQA group size 16 is outside DISPATCH_GQA_GROUP_SIZE {1,2,3,4,8}, so they run with no reference. The README now carries that caveat instead of implying every row is verified.

docs/rocm/backends.md:29 — the trtllm_* wildcard is broader than the gate. Right. CUDA_ONLY_MODULES names trtllm_alltoall, trtllm_ar and trtllm_mnnvl_ar; flashinfer/comm/trtllm_moe_alltoall.py exists and is not gated. Since the point of that table is to render the gate exactly, the nine comm modules are now listed individually, plus a note that an ungated module importing a gated one fails transitively and names the dependency.

Loop closed. The only open items are the two declines already recorded in their threads — the CPU-only-torch import guard and treating an inherited HIP_VISIBLE_DEVICES as an upper bound on pinning — both pre-existing behaviour this PR moves rather than introduces. The second is a genuine gap on shared clusters and deserves its own change; it is not a rider on a docs PR. Later rounds re-raising either should be read against those replies.

Copilot

🔵 Needs a closer look

Review readiness is mixed, and the broad documentation and test-infrastructure changes warrant final human review.

Review details

Suppressed comments (1)

docs/rocm/backends.md:353

The fp8 requirements are incomplete: _fp8_shape_problem also rejects inter_dim values that do not divide CK's stage-2 K tile. On gfx950, explicit block_m=32 or 64 requires divisibility by 256 (128 for block_m=128); gfx942 uses 64 or 128 depending on the width. Document this constraint so configurations satisfying the two listed requirements do not still fail unexpectedly.

* The **weights** may be fp8, matching `hidden_states` otherwise. fp8 needs
  both `w1_scale` and `w2_scale` (neither alone) and `model_dim % 128 == 0`.

Files reviewed: 14/14 changed files
Comments generated: 0 new
Review effort level: Balanced

…iers Two changes the docs needed, both from review. **Supported set is now one configuration** -- ROCm 10.0, torch 2.12.0, Python 3.12, Ubuntu 24.04, amd-aiter 0.1.20, what docker/Dockerfile.rocm builds and what this release is tested on. The old table listed five ROCm and three torch versions as "Supported", which was a claim nothing backed: no run covers 7.0.2 or torch 2.8.0, and the pins are not independent anyway (0.1.20 is cp312-only, torch must stay below 2.13 for a c10 symbol AITER's prebuilt kernels need, and repo.radeon.com has no rocm-rel- directory for 10.0 at all -- which is why the supported configuration is an image rather than a version list). The torch 2.9.1 / rocm-rel-7.2 recipes that contradicted it are gone from the README and the notebook. `ROCM_COMPAT_MATRIX` in hip_utils.py is deliberately untouched. It is an arch-vs-ROCm build guard, not a support claim, and narrowing it would start rejecting installs that work today. **The matrix now has one "works" state.** ✅ vs ◻️ rendered `VALIDATED if entry.evidence else DECLARED` -- that is, whether whoever added the row pasted a measurement string. It read as a support tier and was not one: nobody failed to validate the ◻️ rows, the field simply was not filled in, and a caller cannot act on the difference. `evidence` stays in arch_caps.py as provenance for us; it no longer splits the rendered cell. 54 cells collapse to ✅, the two gfx950 known-bad cells keep ⚠️. Also fixes the backends.md TOC, which listed "Not available on ROCm" second while the heading sat first, and adds the two headings the TOC had never listed. Verified beyond lint, since the docs now make claims I had not run: - the tutorial notebook executes end to end under nbclient with allow_errors=False, exit 0 - the documented benchmark command runs the whole testlist to a CSV, exit 0, and the CSV carries backend_resolved and backend_fallback_reason as the README says

Copilot

🟡 Changes recommended

The README incorrectly implies that the documented gfx950 configuration was tested.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

flashinfer/rocm/hip_utils.py:725

The phrase “including the 10.0 the development image uses” is grammatically incomplete and makes this user-facing recovery message harder to parse.

            "release -- repo.radeon.com publishes no rocm-rel- directory for\n"
            "some of them, including the 10.0 the development image uses.\n\n"

scripts/gen_arch_support_matrix.py:162

Removing the rendered validated/declared distinction leaves this script's module docstring stale: it still says the generated matrix expresses “the difference between a measured row and a declared one.” Update that description to match the new single supported tier.

    # Deliberately not split on `evidence`. That field records which stack a row
    # was measured on, which is provenance for us; rendering it as a second tier
    # told readers a row was less supported when it only meant nobody had pasted
    # a string in. Both states mean the same thing to a caller: it works.

Files reviewed: 16/16 changed files
Comments generated: 1
Review effort level: Balanced

Copilot

🔵 Needs a closer look

Broad routing and support-documentation changes require human review, with two documentation inaccuracies still noted.

Review details

Suppressed comments (2)

README.md:140

The count is incorrect: the cascade wrapper constructors also expose no backend= parameter (flashinfer/cascade.py:307, :660, and :925), as the newly added routing explanation later acknowledges. Listing only these two APIs can mislead users into expecting that cascade can be pinned explicitly; include the cascade wrappers in this statement.

Two ops take no `backend=` argument: `single_decode_with_kv_cache` is
HIP-only, and `aiter_fused_moe` is AITER-only.

flashinfer/rocm/arch_caps.py:443

This note is inaccurate for the decode cascade path. BatchDecodeWithSharedPrefixPagedKVCacheWrapper constructs BatchDecodeWithPagedKVCacheWrapper at flashinfer/cascade.py:674 and invokes it at line 819; its shared-prefix side also calls single_prefill_with_kv_cache. Because this text feeds the generated README, users are incorrectly told that every cascade attention path inherits batch-prefill routing and constraints. Describe the auto-routed prefill and decode paths separately, then regenerate the matrix.

        note='Two-level shared-prefix attention; a fused single-kernel variant is gated behind `FLASHINFER_HIP_FUSED_CASCADE=1`. The `hip` backend is the merge kernels only -- each level\'s attention runs through `BatchPrefillWithPagedKVCacheWrapper` at `backend="auto"`, so it can dispatch to AITER. The cascade wrappers expose no `backend=` to override that.',

Files reviewed: 23/23 changed files
Comments generated: 0 new
Review effort level: Balanced

The cascade note said each level runs through `BatchPrefillWithPagedKVCacheWrapper`. That is true of two of the three wrappers; `BatchDecodeWithSharedPrefixPagedKVCacheWrapper` builds a `BatchDecodeWithPagedKVCacheWrapper` (cascade.py:674, used at :819) and also calls `single_prefill_with_kv_cache`. Since this note feeds the generated README, it told readers every cascade path inherited batch-prefill routing and constraints. Both it and backends.md now name the entry point per wrapper. The README also undercounted what cannot be pinned: all three cascade wrappers take no `backend=` either, not just single_decode and aiter_fused_moe. benchmarks/README.md still said graph-captured `auto` stays on fa2 "by design". #336 changed that -- declare `max_seq_len` and `auto` may select AITER, replaying for any shorter sequence. The README's newly un-commented Benchmarking section links there, so the stale claim was reachable.

Copilot

🔵 Needs a closer look

Its broad documentation, packaging, routing, and test-infrastructure scope warrants final human review; one minor shell-safety nit also remains.

Review details

Suppressed comments (1)

flashinfer/rocm/hip_utils.py:725

The displayed fix is not shell-safe: <version> and <your-rocm> are parsed as input/output redirections, so copying it fails with version: No such file or directory before pip runs. Use shell-safe placeholder names and explicitly tell the user to replace them.

            "  2. pip install torch==<version> -f "
            "https://repo.radeon.com/rocm/manylinux/rocm-rel-<your-rocm>/\n\n"

Files reviewed: 23/23 changed files
Comments generated: 0 new
Review effort level: Balanced

…ntal" "Experimental" was inherited from #221 and overstates it. The flag swaps one branch at cascade.py:559 -- folding each level's merge into its prefill kernel via partial_state, instead of running the level and then merge_state_in_place. It is off by default, and both branches are covered: tests/rocm/test_cascade.py and test_shared_prefix_kernels.py flip the module constant and exercise each. The label told a reader to avoid a tested, opt-in path without saying what it was or what it costs. All three places now describe the behaviour, and note it is read once at import so it must be set before importing flashinfer -- which the old wording never mentioned and is the part that actually catches people out.

demandal25 · 2026-09-04T14:37:57Z

Disposition of the suppressed comments in reviews 5114041845 and 5114112319.

Fixed in f69662e

The cascade note was wrong about the decode path. It said each level runs through BatchPrefillWithPagedKVCacheWrapper; that holds for two of the three wrappers, but BatchDecodeWithSharedPrefixPagedKVCacheWrapper builds a BatchDecodeWithPagedKVCacheWrapper (cascade.py:674, used at :819) and also calls single_prefill_with_kv_cache. Since that note feeds the generated README, readers were told every cascade path inherited batch-prefill routing and constraints. Both the note and backends.md now name the entry point per wrapper.
The README undercounted what cannot be pinned — all three cascade wrappers take no backend= either, not just single_decode_with_kv_cache and aiter_fused_moe.
benchmarks/README.md still said graph-captured auto stays on fa2 "by design" — feat(rocm): capacity-based CUDA-graph capture for AITER decode (max_seq_len) #336 changed that: declare max_seq_len and auto may select AITER, replaying for any shorter sequence. The un-commented Benchmarking section links there, so the stale claim was reachable.

Fixed in 632a9e7, from a maintainer question rather than the review

FLASHINFER_HIP_FUSED_CASCADE was labelled "Experimental". It is not: the flag swaps one branch at cascade.py:559 — folding each level's merge into its prefill kernel via partial_state instead of merging afterwards — it is off by default, and both branches are covered by test_cascade.py and test_shared_prefix_kernels.py. The label warned readers off a tested path without saying what it did. All three places now describe the behaviour, and note that it is read once at import, which is the part that actually catches people out.

Stale

CLAUDE.md and the benchmarks/README.md known-bad passage were both fixed in 71fca20, before this review ran; the hip_utils.py grammar nit in ee251b7.

Copilot

🟡 Changes recommended

The root conftest fails non-xdist runs when plugin autoload is disabled; the hook must be marked optional.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

conftest.py:41

Importing xdist only proves that the package is installed, not that its pytest plugin is loaded. With -p no:xdist or PYTEST_DISABLE_PLUGIN_AUTOLOAD=1, this root conftest still defines an unknown hook and pytest raises PluginValidationError, so non-xdist runs fail despite the guard's stated purpose. Mark the hook optional so disabling plugin autoload remains supported.

Files reviewed: 23/23 changed files
Comments generated: 3
Review effort level: Balanced

`import xdist` proves the package is installed, not that its plugin is loaded. Under `-p no:xdist` or PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 the import still succeeds, the hookspec does not exist, and pluggy aborts the entire session. Reproduced before fixing: $ pytest tests/rocm/test_arch_caps.py -p no:xdist INTERNALERROR> pluggy._manager.PluginValidationError: unknown hook 'pytest_xdist_auto_num_workers' in plugin <module 'conftest' ...> `@pytest.hookimpl(optionalhook=True)` is what makes pluggy skip an impl whose spec is absent. All three modes now work -- `-p no:xdist`, autoload disabled, and `-n auto` still reporting 4 workers -- with a test asserting the marker, since the failure mode is a session abort rather than a wrong answer. Separately, the description I gave FLASHINFER_HIP_FUSED_CASCADE last commit overstated it on two axes. Only `MultiLevelCascadeAttentionWrapper.run` consults the flag; the two shared-prefix wrappers merge unconditionally. And even there, a level that resolves to AITER still merges post-hoc, because the AITER kernel takes no partial state (`rocm/prefill.py:3056`). Corrected in the README env-var table, backends.md and the capability note. The recovery command used `<version>` and `<your-rocm>`, which a shell parses as redirections -- copying it fails with "version: No such file or directory" before pip runs. Now bare names with an explicit instruction to replace them.

demandal25 · 2026-09-04T14:54:01Z

Disposition of the suppressed comments in reviews 5114275876 and 5114371960. Both were right, and the second is the most serious finding on this PR.

conftest.py — the xdist guard did not guard. import xdist proves the package is installed, not that its plugin is loaded. Under -p no:xdist or PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 the import still succeeds, the hookspec does not exist, and pluggy aborts the whole session — not the test, the session:

$ pytest tests/rocm/test_arch_caps.py -p no:xdist
INTERNALERROR> pluggy._manager.PluginValidationError: unknown hook
'pytest_xdist_auto_num_workers' in plugin <module 'conftest' ...>

Reproduced, then fixed in 4f6130d with @pytest.hookimpl(optionalhook=True), which is what makes pluggy skip an implementation whose spec is absent. Verified in all three modes — -p no:xdist, autoload disabled, and -n auto still reporting 4 workers — with a test on the marker, since the failure mode is a session abort rather than a wrong answer. Worth noting this became reachable because this PR moved the hook to the repo root: in tests/rocm/ it only ever loaded alongside the rocm suite.

hip_utils.py — the recovery command was not shell-safe. pip install torch==<version> -f .../rocm-rel-<your-rocm>/ — a shell parses <version> as a redirection, so copying it fails with version: No such file or directory before pip runs. Now bare TORCH_VERSION / ROCM_VERSION with an explicit instruction to replace them.

The three inline threads on the fused-cascade wording are answered and resolved: the description I added one commit earlier overstated it on two axes, and both corrections are in 4f6130d.

Copilot

🔵 Needs a closer look

Two user-facing commands are currently broken, and companion-package metadata remains inconsistent.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

README.md:49

The linked “full recipe” does not put the shell in the mounted source tree: CONTRIBUTING.md:39 mounts /workspace, but the image's WORKDIR is /home/$USERNAME and the command supplies neither -w /workspace nor a subsequent cd. Following its next pip ... . command therefore targets the home directory and fails. Add the working-directory step to the linked recipe before directing users to it.
examples/amd_flashinfer_rocm_tutorial.ipynb:47
This copy-paste command is invalid shell syntax: Bash interprets the angle brackets in <container-name> as redirections and exits before invoking Docker. Use a concrete shell-safe name (or a defined variable) instead.

pyproject.toml:48

This corrects only the main wheel's metadata. amd-flashinfer-jit-cache/pyproject.toml:39-40 still emits https://github.com/ROCm/flashinfer for the companion wheel that this PR now documents building and installing, so its Homepage and Repository links still lead to the different repository. Update that package's URLs in the same metadata correction.

Homepage = "https://github.com/AMD-Ecosystem/flashinfer"
Repository = "https://github.com/AMD-Ecosystem/flashinfer"

Files reviewed: 23/23 changed files
Comments generated: 0 new
Review effort level: Balanced

CONTRIBUTING's `docker run` mounts the source at /workspace but never cd's there, and the image's WORKDIR is /home/$USERNAME -- so the very next command in that section, `pip install --no-build-isolation -ve .`, ran against an empty home directory. The README block already had `-w /workspace`; the "full recipe" it points at did not. Also quotes "$PWD", which breaks on a path with spaces. The notebook's `docker run` used `--name=<container-name>`. Bash reads the angle brackets as redirections and exits before docker is invoked -- the same defect as the `<version>` placeholder fixed in 4f6130d, which I did not sweep for elsewhere at the time. Now a concrete name; a repo-wide check finds no other angle-bracket placeholder in a shell block. The pyproject URL correction covered only the main wheel. amd-flashinfer-jit-cache/pyproject.toml still published github.com/ROCm/flashinfer as Homepage and Repository -- a different repository -- and this PR is what tells people to build and install that wheel, so its METADATA is now reachable.

demandal25 · 2026-09-04T15:23:06Z

Disposition of the three suppressed comments in review 5114575271. All correct, all fixed in 3012c39.

README.md:49 — the linked "full recipe" left the shell outside the source tree. CONTRIBUTING.md's docker run mounts /workspace but passes no -w, and the image's WORKDIR is /home/$USERNAME, so the very next command in that section — pip install --no-build-isolation -ve . — ran against an empty home directory. The README block already had -w /workspace; the recipe it points at did not. Also quoted "$PWD".
examples/…tutorial.ipynb:47 — --name=<container-name> is invalid shell. Bash reads the angle brackets as redirections and exits before docker runs. Same defect as the <version> placeholder fixed in 4f6130d; I fixed that one without sweeping for siblings. Now a concrete name, and a repo-wide check finds no other angle-bracket placeholder in a shell block.
pyproject.toml:48 — the metadata fix covered only the main wheel. amd-flashinfer-jit-cache/pyproject.toml still published github.com/ROCm/flashinfer as Homepage and Repository. That is a different repository, and this PR is precisely what tells people to build and install that wheel, so its METADATA is now reachable. Both URLs repointed.

Separately: I had reported "no new review" shortly before this one was submitted, having checked unresolved-thread count — which structurally cannot show suppressed comments. I then audited all suppressed blocks across every review on this PR (32 unique items) and confirmed each is fixed, declined with evidence, or stale.

Copilot

🔵 Needs a closer look

The broad cross-cutting changes and remaining contradictory installation comments warrant final human review.

Review details

Suppressed comments (3)

amd-flashinfer-jit-cache/pyproject.toml:40

The runtime-dependency comment immediately above this hunk still gives the removed ROCm 7.2 / torch 2.9.1 install command. Point it at the supported development image instead so this metadata file no longer contradicts the refreshed installation docs.

Repository = "https://github.com/AMD-Ecosystem/flashinfer"

amd-flashinfer-jit-cache/pyproject.toml:39

The build-system comment at lines 2–5 still directs maintainers to the obsolete ROCm 7.2 / torch 2.9.1 stack. This contradicts the PR's single supported ROCm 10.0 image setup; replace it with the current Dockerfile/README guidance.

Homepage = "https://github.com/AMD-Ecosystem/flashinfer"

pyproject.toml:47

The package metadata still contains an installation example for the now-unsupported ROCm 7.2 / torch 2.9.1 stack at lines 20–23. This contradicts the PR's single supported ROCm 10.0 image story and leaves one of the advertised removed ROCm 7.x references in a contributor-facing file. Update that comment to direct readers to docker/Dockerfile.rocm and the README instead.

Homepage = "https://github.com/AMD-Ecosystem/flashinfer"

Files reviewed: 24/24 changed files
Comments generated: 0 new
Review effort level: Balanced

My earlier sweep grepped the docs and flashinfer/rocm/ and stopped there, so three copies of a `pip install torch==2.9.1 -f .../rocm-rel-7.2/` example survived in pyproject comments -- the main package's runtime-dependency note and both of amd-flashinfer-jit-cache's. Those are contributor-facing files that now contradict the single-configuration statement. All three point at the development image instead, which is the real answer: torch is not pip-installable for ROCm 10.0 at all. Then swept the whole repo rather than one directory, which turned up test prose naming the removed gate as the live example -- "currently gfx950 on ROCm 7.2.x". The logic in those tests is generic (`capability_available`) and still correct; only the comments named a row that no longer exists, and they now say the skip is dormant until a gate is added. Also generalises a synthetic fallback-reason string and one evidence fixture. Left deliberately: `test_arch_caps.py:429` explains *why* TestKnownBadWindows uses a synthetic row, so naming the dropped one is the point; the `_window` tests use 7.2/7.3 as arbitrary interval literals; and the AITER ABI headers record which release a struct was generated against, which is provenance I cannot restate without regenerating them.

demandal25 · 2026-09-04T15:37:58Z

Disposition of the three suppressed comments in review 5114939055. All correct, all fixed in 134457c — and they share one cause.

All three point at the same thing: a pip install torch==2.9.1 -f .../rocm-rel-7.2/ example living in a pyproject comment. There were three copies — the main package's runtime-dependency note, and both of amd-flashinfer-jit-cache/pyproject.toml's (build-system and runtime). My earlier ROCm 7.x sweep grepped the docs and flashinfer/rocm/ and stopped there, so contributor-facing config files kept contradicting the single-configuration statement. All three now point at the development image, which is the real answer: torch is not pip-installable for ROCm 10.0 at all.

Having been caught by a partial sweep, I then ran it across the whole repository instead of one directory. That turned up test prose naming the removed gate as the live example ("currently gfx950 on ROCm 7.2.x"). The logic in those tests is generic — capability_available(...) — and still correct; only the comments named a row that no longer exists, so they now say the skip is dormant until a gate is added. Also generalised a synthetic fallback-reason string and one evidence fixture.

Three deliberate non-changes, each for a reason rather than an oversight:

tests/rocm/test_arch_caps.py:429 explains why TestKnownBadWindows exercises a synthetic row; naming the row that was dropped is the point of the sentence.
The _window tests use 7.2/7.3 as arbitrary literals for half-open interval rendering — fixture data for a generic mechanism.
The vendored AITER ABI headers record which release a struct was generated against. That is provenance I cannot restate without regenerating them against the current wheel, which is a change with its own blast radius and does not belong in a docs PR.

Copilot

🔵 Needs a closer look

Two documented build paths remain broken in the fresh development image, and final human review is warranted.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

CONTRIBUTING.md:87

This command is not runnable in the documented fresh development image: docker/Dockerfile.rocm does not install the build frontend, and the preceding editable-install recipe installs . rather than .[dev] (where build is declared). Use the already-established pip wheel frontend or explicitly install build before this loop.
amd-flashinfer-jit-cache/README.md:11
The newly linked build path is incomplete for the documented fresh development image: docker/Dockerfile.rocm does not install the build frontend, and the Quick start installs . rather than the .[dev] extra that declares it. Following this link therefore reaches a python -m build command that fails with No module named build; state the prerequisite or use pip wheel.

Files reviewed: 28/28 changed files
Comments generated: 0 new
Review effort level: Balanced

demandal25 · 2026-09-04T16:01:26Z

Disposition of the two suppressed comments in review 5115085055 (CONTRIBUTING.md:87 and amd-flashinfer-jit-cache/README.md:11). Both make the same claim, and both are declined.

The claim is that python -m build fails in the documented environment with No module named build, because docker/Dockerfile.rocm does not install the build frontend and the Quick start installs . rather than .[dev]. The first half is true — the Dockerfile's pip install list does not name build — but the conclusion does not follow: the rocm/pytorch base image already provides it.

$ docker run --rm rocm/pytorch:rocm10.0_ubuntu24.04_py3.12_pytorch_release_2.12.0 \
    bash -lc 'python -c "import build; print(1)"'
1

$ docker run --rm demandal-fi-rocm10:latest bash -lc 'pip show build | head -2'
Name: build
Version: 1.5.0

So the documented loop runs as written in a fresh container built from that Dockerfile — which is also how the two per-arch jit-cache wheels in this PR's test plan were produced. Rewriting it to pip wheel, or adding a .[dev] prerequisite readers do not currently need, would be a change made on a false premise.

Worth recording that the dependency is transitive: build reaches the image through the base rather than through anything this repo declares, so a future base image could drop it. [project.optional-dependencies] dev does declare build, so .[dev] is the robust route if that ever happens. Not changing the documented command for a hypothetical, but noting it here so the next person hitting it knows where to look.

demandal25 added 5 commits September 3, 2026 10:54

docs: state the real cold-JIT cost for an AITER variant

0a1bdf1

'a few minutes' understated it; docs/rocm/backends.md already budgets 20+ minutes for a cold AITER variant, and a single-prefill fp16 variant took 62.9s to build during wheel verification.

Copilot AI balanced review requested due to automatic review settings September 3, 2026 15:55

Copilot started reviewing on behalf of demandal25 September 3, 2026 15:56 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread conftest.py

Comment thread examples/amd_flashinfer_rocm_tutorial.ipynb

Comment thread CONTRIBUTING.md Outdated

Comment thread docs/rocm/backends.md Outdated

demandal25 changed the title ~~Bring the user-facing docs up to v0.6.18, and make -n auto GPU-derived everywhere~~ Bring the user-facing docs up to v0.6.18 Sep 3, 2026

demandal25 and others added 3 commits September 3, 2026 12:15

Merge branch 'amd-integration' into rocm-release-docs-refresh

768e741

Copilot AI review requested due to automatic review settings September 3, 2026 16:53

Copilot started reviewing on behalf of demandal25 September 3, 2026 16:55 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread conftest.py

Comment thread flashinfer/rocm/hip_utils.py Outdated

Copilot AI review requested due to automatic review settings September 3, 2026 17:13

Copilot started reviewing on behalf of demandal25 September 3, 2026 17:14 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 3, 2026 17:53

Copilot started reviewing on behalf of demandal25 September 3, 2026 17:54 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 3, 2026 19:07

Copilot started reviewing on behalf of demandal25 September 3, 2026 19:09 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread README.md

Copilot AI review requested due to automatic review settings September 4, 2026 14:15

Copilot started reviewing on behalf of demandal25 September 4, 2026 14:16 View session