refactor: move the ROCm C++ to root csrc/rocm and apply the directory/rocm convention - #333
Merged
Merged
Conversation
include/flashinfer/attention/aiter/ held six files, all fork-owned -- upstream has zero files at that path: git ls-tree -r --name-only upstream/main -- include/flashinfer/attention/aiter/ | wc -l 0 It was the last fork code sitting inside an upstream-owned directory, and its four public headers were the only non-rocm/ flashinfer headers any ROCm translation unit included. Moving it to include/flashinfer/rocm/attention/aiter/ makes "no ROCm TU reaches an upstream-owned header" true by construction rather than by inspection, and leaves include/flashinfer/attention/ purely upstream. Rewrites 11 include lines (5 in csrc .cu/.cc, 6 cross-includes among the moved headers) plus six textual references that no C++ tool would catch: .pre-commit-config.yaml, docs/rocm/backends.md, flashinfer/aiter_utils.py, .claude/skills/pr-workflow/SKILL.md, and two in tests/rocm_tests/test_aiter_version_gate_hip.py -- the one at :75 is a live path construction, not prose, so missing it breaks that test. The REUSE hook regex collapses from an alternation to a single prefix, since the aiter headers now live under rocm/. Coverage is unchanged, not merely believed to be -- same 47 files, same basenames: old ^include/flashinfer/(rocm|attention/aiter)/.*\.(cuh|hpp|h)$ -> 47 new ^include/flashinfer/rocm/.*\.(cuh|hpp|h)$ -> 47 diff of the two basename lists: empty Verified by preprocessing each of the three moved entry-point headers and grepping the closure for any include/flashinfer/ path outside rocm/: empty for all three. Negative control: a TU on the old path fails with "'flashinfer/attention/aiter/batch_prefill.cuh' file not found".
Three things, all about the duplicate-guard tripwires. 1. The message was factually wrong in 29 of 30 headers. It read "... and ... both define FLASHINFER_X_; include only one", but the fork header does not define the upstream guard -- it defines FLASHINFER_ROCM_*. Someone debugging the error would go looking for a #define that is not there. Reworded to state the actual reason: the two headers define the same symbols. The thirtieth, rocm/dispatch.cuh, keeps its wording: it genuinely does define the DISPATCH_* macros that upstream's utils.cuh defines, so "both define" is true there. 2. mma had no tripwire at all. Its only #error (:169) is an unrelated "Unsupported GFX platform for MFMA ops" arch guard, so nothing guarded include/flashinfer/mma.cuh against rocm/mma_hip.h -- the two were kept apart by the namespace name alone. 3. With the tripwire in place the rename is redundant, so flashinfer::mma_hip becomes flashinfer::mma, matching every other fork header (which keeps its upstream namesake's symbol names). 10 call sites across prefill.cuh, permuted_smem.cuh and mma_types.hpp. Note what this does NOT do: the tripwire fires only when the upstream header is included first, and the obvious way to close that -- defining upstream's guard in the fork header -- is actively harmful. It makes upstream's #ifndef skip its whole body, which is the silent shadow #329 removed. Measured: #define FLASHINFER_EXCEPTION_H_ #include <flashinfer/exception.h> flashinfer::Error e(...); // error: use of undeclared identifier 'flashinfer' The reverse order is not silent regardless -- the compiler rejects it as a redefinition, just without naming the two files: fork-then-upstream -> exception.h:74:7: error: redefinition of 'Error' upstream-then-fork -> rocm/exception.h:6:2: error: "... include only one" Closing that gap properly would mean editing upstream headers, which the additive-only policy rules out. CONTRIBUTING.md now records this so the "improvement" is not proposed again. Verified: rocm/mma_hip.h compiles standalone; the new tripwire fires with its named message under -DFLASHINFER_MMA_CUH_; prefill.cuh, the heaviest mma consumer, is 0 errors under hipcc -fsyntax-only for gfx942.
…nd profiler Three fork-owned trees still used a name-suffixed form instead of a rocm/ subdirectory, and the Dockerfile sat in a directory it does not belong to: tests/rocm_tests/ -> tests/rocm/ benchmarks/rocm_benchmarks/ -> benchmarks/rocm/ rocm_profiler/ -> profiler/rocm/ .devcontainer/rocm/Dockerfile -> docker/Dockerfile.rocm.dev All three destinations are collision-free against upstream (0 files at tests/rocm, benchmarks/rocm, profiler/rocm), so the additive-only policy holds exactly as before. profiler/ already exists here as an upstream directory, which is why the profiler moves under it rather than staying at the root. The Dockerfile was never a devcontainer: .devcontainer/rocm/ had no devcontainer.json, unlike its cu126/cu128/cu129/cu130 siblings, so the only thing that ever used it was `docker build -f` in CONTRIBUTING.md:19 -- also its only reference in the repo, and nothing in CI builds it. The sibling convention is the inverse, e.g. .devcontainer/cu130/ devcontainer.json points at ../../docker/Dockerfile.cu130.dev, hence the .dev suffix here. docker/ is pruned from the sdist, so packaging is unchanged. With the directory carrying the ROCm marker, the per-file _hip suffix is redundant, so 46 test files and 3 benchmarks drop it (plus one _rocm). The rule is anchored to the trailing suffix, which is what correctly leaves test_hip_utils.py and test_modules_hip_codegen.py alone -- there "hip" names the module under test, it is not a platform tag. Dropping it does NOT cause the pytest "import file mismatch" it looks like it should. tests/__init__.py exists, so upstream's file resolves as module tests.test_activation while ours resolves as test_activation -- different names. Proven rather than argued: both now collect together, tests/rocm/test_activation.py: 73 tests/utils/test_activation.py: 1890 with 0 collection errors across the full testpaths (36,640 tests), and conftest.py had in any case already coexisted in both directories. File counts are unchanged on both moves (57 tests, 9 benchmarks), and the 265 path-sensitive tests -- coverage, build-backend, canary, jit-flag hooks, arch-matrix, run-header -- pass.
…ders math_hip.h -> math.h memory_ops_hip.h -> memory_ops.h mma_hip.h -> mma.h vec_dtypes_hip.h -> vec_dtypes.h The suffix separated a backend implementation from a same-stem wrapper sitting beside it (math_hip.h under math_ops.hpp, and so on). #329 deleted the wrappers, so nothing is left to disambiguate and the directory already says ROCm -- these were 4 of 41 headers under include/flashinfer/rocm/ still carrying it. Same reasoning as the tests/benchmarks rename in the previous commit. upstream_canary.py pairs on basename *including extension* (scripts/upstream_canary.py:273-282), with no .h/.cuh normalization, so math.h still does not pair with upstream's math.cuh. The four remain correctly listed as ROCm-only with no upstream counterpart; no new pairs and no spurious drift. One hazard this introduces, hence the note: include/flashinfer/rocm/ now contains math.h and memory_ops.h, and a quoted same-directory `#include "math.h"` from a future header there would resolve to ours before libc's. Every current include is fully qualified ("flashinfer/rocm/math.h") and the one bare use is angled (vec_dtypes.h:26), which still resolves correctly: hipcc -E ... | grep math.h -> "/usr/include/math.h" Verified: all four headers compile standalone, and prefill.cuh, permuted_smem.cuh and sampling.cuh -- the heaviest consumers -- are 0 errors under hipcc -fsyntax-only for gfx942.
Upstream keeps C++ at the repo root: 850 files under csrc/ against 23 inside the flashinfer package. This fork had its 65 ROCm sources the other way round. They now sit at csrc/rocm/, a path upstream owns no files in, so the additive-only policy holds exactly as before. The wheel does not change, and that is the point. build_backend_rocm.py already materialized flashinfer/include from the root include/; it now drives both trees from one _trees() table with the same three modes -- symlink for editable, filtered copy for wheel, cleared for sdist. Since the *destination* is untouched, get_csrc_dir() still returns <pkg>/csrc/rocm, so there is no public API change (it is re-exported at flashinfer/rocm/api.py:29) and the package-data globs are unmodified. Proven against a wheel built from the parent commit, full inventory, not counts: 431 members both sides, and the flashinfer/csrc/rocm/ set is byte-identical at 65 files. The only diff is the header renames from the two preceding commits. Zero stray flashinfer/data/ entries -- the upstream flashinfer.data package-dir scheme was evaluated and rejected precisely because it ships build_backend*.py into the wheel. _trees() is a function, not a module constant, so rebinding _root redirects every tree at once; that is what the test fixture does instead of patching two path globals. Two traps closed rather than discovered later: - test_build_backend.py::_isolated_project copied include/ and flashinfer/ but never root csrc/, so after this move the wheel inventory assertion would have compared 0 to 0. A/B'd by removing the new copytree line: the build then raises "missing source tree: .../csrc/rocm" instead of silently shipping an empty wheel. An explicit non-empty assertion now guards it too. - amd_coverage.py's _CSRC_DIR pointed at flashinfer/csrc/rocm, which after this commit only exists post-install. It reads the source tree now; a stale value there returns None and the JIT-reach section disappears from the report rather than erroring. Rollout: flashinfer/csrc/ is gitignored and generated, so existing editable installs need a reinstall, and a fresh worktree needs a second symlink -- CLAUDE.md and the pr-workflow skill carry the updated recipe in this commit rather than in the docs commit, since a worktree created from the old recipe will not build. Link flashinfer/csrc/rocm, never flashinfer/csrc, or upstream's 850-file CUDA tree lands inside the package. MANIFEST.in's recursive-include is deleted, not repointed: setuptools-scm's file finder already ships every tracked file, so the root tree needs no graft.
Follows the four preceding commits. The code-structure tree showed csrc/rocm under flashinfer/ rather than at the root, and the porting cheat sheet still pointed contributors at the old paths. Two distinctions the previous wording collapsed, and the reason most of these lines could not just be sed'd: - csrc/rocm/ is the *source*; flashinfer/csrc/rocm is the generated destination that build_backend_rocm.py materializes and that FLASHINFER_CSRC_DIR resolves to. Docs telling you where to add a file must say the former; docs describing runtime resolution must say the latter. Both appear, deliberately. - Inside a rocm/ directory a _hip suffix is redundant, but _aiter still means something (it names the backend a file routes to, not the platform), so the additive-only guidance keeps one and drops the other. CODEOWNERS is deliberately not touched. It looked like csrc/rocm/ would inherit `csrc/ @wenscarl @yzh119 ...`, but the file is inert on this fork -- GitHub rejects every entry: gh api repos/AMD-Ecosystem/flashinfer/codeowners/errors # 137 errors, all "Unknown owner ... make sure @x exists and has # write access to the repository" and PRs #328, #329 and #331 -- all of which touched csrc/ or include/flashinfer/ -- auto-requested zero reviewers. Adding rocm/ lines would be a no-op that enlarges a file that is pure rebase surface.
All of these are collateral from sed-based renames reaching past their
intent. Found by two rounds of /code-review, not by the test suite --
none of them fails a test, which is why each is listed.
1. A mangled identifier. The `test_*_hip` -> `test_*` rewrite also fired
inside a name that legitimately ends in the platform word:
def test_nvshmem_helpers_stay_absent_on_rocm() -> ..._absent_on()
2. Three test *functions* in test_rope.py renamed that should not have
been. test_generalized_rope_quantize_hip, ..._append_kv_cache_hip and
test_rope_quantize_fp8_append_paged_kv_cache_decode_hip lost their
suffix, colliding them by name with upstream's identically-named
functions in tests/attention/test_rope.py. The file-level suffix was
vestigial once the directory says rocm; the function-level one is not,
because `-k` selects across files. Restored -- and the second review
caught that the first fix was itself half-done, having restored the
three `def` lines but not the four comment/docstring references to
them, which then pointed at upstream's tests. test_rope.py is now
byte-identical to base.
3. CHANGELOG.md reverted to base. It is a historical record: one edit
rewrote the title of merged PR #192, another invented
`benchmarks/rocm/bench_decode_graph_hip.py`, a file that has never
existed under either name.
4. Stale paths in text the renames should have carried but missed --
two relative markdown links to the retired rocm_profiler/ (and one
link *label* left behind when only its target was fixed), the two
report headings in amd_coverage.py, and three places naming the
generated flashinfer/csrc/rocm copy where they mean the csrc/rocm
source a reader is being sent to edit: the aiter_loader.cc runtime
error text, backends.md, and activation.cuh's launcher comment.
Also closes a test gap rather than only fixing code: the csrc tree's
symlink target was asserted nowhere. test_editable_symlink_is_relative
went through a helper hardcoded to _trees()[0], so the include tree was
covered and csrc/rocm -- whose "../../csrc/rocm" target is depth-derived
rather than literal -- was not. Now parametrized over both. A/B'd by
making only the csrc link absolute:
tgt = src if dst.name == 'rocm' else _link_target(src, dst)
# before: 29 passed. after: FAILED ...test_editable_symlink_is_relative[csrc]
Rebased onto 94ac35d after #332 landed. That PR added
benchmarks/rocm_benchmarks/bench_norm.py into a directory this branch
renames -- git flagged it as a file-location conflict -- and introduced
four fresh references to the old path (docs/rocm/backends.md,
flashinfer/rocm/norm.py, tests/rocm/test_norm.py,
tests/rocm/test_rmsnorm_aiter.py), all retargeted here. The one content
conflict, a comment in test_norm.py, keeps #332's newer wording: it
describes the native default that PR introduced, and the older text it
replaced referenced a test file this branch renames anyway.
There was a problem hiding this comment.
Pull request overview
Refactors the AMD/ROCm fork’s source-tree layout to match upstream conventions by moving ROCm C++ sources to repo-root csrc/rocm/, moving AITER headers under include/flashinfer/rocm/attention/aiter/, and renaming ROCm test/benchmark/profiler directories to the */rocm/ convention—while keeping the installed wheel layout (flashinfer/csrc/rocm, flashinfer/include) stable via build-time materialization.
Changes:
- Move ROCm C++ sources from
flashinfer/csrc/rocm/→csrc/rocm/and update build/test tooling to stage the same files back into the wheel. - Rename ROCm-specific trees (
tests/rocm_tests→tests/rocm,benchmarks/rocm_benchmarks→benchmarks/rocm,rocm_profiler→profiler/rocm) and retarget references across docs/tests/scripts. - Drop redundant
_hipsuffixes in ROCm intrinsic headers and update include paths and tripwire messages accordingly.
Reviewed changes
Copilot reviewed 88 out of 192 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rocm/test_worker_pinning.py | Update doc reference to new test filename. |
| tests/rocm/test_run_header.py | Update doc reference to new conftest path. |
| tests/rocm/test_rope_aiter.py | Update comment references after renames. |
| tests/rocm/test_rocm_dispatch.py | Update comment references after renames. |
| tests/rocm/test_rmsnorm_aiter.py | Update comment references after benchmark path rename. |
| tests/rocm/test_quantization_segments.py | New ROCm quantization + workspace-dir tests. |
| tests/rocm/test_prefill_decode_dispatch.py | Update comment references after renames. |
| tests/rocm/test_norm.py | Update benchmark path reference. |
| tests/rocm/test_modules_hip_codegen.py | New GPU-free HIP JIT URI/generator behavior tests. |
| tests/rocm/test_logits_processor.py | Update comment reference after renames. |
| tests/rocm/test_logits_cap.py | New ROCm logits-soft-cap conformance tests. |
| tests/rocm/test_jit_env.py | New GPU-free “import flashinfer without visible GPU” tests. |
| tests/rocm/test_fused_moe_guards.py | New fused-MoE AITER guard tests. |
| tests/rocm/test_force_cta_tile_q.py | New subprocess tests for CTA_TILE_Q override env var. |
| tests/rocm/test_customize_prefill_use_softmax.py | Update comment references after renames. |
| tests/rocm/test_customize_prefill_signature.py | New ROCm-vs-upstream signature parity tests for custom prefill. |
| tests/rocm/test_comm_import_gate.py | New tests for CUDA-only comm module ImportError gating on ROCm. |
| tests/rocm/test_arch_caps.py | Update comment references after file renames. |
| tests/rocm/test_append_paged_mla_kv_cache.py | New ROCm dtype-coverage test for MLA append. |
| tests/rocm/test_amd_coverage.py | Update temporary-tree layout expectations for csrc/rocm. |
| tests/rocm/test_aiter_version_gate.py | Update AITER header path references after header move. |
| tests/rocm/test_aiter_source_probes.py | New GPU-free AITER probe/lookup tests. |
| tests/rocm/test_aiter_jitspec_refresh.py | New tests for AITER JIT link-line refresh + lock behavior. |
| tests/rocm/test_activation.py | New native activation correctness + routing tests. |
| tests/rocm/test_activation_aiter.py | New AITER activation backend tests + backend validation. |
| scripts/gen_arch_support_matrix.py | Update doc reference to new test location. |
| scripts/amd_coverage.py | Switch measured C++ surface root from flashinfer/csrc/rocm → csrc/rocm. |
| README.md | Update ROCm test/benchmark/profiler paths and wording. |
| pyproject.toml | Update pytest testpaths to tests/rocm and fix inline comments. |
| profiler/rocm/rocm_profiler.py | Relocated ROCm profiler module under profiler/rocm. |
| MANIFEST.in | Remove explicit graft of generated flashinfer/csrc/rocm; adjust pruning. |
| include/flashinfer/rocm/vec_dtypes.h | Update tripwire error message + rename include usage. |
| include/flashinfer/rocm/utils.cuh | Update tripwire error message; contains CTA_TILE_Q override parser. |
| include/flashinfer/rocm/sampling.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/quantization.cuh | Update tripwire error message. |
| include/flashinfer/rocm/mma.h | Add tripwire vs upstream mma; rename namespace mma_hip → mma. |
| include/flashinfer/rocm/mma_types.hpp | Rename namespace mma_hip → mma. |
| include/flashinfer/rocm/memory_ops.h | New/renamed ROCm memory ops header (drops _hip suffix). |
| include/flashinfer/rocm/math.h | Update tripwire error message; header renamed (drops _hip suffix). |
| include/flashinfer/rocm/layout.cuh | Update tripwire error message. |
| include/flashinfer/rocm/fastdiv.cuh | Update tripwire error message. |
| include/flashinfer/rocm/exception.h | Update tripwire error message. |
| include/flashinfer/rocm/attention/variants.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/attention/variant_helper.cuh | Update tripwire error message. |
| include/flashinfer/rocm/attention/state.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/attention/scheduler.cuh | Update tripwire error message. |
| include/flashinfer/rocm/attention/prefill.cuh | Update tripwire error message; update MMA/memory/math includes + namespace. |
| include/flashinfer/rocm/attention/pos_enc.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/attention/pod.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/attention/permuted_smem.cuh | Update tripwire error message; update MMA/memory include + namespace. |
| include/flashinfer/rocm/attention/page.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/attention/norm.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/attention/heap.h | Update tripwire error message. |
| include/flashinfer/rocm/attention/frag_layout_swizzle.cuh | Update tripwire error message. |
| include/flashinfer/rocm/attention/default_prefill_params.cuh | Update tripwire error message. |
| include/flashinfer/rocm/attention/default_decode_params.cuh | Update tripwire error message. |
| include/flashinfer/rocm/attention/decode.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/attention/cascade.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/attention/batch_pod.cuh | Update tripwire error message; update includes to renamed intrinsic headers. |
| include/flashinfer/rocm/attention/attention_impl.cuh | Update tripwire error message. |
| include/flashinfer/rocm/attention/allocator.h | Update tripwire error message. |
| include/flashinfer/rocm/attention/aiter/single_prefill.cuh | Update include paths for moved AITER headers. |
| include/flashinfer/rocm/attention/aiter/mha_batch_prefill_args.h | New vendored AITER batch-prefill args struct header. |
| include/flashinfer/rocm/attention/aiter/batch_prefill.cuh | Update include paths for moved AITER headers. |
| include/flashinfer/rocm/attention/aiter/batch_decode.cuh | Update include paths for moved AITER headers. |
| include/flashinfer/rocm/attention/aiter/aiter_loader.h | New AITER loader interface header under ROCm include tree. |
| include/flashinfer/rocm/attention/activation.cuh | Update comment path for moved launcher source file. |
| flashinfer/rocm/norm.py | Update benchmark path in selector comment. |
| flashinfer/hip_utils.py | Update doc reference to new test location. |
| flashinfer/arch_caps.py | Update doc references to renamed tests. |
| flashinfer/aiter_utils.py | Update doc path for moved vendored structs directory. |
| docs/rocm/backends.md | Update paths to moved C++/headers/tests/benchmarks. |
| csrc/rocm/single_prefill.cu | ROCm single-prefill implementation moved to repo-root csrc/rocm. |
| csrc/rocm/single_prefill_kernel_inst.jinja | ROCm single-prefill template instantiations moved to csrc/rocm. |
| csrc/rocm/single_prefill_jit_pybind.cu | ROCm single-prefill pybind fragment moved to csrc/rocm. |
| csrc/rocm/single_prefill_customize_config.jinja | Update math include after intrinsic header rename. |
| csrc/rocm/single_prefill_aiter.cu | Update include path to moved AITER headers. |
| csrc/rocm/single_prefill_aiter_jit_pybind.cu | New/relocated pybind fragment for AITER single-prefill. |
| csrc/rocm/single_decode.cu | ROCm single-decode implementation moved to repo-root csrc/rocm. |
| csrc/rocm/single_decode_kernel_inst.jinja | ROCm single-decode template instantiations moved to csrc/rocm. |
| csrc/rocm/single_decode_jit_pybind.cu | ROCm single-decode pybind fragment moved to csrc/rocm. |
| csrc/rocm/single_decode_customize_config.jinja | Update math include after intrinsic header rename. |
| csrc/rocm/runtime_utils.h | New/relocated DLL visibility helper for ROCm C++ tree. |
| csrc/rocm/rope_aiter.cu | ROCm AITER RoPE entry point under repo-root csrc/rocm. |
| csrc/rocm/rope_aiter_jit_pybind.cu | ROCm AITER RoPE pybind fragment under csrc/rocm. |
| csrc/rocm/renorm.cu | ROCm sampling renorm entry points under repo-root csrc/rocm. |
| csrc/rocm/quantization.cu | ROCm quantization bindings under repo-root csrc/rocm. |
| csrc/rocm/pytorch_conversion_utils.h | New/relocated small PyTorch conversion helpers under csrc/rocm. |
| csrc/rocm/pod_kernel_inst.jinja | ROCm POD instantiations under repo-root csrc/rocm. |
| csrc/rocm/pod_jit_pybind.cu | ROCm POD pybind fragment under repo-root csrc/rocm. |
| csrc/rocm/pod_customize_config.jinja | Update math include after intrinsic header rename. |
| csrc/rocm/page_aiter_jit_pybind.cu | ROCm AITER page-append pybind fragment under csrc/rocm. |
| csrc/rocm/norm_aiter.cu | ROCm AITER norm entry points under repo-root csrc/rocm. |
| csrc/rocm/norm_aiter_jit_pybind.cu | ROCm AITER norm pybind fragment under csrc/rocm. |
| csrc/rocm/fused_moe_aiter_jit_pybind.cu | ROCm fused-MoE AITER pybind fragment under csrc/rocm. |
| csrc/rocm/flashinfer_sampling_binding.cu | ROCm sampling TORCH_LIBRARY bindings under csrc/rocm. |
| csrc/rocm/flashinfer_rope_binding.cu | ROCm RoPE TORCH_LIBRARY bindings under csrc/rocm. |
| csrc/rocm/flashinfer_quantization_binding.cu | ROCm quantization TORCH_LIBRARY bindings under csrc/rocm. |
| csrc/rocm/flashinfer_page_binding.cu | ROCm page-cache TORCH_LIBRARY bindings under csrc/rocm. |
| csrc/rocm/flashinfer_norm_binding.cu | ROCm norm TORCH_LIBRARY bindings under csrc/rocm. |
| csrc/rocm/flashinfer_cascade_binding.cu | ROCm cascade TORCH_LIBRARY bindings under csrc/rocm. |
| csrc/rocm/batch_ragged_prefill_aiter.cu | Update include path to moved AITER headers. |
| csrc/rocm/batch_ragged_prefill_aiter_jit_pybind.cu | ROCm AITER ragged batch-prefill pybind fragment under csrc/rocm. |
| csrc/rocm/batch_prefill_ragged_kernel_inst.jinja | ROCm batch ragged prefill instantiations under csrc/rocm. |
| csrc/rocm/batch_prefill_paged_kernel_inst.jinja | ROCm batch paged prefill instantiations under csrc/rocm. |
| csrc/rocm/batch_prefill_paged_aiter.cu | Update include path to moved AITER headers. |
| csrc/rocm/batch_prefill_paged_aiter_jit_pybind.cu | ROCm AITER paged batch-prefill pybind fragment under csrc/rocm. |
| csrc/rocm/batch_prefill_jit_pybind.cu | ROCm batch-prefill pybind fragment under csrc/rocm. |
| csrc/rocm/batch_prefill_customize_config.jinja | Update math include after intrinsic header rename. |
| csrc/rocm/batch_prefill_aiter_customize_config.jinja | New/relocated AITER customize config template under csrc/rocm. |
| csrc/rocm/batch_pod_kernel_inst.jinja | ROCm batch POD instantiations under csrc/rocm. |
| csrc/rocm/batch_pod_jit_pybind.cu | ROCm batch POD pybind fragment under csrc/rocm. |
| csrc/rocm/batch_pod_customize_config.jinja | Update math include after intrinsic header rename. |
| csrc/rocm/batch_decode_kernel_inst.jinja | ROCm batch decode instantiations under csrc/rocm. |
| csrc/rocm/batch_decode_jit_pybind.cu | ROCm batch decode pybind fragment under csrc/rocm. |
| csrc/rocm/batch_decode_customize_config.jinja | Update math include after intrinsic header rename. |
| csrc/rocm/batch_decode_aiter.cu | Update include path to moved AITER headers. |
| csrc/rocm/batch_decode_aiter_jit_pybind.cu | ROCm AITER batch-decode pybind fragment under csrc/rocm. |
| csrc/rocm/aot_extension_utils.h | ROCm dispatch/extension helper header under csrc/rocm. |
| csrc/rocm/aiter_tensor_compat.h | New at::Tensor ↔ aiter_tensor_t adapter header under csrc/rocm. |
| csrc/rocm/aiter_loader.cc | Update include path + update message path reference to new csrc/rocm. |
| csrc/rocm/activation.cu | ROCm native activation implementation under repo-root csrc/rocm. |
| csrc/rocm/activation_aiter.cu | ROCm AITER activation entry point under csrc/rocm. |
| csrc/rocm/activation_aiter_jit_pybind.cu | ROCm AITER activation pybind fragment under csrc/rocm. |
| CLAUDE.md | Update worktree recipe to add flashinfer/csrc/rocm symlink. |
| benchmarks/rocm/testlist_rocm.txt | Rename/retarget ROCm benchmark testlist path. |
| benchmarks/rocm/bench_silu_and_mul.py | Retarget run instructions + profiler import path. |
| benchmarks/rocm/bench_rope.py | Retarget run instructions + profiler import path. |
| benchmarks/rocm/bench_norm.py | Retarget run instructions after directory rename. |
| benchmarks/rocm/bench_mla.py | Retarget run instructions + profiler import path. |
| benchmarks/rocm/bench_fused_moe_aiter.py | Retarget run instructions + profiler import path. |
| benchmarks/rocm/bench_fa2_prefill.py | Retarget run instructions + profiler import path. |
| benchmarks/rocm/bench_decode_graph.py | Retarget run instructions after directory rename. |
| benchmarks/rocm/bench_batch_decode.py | Retarget run instructions + profiler import path. |
| benchmarks/rocm/bench_aiter_prefill.py | Retarget run instructions + profiler import path. |
| benchmarks/README.md | Retarget ROCm benchmark/profiler references. |
| .pre-commit-config.yaml | Retarget reuse checking file patterns after header moves. |
| .gitignore | Ignore generated flashinfer/csrc/ and update ROCm benchmark output note. |
| .github/workflows/arch-caps-conformance.yml | Retarget hardware-less CI to tests/rocm/* paths. |
| .claude/skills/pr-workflow/SKILL.md | Update worktree setup recipe for new flashinfer/csrc/rocm symlink. |
| .claude/skills/code-coverage/SKILL.md | Retarget test path references after directory rename. |
| .claude/skills/benchmark-kernel/SKILL.md | Retarget benchmark/profiler paths after directory rename. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Drops the .dev suffix added when this moved out of .devcontainer/. The suffix was chosen to mirror the CUDA pair, where Dockerfile.cu130 is the runtime image and Dockerfile.cu130.dev is what the devcontainer builds. That mapping does not hold here: there is no runtime Dockerfile.rocm for it to be distinguished from, and the ROCm CI image is already spelled Dockerfile.rocm_ci rather than Dockerfile.rocm.ci, so .dev matched neither neighbour. Only reference is CONTRIBUTING.md:19, updated. docker/ is pruned from the sdist, so packaging is unaffected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Aligns the fork's C++ and directory layout with upstream's: ROCm sources move from
flashinfer/csrc/rocm/to the repo-rootcsrc/rocm/, the last fork-owned headers leave an upstream-owned directory, and therocm_tests/rocm_benchmarks/rocm_profilertrees adopt thedirectory/rocm/convention already used bydocs/rocm/andinclude/flashinfer/rocm/. No kernel code changes; the installed wheel is unchanged apart from header renames.What changed
Layout
csrc/rocm/— the 65 ROCm.cu/.cc/.h/.jinjasources, moved fromflashinfer/csrc/rocm/. Upstream keeps C++ at the repo root (850 files undercsrc/against 23 inside the package); this fork had it the other way round.csrc/rocm/has zero upstream files, so the additive-only policy holds exactly as before.include/flashinfer/rocm/attention/aiter/— the 6 AITER headers, moved out ofinclude/flashinfer/attention/aiter/. Upstream owns zero files at the old path, and these were the only non-rocm/flashinfer headers any ROCm TU included, so "no ROCm TU reaches an upstream-owned header" is now true by construction rather than by inspection.tests/rocm/,benchmarks/rocm/,profiler/rocm/— renamed fromtests/rocm_tests/,benchmarks/rocm_benchmarks/,rocm_profiler/. All three destinations are collision-free upstream;profiler/already exists here as an upstream directory, which is why the profiler moves under it rather than staying at the root.docker/Dockerfile.rocm— moved from.devcontainer/rocm/Dockerfile, which was never a devcontainer: it had nodevcontainer.json, unlike itscu126–cu130siblings, and its only use wasdocker build -f. The sibling convention is the inverse (it sits besideDockerfile.rocm_ci, the CI counterpart).Naming
_hipsuffix from 4 intrinsic headers (math.h,memory_ops.h,mma.h,vec_dtypes.h), 46 test files and 3 benchmarks. The suffix separated a backend implementation from a same-stem wrapper beside it; refactor(rocm): remove the gpu_iface layer and flatten the ROCm headers #329 deleted the wrappers, and the directory already says ROCm.flashinfer::mma_hip→flashinfer::mma, now that it carries a tripwire instead of relying on the name for separation.Build
build_backend_rocm.py— generalized from managing one generated tree to a_trees()table of two:flashinfer/includefrominclude/, andflashinfer/csrc/rocmfromcsrc/rocm/. Same three modes as before (symlink for editable, filtered copy for wheel, cleared for sdist).MANIFEST.in—recursive-include flashinfer/csrc/rocmdeleted rather than repointed: setuptools-scm's file finder already ships every tracked file, so the root tree needs no graft.Architecture / design notes
The wheel does not change, and that is the design. Only the source moves; the materialized destination stays
flashinfer/csrc/rocm, soget_csrc_dir()returns the same path,flashinfer.get_csrc_dir/get_includekeep their contract, and thepackage-dataglobs are untouched. Verified by full inventory diff against a wheel built from the parent commit — 431 members both sides, thecsrcset byte-identical at 65 files, and the only delta is the header renames.Upstream's
flashinfer/data/staging was evaluated and rejected. Mirroring theirpackage-dirremap would shipbuild_backend.py,build_backend_rocm.pyandbuild_utils.pyinto the wheel asflashinfer/data/*.py(distutils globs*.pyin each declared package dir), change a public API return value, and reintroduce the persistent shadowing copies this fork's_restoring_pkg_treesdeliberately avoids. It buys convergence of two delegating lines inenv.py. Extending the existing mechanism to a second tree costs none of that.The duplicate-guard tripwires were wrong in two ways. 29 of 30 claimed "... and ... both define FLASHINFER_X_", but the fork header defines
FLASHINFER_ROCM_*— a reader would hunt for a#definethat isn't there. Andmmahad no tripwire at all; its only#erroris an unrelated GFX-arch guard, somma.cuhandrocm/mma.hwere kept apart by the namespace name alone.Worth recording explicitly, because it is the obvious "improvement" someone will propose: defining upstream's guard in the fork header is actively harmful. It makes upstream's
#ifndefskip its entire body, which is the silent shadow #329 removed. Measured —#define FLASHINFER_EXCEPTION_H_then including upstream's header yieldserror: use of undeclared identifier 'flashinfer'. The reverse include order is not silent regardless; the compiler rejects it asredefinition of 'Error', just without naming the two files. Closing that gap properly would mean editing upstream headers, which additive-only rules out.CONTRIBUTING.mdnow says so.CODEOWNERS deliberately untouched.
csrc/rocm/nominally inheritscsrc/ @wenscarl @yzh119 ..., but the file is inert on this fork — GitHub reports 137 "Unknown owner" errors (every entry), and PRs #328, #329 and #331 all touchedcsrc/orinclude/flashinfer/and auto-requested zero reviewers. Addingrocm/lines would be a no-op enlarging a file that is pure rebase surface.Test plan
test_fused_moe_aiter.py:282gating ondevice_count() < 2FLASHINFER_ROCM_ARCH_LIST="gfx942,gfx950"— the real assertion for a path change, since a brokenFLASHINFER_CSRC_DIRfails at compile timeflashinfer/data/entriescsrc/rocm/(65 files), and a wheel built from it has all 65exception.h/allocator.has the positive control (upstream'smath.cuh/mma.cuhabort on missingcuda_fp16.hbefore a guard can fire)upstream_canary.py— 25 forked headers / 21 ROCm-only, unchanged by the renamespre-commit run -aTwo verification steps were A/B'd because they would otherwise have passed vacuously:
_isolated_projectdid not copy rootcsrc/(the wheel assertion would have compared 0 to 0), andtest_editable_symlink_is_relativeonly covered the include tree, so an absolute csrc link passed all 29 tests. Both now fail when the thing they check is broken.Reviewer notes
Clear
~/.cache/flashinferbefore testing this branch.JitSpec.build()only writesbuild.ninjawhen absent, so a warm cache keeps-Iflags pointing at pre-move paths.Existing editable installs need a reinstall, and a fresh worktree now needs a second symlink —
CLAUDE.mdand thepr-workflowskill carry the updated recipe in the same commit as the move, since a worktree made from the old recipe will not build. Linkflashinfer/csrc/rocm, neverflashinfer/csrc, or upstream's 850-file CUDA tree lands inside the package.Checking out across this commit destroys the generated
flashinfer/csrc/rocm— git deletes it while the pre-move side has it tracked, and does not restore the symlink coming back. Re-run the recipe after any such checkout or rebase. Two traps compound it:jit/core.pyrecreates the directory viamakedirs(..., exist_ok=True)at import, so a laterln -ssilently lands inside it rather than replacing it (clear the path first), and a cachedbuild.ninjastill naming the vanished file fails asninja: error: ... missing and no known rule to make itrather than anything naming the symlink.tests/rocm/test_rope.pyandCHANGELOG.mdappear in the diff only as renames/reverts and are byte-identical to their base contents — a sed reached into function names and historical entries, and the last commit undoes that.Rebased onto
94ac35dc7after #332 landed. That PR addsbenchmarks/rocm_benchmarks/bench_norm.pyinto a directory this branch renames, so git reports it as a file-location conflict; the file is carried tobenchmarks/rocm/bench_norm.pyand the four fresh references to the old path it introduced are retargeted. The single content conflict is a comment intest_norm.py, resolved in favour of #332's newer wording since it describes the native default that PR introduced. The suite numbers above were measured at0a92383c8, the pre-rebase tip; the rebase carried only these path fixes and the one comment.