Repository navigation
The internal HIP AllReduce is a stub — injected a direct-P2P version, +7–12% decode on 2× RDNA4 #25197
JohnTDI-cpu
started this conversation in
Show and tell
Replies: 1 comment
|
@JohnTDI-cpu I have 2x R9700. Do you know of any issues running tensor split with ROCM on Ubuntu 24.04 server? Do you know if this will run with a Pytorch/ROCM container and a clean build of llama.cpp (latest). If I can get clean baseline data with tensor split I could also test your changes. Thanks. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
The internal HIP AllReduce is a stub — injected a direct-P2P version, +7–12% decode on 2× RDNA4
Was tinkering with tensor-parallel on my 2× R9700 box and noticed something: the internal
NCCL-free AllReduce (
ggml/src/ggml-cuda/allreduce.cu) only exists for CUDA/MUSA — on HIPit's a stub, so
--split-mode tensorquietly falls back to the meta-backend butterfly(host-staged, so the partial crosses PCIe twice). Figured I'd try a direct peer-to-peer
version for the HIP branch. Turned out to be a nice little decode win with perplexity
unchanged, so sharing it here as a data point. Big caveat up front: this is just me
experimenting on a single 2× gfx1201 box, so take the numbers with a grain of salt — I could
be missing something. Not looking to push a PR, just curious if it's useful / holds up
elsewhere.
The numbers (
llama-bench -sm tensor -fa on -n 128 -r 5, decode tg128 t/s)2× Radeon AI PRO R9700 (gfx1201, RDNA4), ROCm 7.2.4, latest
master. Two model sizes:Smaller model gains a bit more over the butterfly — faster decode means the AllReduce
latency is a bigger slice, so the lower-latency direct path helps relatively more.
Quality: bit-identical reduction (not just "close")
On the decode path (where the P2P kernel actually runs — forced with
-ub 1) the reductionis bit-identical to the butterfly: PPL 6.0464 == 6.0464 (Qwen-27B, 8 chunks). Which
makes sense — it's just an fp32 sum of two partials, same as the butterfly's ADD. Prefill
stays on the butterfly (see size dispatch below), so it's identical to stock there too. The
tiny ~0.02% gap vs single-GPU (27B 6.9186/6.9199, 11B 9.1844/9.1869) is just fp
non-associativity of splitting the matmul — any tensor split has it, butterfly included.
What it actually does
For batch-1 decode the butterfly's host staging is the slow part. The HIP version does a
one-shot direct P2P instead: each rank writes its partial into a peer-accessible VRAM
buffer, signals arrival with a token in the peer's VRAM inbox (then spins on its own
inbox — local reads, no PCIe polling), and reads the peer's partial straight over PCIe
(one traversal) and sums. Falls back to the butterfly cleanly if P2P isn't available
(
pipeline_initreturnsnullptr).Size dispatch: only small (decode) reductions use the P2P kernel; large ones (prefill
ubatch, MBs) return
falseand go to the butterfly — the P2P kernel doesn't overlap thetransfer with compute, so it'd regress prefill otherwise. Prefill throughput stays at stock.
One footgun I hit:
hipDeviceEnablePeerAccessreturnshipErrorPeerAccessAlreadyEnabledand leaves a sticky last-error, which a later
hipGetLastError()trips on and aborts anunrelated kernel (shows up as a random "ROCm error" mid-run). One line:
(void) hipGetLastError();.Want to try it? (and please do, if you have the hardware)
It's a swap of just the HIP branch in
allreduce.cu(the CUDA part is byte-identical toupstream, so it applies cleanly — one file). I put the patch + the ready file + these numbers
in a small standalone repo so it's easy to try: https://github.com/JohnTDI-cpu/llama-hip-p2p-allreduce. Then:
(Use
HIP_VISIBLE_DEVICES, not-dev—-devsweeps per-device and you end up measuring1-GPU metas instead of real 2-GPU TP. Took me a while to notice that one.)
If you've got a 2× RDNA3/4 setup (R9700 / 7900 XTX / W7900 / dual RX 7900 / etc.) I'd be
really grateful for a data point from your box — this is only tested on my gfx1201 pair so
far. Just build the branch and paste your
tg128(stock-sm tensorvs patched) and a PPLnumber. Most curious whether the direct-P2P +
__threadfence_systembehaves on other RDNAparts, and whether P2P is even enabled on your board (
rocminfo/ it falls back to butterflyif not). Any result — win, no-change, or breakage — is useful.
Fair warning / scope
memory-model assumptions aren't verified on CDNA either.
+7–12% over the butterfly + not crashing the internal-AllReduce path + no prefill hit.
→ 29.8); the ROCm direct-P2P scales instead. Different backend though.
Not a PR (yet) — on purpose
I didn't want to drop review work on the maintainers for something I've only run on a single
2× gfx1201 box. So this is a heads-up + a repo to try, not a merge request. If a few people
(ideally other RDNA3/4) can confirm it holds up, I'll clean it into a proper PR. For now:
is this useful / worth pursuing? Any red flags you'd expect with direct-P2P on other AMD
archs? Repo + patch linked above, and I'm happy to do the PR legwork once it's had some
independent testing.
All reactions