Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are build-detection/compilation-guard issues in the new ROCm linking path that can cause ROCm builds to incorrectly include CUDA headers or link against the wrong runtime.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a native ROCm/HIP execution path targeting AMD gfx1151 (Strix Halo / Radeon 8060S) while keeping NVIDIA CUDA behavior as a separately gated path, including build/link fixes, runtime feature gating, and validation docs.
Changes:
- Add explicit HIP/ROCm runtime detection to prevent
gfx11xxcapability tuples from being treated as NVIDIA SM versions and to disable CUDA-only optional backends on HIP. - Extend native/JIT paths (TVM JIT matchers, fast index copy, Triton kernels, GGUF JIT) to accept ROCm devices and avoid CUDA-only flags/launch kwargs/PTX.
- Add regression tests plus detailed LAN-223 validation and ROCm deployment documentation.
File summaries
| File | Description |
|---|---|
| tests/utils/test_rocm_runtime.py | Adds regression tests for HIP gating and CUDA-only optional backend suppression. |
| tests/kernels/test_pinned_tensor.py | Adjusts UVA identity test behavior for HIP runtime semantics. |
| setup.py | Links native extensions against HIP runtime when ROCm is detected; skips nvcc toolchain checks on ROCm. |
| python/freetoken/utils/arch.py | Adds is_rocm_runtime() and gates CUDA capability queries on non-HIP builds. |
| python/freetoken/utils/init.py | Exports is_rocm_runtime from utils. |
| python/freetoken/moe/fused_nvfp4.py | Routes HIP prefill to safer serial Triton NVFP4 path. |
| python/freetoken/kernel/utils.py | Drops nvcc-only flags when compiling under HIP. |
| python/freetoken/kernel/triton/norm.py | Avoids passing CUDA-only launch_pdl kwarg to AMD Triton backend. |
| python/freetoken/kernel/triton/e4m3_compat.py | Forces e4m3 “native” detection off on HIP to avoid tuple false-positives. |
| python/freetoken/kernel/triton/attention.py | Adjusts HIP decode tiling to satisfy RDNA WMMA constraints. |
| python/freetoken/kernel/triton/activation.py | Avoids inlining PTX on HIP and avoids CUDA-only launch kwargs. |
| python/freetoken/kernel/gguf.py | Adds ROCm fallback discovery for Thrust headers and libamdhip64.so linker path. |
| python/freetoken/kernel/csrc/pinned_tensor.cpp | Switches pinned tensor extension to HIP/CUDA compat shim header. |
| python/freetoken/kernel/csrc/jit/store.cu | Allows ROCm tensors in TVM JIT store path device checks. |
| python/freetoken/kernel/csrc/jit/index.cu | Allows ROCm tensors in TVM JIT index path device checks. |
| python/freetoken/kernel/csrc/jit/fast_index_copy.cuh | Adds HIP-safe load/store fallbacks and broadens accepted device types. |
| python/freetoken/kernel/csrc/include/freetoken/utils.cuh | Adds HIP runtime includes/aliases and HIP LaunchKernel implementation. |
| python/freetoken/kernel/csrc/hip_compat.h | Introduces CUDA-runtime API name shims for HIP linking in host C++ extensions. |
| python/freetoken/kernel/csrc/gguf/dispatch.h | Fixes HIP shuffle mask width expectations by widening masks for HIP. |
| python/freetoken/kernel/csrc/cpu_moe/cpu_moe_ext.cpp | Switches CPU MoE extension to HIP/CUDA compat shim header. |
| python/freetoken/kernel/backend.py | Adds HIP detection and disables CUDA-only optional backend probes on ROCm. |
| python/freetoken/engine/engine.py | Skips optional Triton prefill warmup by default on ROCm unless explicitly enabled. |
| pyproject.toml | Loosens torch/triton constraints and adds ROCm classifier. |
| docs/lan223-rocm-validation-2026-08-28.md | Records LAN-223 reproducible ROCm validation evidence and known limitations. |
| docs/amd-rocm-gfx1151.md | Adds ROCm/GFX1151 port documentation and validation procedure. |
| .gitignore | Ignores ROCm hipify-generated artifacts (*.hip, *_hip.*). |
Review details
- Files reviewed: 25/26 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Lets pinned_tensor.cpp and cpu_moe_ext.cpp call the CUDA Runtime API names they | ||
| // were written against while actually linking HIP on ROCm builds. Only the calls | ||
| // those two files use are covered -- this is not a general CUDA/HIP compat layer. | ||
| #if defined(__HIP_PLATFORM_AMD__) || defined(__HIPCC__) |
There was a problem hiding this comment.
Fixed in 54d6ab2. setup.py now passes FREETOKEN_USE_ROCM=1 to both host extensions whenever the active PyTorch build reports torch.version.hip. hip_compat.h selects HIP when that explicit macro is present, in addition to the HIP compiler macros. Verified on LAN-223: setup.py build_ext --inplace compiled and linked both _pinned_tensor and _cpu_moe against libamdhip64.
|
|
||
| ROOT = Path(__file__).parent | ||
| IS_ROCM = CUDA_HOME is None and ROCM_HOME is not None |
There was a problem hiding this comment.
Fixed in 54d6ab2. IS_ROCM now derives solely from torch.version.hip rather than CUDA_HOME/ROCM_HOME precedence, so a CUDA toolkit installed beside an active HIP PyTorch build cannot select cudart or trigger nvcc checks. Added regression coverage and verified 12 focused ROCm runtime and pinned-extension tests on LAN-223, plus a successful native host-extension build.
d6531ff to
b8e48bd
Compare
|
Follow-up GMKtec EVO-X2 validation evidence for the current PR head:
Detailed reproducible artifacts and boundaries are recorded in |
|
Validation update for
The commit adds GMKtec EVO-X2 validation documentation, reproducible local runner controls, and regression coverage. It does not include private model files or host artifacts. |
|
Validation follow-up: full-context MoE cache telemetry is now recorded in commit A temporary loopback-only Qwen instance enabled This supports the prior measured rejection of a larger static cache: reducing misses by raising cache capacity did not improve sustained end-to-end TPS. The normal no-counter Qwen service was immediately restored and passed the deterministic AIME quality gate with the required SHA-1 The report documents the exact methodology, limitation, and GMKtec EVO-X2 artifact root. This is evidence for the native AMD port, not a claim of quantization-equivalent parity with the separate Q4_K_M llama.cpp control. |
|
Exact-format ROCm comparison update in commit The branch now serves the exact same Matched protocol: same model file, same host and GPU, auto DPM policy, 8,192-token context, one request, same fixed prompt, greedy decoding, 256 requested output tokens, warmup, and three scored samples.
The fresh same-format difference is 0.680 TPS, or 1.39% in favor of this llama.cpp control. This removes the previous NVFP4-versus-Q4_K_M representation mismatch. FreeToken is near parity but does not yet meet or exceed llama.cpp under this strict workload. A temporary The normal NVFP4 FreeToken endpoint was restored after the time-share test and passed its deterministic AIME hash gate. The report includes artifact locations, startup geometry, the 512-token raw-prompt check, and the full method. |
|
Final GMKtec EVO-X2 native ROCm/HIP Q4 recovery evidence is now on commit a937862.
The matched exact-Q4 comparison remains transparent: FreeToken recovery configuration was 47.960 TPS versus fresh llama.cpp ROCm 10 at 48.831 TPS, 1.78 percent lower. This PR does not claim throughput parity where the measured result is below it. Raw artifacts and the machine-checkable endurance summary are documented in docs/gmktec-evo-x2-rocm-validation-2026-08-30.md. |
8dbcb19 to
a937862
Compare
What: - Remove .agents/learnings and .plans/rocm-consolidation files from the branch. - Remove internal increment and plan-path references from source comments and public installation docs. - Keep implementation comments that explain correctness, ownership, profiler intent, source attribution, or ROCm safety behavior. - Clarify public ROCm documentation: gfx1100 has recorded serving smoke on ROCm 7.2.1; the ROCm 7.14.x container is a reference environment, and other target cells remain compile-only until physical serving evidence exists. Why: - Keep merge surface focused on code, tests, reproducibility tooling, and user-facing documentation. - Prevent private planning history, review workflow language, stale plan paths, and local process notes from entering the upstream repository. - Avoid presenting compile success or a reference container as cross-target serving or performance proof. Related upstream work informing this branch: - PR FlashML-org#132: portable ROCm/HIP foundation. - PR FlashML-org#133: TVM-FFI index/store portability. - PR FlashML-org#135: RCCL tensor-parallel communication. - PR FlashML-org#136: native GGUF ROCm build and Q4_0 kernels. - PR FlashML-org#137: earlier AMD serving bring-up. - PR FlashML-org#217: source-fork ROCm, Qwen3.5 GGUF, and performance experiments. - PR FlashML-org#241: gfx1150 build, JIT, Triton, and attention hardening. - PR FlashML-org#260: gfx1151 validation and fallback/build evidence. - PR FlashML-org#316: HIP graph-capture-safe expert copies. - PR FlashML-org#378: CPU/Hybrid MoE graph replay safety. - Local branch milestones: 436263f, 926c1e8, e1d1856, 8a70c7e, and e5fd30f. Evidence: - 170 focused tests passed after cleanup. - gfx1100 is the only target with end-to-end Qwen3.5 GGUF serving smoke recorded here. - Remaining matrix targets are compile-only; no new throughput claim is published without a matching A/B manifest.
08b5e3e to
9bdb73b
Compare
|
Tried this branch on a different ROCm target than the one validated here — a discrete gfx1011 card (AMD Radeon Pro V520 / BC-160, Navi12) instead of the gfx1151 APU (GMKtec EVO-X2) this PR's validation evidence above is based on. Wanted to share what I found in case it's useful, since the code paths that broke are architecture-agnostic (not gfx1151-specific). Setup: 1× BC-160 (8GB), AMD TheRock nightly ROCm/PyTorch ( Bugs found getting it to load (all reproduce regardless of GPU arch, I think):
gfx1011-specific / hardware finding: Where I got stuck: after all of the above plus Happy to share the exact patch diffs if useful — didn't want to open a PR against a WIP branch without checking first. |
|
Rebase update:
Focused local validation:
HIP safety and qualification boundary:
|
|
Follow-up after deeper isolation (rounds 2-6, same BC-160/gfx1011 host as above). Ruled out further:
Narrowed to the GPU-offload routed-expert path on decode: layer-by-layer instrumentation (NaN/absmax after each sublayer, and separately for router/shared-expert/routed-expert inside The decode admission path ( Opened a separate issue against Filing this as additional evidence rather than a fix — stopping active work here since it's now outside what source-reading/tracing can resolve without live kernel-level debugging ( |
# Conflicts: # pyproject.toml
|
Validation follow-up for commit
GitHub currently reports no check runs or status contexts for this head. No merge or new parity claim is implied. |
Summary
Adds and validates a native ROCm/HIP execution path for AMD Radeon 8060S
gfx1151, while retaining NVIDIA CUDA as a separate execution path.The branch includes the upstream ROCm work plus focused fixes from full-model validation on the GMKtec EVO-X2:
gfx1151cannot be treated as an NVIDIA compute capability.Validation
Validated on a GMKtec EVO-X2 with AMD Radeon 8060S
gfx1151, PyTorch2.13.0+rocm10.0.0, and HIP7.15.26333:Detailed source, build, validation, quality, and performance boundaries are recorded in the repository documentation and retained raw artifacts.
Scope
This PR does not add llama-swap integration. CUDA graph capture remains disabled for the HIP MVP. Performance claims remain limited to the documented same-model, same-format controls and do not claim parity where the evidence does not prove it.