Support RDNA3.5 (gfx1151 / Strix Halo) builds: wave32 kernels + vendored-glm/HIP fixes#17
Open
warrenrross wants to merge 1 commit into
Open
Conversation
…IP fixes Arch-gate the warp size (__gfx9__ => 64, else 32) via a single WARP_SIZE constant threaded through the reduction/tiling kernels, and add an `if constexpr` guard so the wave64-only bs64 backward kernel is never instantiated on a wave32 device (where rocprim's hardcoded size-64 warp reduce silently no-ops -> wrong color/SH gradients, no error). Fix the HIP glm plumbing: vendored-glm include dir + -DTORCH_HIP_VERSION=8000. Zero-regression for CDNA (wave64) -- same source compiles byte-identical device code there. Validated on gfx1151 (RDNA3.5): build + import, rasterize+backward smoke, gradcheck vs the fork's torch reference (max abs err 2.0e-6), and a 7000-step simple_trainer run. Human-guided, agent-authored. Co-Authored-By: Claude Opus 4.8 <[email protected]>
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.
Addresses #8 (gfx1151 / RDNA3.5 support); relates to #4 (7900XTX — same wave32 root cause) and #3 (glm headers).
Ready-to-use artifacts mirroring this patch (for RDNA3.5 users + reproducers):
docker pull warrenrross/gsplat-rocm:gfx1151-rocm72-trainerSummary
ROCm/gsplatcurrently targets only CDNA Instinct GPUs (MI300X/MI325X), which arewave64. On RDNA3.5 APUs (gfx1151, "Strix Halo" — AMD Ryzen AI Max), the build
fails for two unrelated reasons:
RDNA is wave32, but the kernels hardcode wavefront size 64
(
rocprim::warp_reduce<…,64>,rocprim_warpSum<64>,cg::tiled_partition<64>,(block_size+63)/64,LOGICAL_WARP_SIZE=64, manual__shfloffset=32).-mwavefrontsize64does not help (rocPRIM hardcodes wave32 for RDNA regardless;forcing
ROCPRIM_NAVI=0just trips a DPP-broadcast assert).The dangerous part is not a build failure — it is a silent correctness failure.
In
RasterizeToPixels3DGSBwd.cuthebs64kernel's DPP reduction is commented out(ROCm 6.4.1 compiler issue) and replaced by
rocprim_warpSum<64>(...). On a wave32device that hardcoded-
64rocPRIM instantiation does not error: it hitsrocPRIM's
enable_if<VirtualWaveSize <= max_size()>guard(
check_virtual_wave_size<64, size32>) and becomes a silent no-op, so the warpsum simply does not happen → wrong color/SH gradients with no error, no warning, no
crash. The build succeeds,
import gsplatsucceeds, training runs and quietlyconverges to garbage. This is the headline reason the change is a correctness fix.
The HIP build's glm handling breaks during/after hipify: the vendored-glm
include dir isn't passed to the HIP compile, hipify copies glm's
.hppheaders butnot their sibling
.inlfiles, and hipify rewrites glm'sCUDA_VERSIONgate to anundefined
TORCH_HIP_VERSIONso glm#errors out.This PR adds an RDNA3.5 build path: gate the wavefront-size literals on the target arch
(or, minimally, provide wave32 variants) and fix the HIP glm include/define plumbing.
Validated end-to-end on gfx1151 — builds cleanly (
gsplat/csrc.so;import gsplatworks), passes a GPU rasterize+backward smoke test and a numerical gradcheck against the
fork's own torch reference (max abs error 2.0e-6), and completes a 7000-step
simple_trainer.pytraining run — insiderocm/pytorch:rocm7.2.1_ubuntu24.04_py3.12_pytorch_release_2.9.1withPYTORCH_ROCM_ARCH=gfx1151.Environment
RDNA3.5, wave32, unified memory (GTT).
rocm/pytorch:rocm7.2.1_ubuntu24.04_py3.12_pytorch_release_2.9.1. (This patch keys thewarp size off the
__gfx9__arch predicate rather than the deprecated__AMDGCN_WAVEFRONT_SIZE, so it builds on current ROCm — the older "build on 7.0"constraint no longer applies. Stock ROCm ≥7.2.1 also carries the gfx1151
first-dispatch HSA fix, ROCm #5853.)
GPU_ARCHS=gfx1151 PYTORCH_ROCM_ARCH=gfx1151.Changes
1.
setup.py— pass the vendored glm include to the HIP compile.Add the vendored glm dir as the first HIP
include_dirsentry (~L229):Rationale: the fork needs its vendored glm (system
libglm-dev1.0.1 isAPI-incompatible). Without the explicit include, the hipified tree can't find glm
headers.
2.
setup.py— define-DTORCH_HIP_VERSION=8000inhipcc_flags(~L211).hipify rewrites glm's
CUDA_VERSIONcheck toTORCH_HIP_VERSION, which is undefinedwhen
glm/simd/platform.his parsed → evaluates0 < 7000→#error "GLM requires CUDA 7.0 or higher". Defining it high satisfies the gate.3. Build-doc / hipify note — complete the glm copy into the hipified tree.
hipify copies glm's
.hppbut not its sibling.inlfiles, so the HIP compile failswith
fatal error: scalar_constants.inl: No such file or directory. After the first(hipify) pass, top up the hipified glm:
Ideally this is folded into the hipify step in
setup.py(copy the whole vendored glmdir, not just hipify's header selection) so a one-pass build works. The two-pass
workaround (hipify, top-up glm, recompile) is documented for now.
4. Arch-keyed
WARP_SIZEconstant (the wave32 fix). Rather than a blind64→32flip, introduce a single compile-timeWARP_SIZEconstant ingsplat/cuda/include/Common.cuh, selected by the__gfx9__architecture predicate,and thread it through every hardcoded site. The same source then compiles to a
correct wave64 object on CDNA and a correct wave32 object on RDNA — no regression to the
Instinct line.
Why
__gfx9__, not__AMDGCN_WAVEFRONT_SIZE? The latter is deprecated as of ROCm7.0.2 and slated for removal, and
warpSizeis not a host-side constant (this value isalso read in host-side launch-sizing code). AMD's rocPRIM maintainers recommend the
__gfx9__-family predicate as the durable form(ROCm/ROCm#4121). Only gfx9 (CDNA/GCN) is
wave64; every gfx10/11/12 (RDNA) target is wave32.
WARP_SIZEis threaded through (audit IDs F2–F7):threadIdx.x % 64→% WARP_SIZE,for (i = 0; i < 64; ...)→i < WARP_SIZE,offset = 64/2→offset = WARP_SIZE/2,LOGICAL_WARP_SIZE = 64→= WARP_SIZE,(block_size + 63)/64→(block_size + WARP_SIZE - 1)/WARP_SIZE, acrossProjection2DGSFused.cu,Projection2DGSPacked.cu,ProjectionEWA3DGSFused.cu,ProjectionEWA3DGSPacked.cu,RasterizeToPixels2DGSBwd.cu,RasterizeToPixels3DGSBwd.cu,RasterizeToPixelsFromWorld3DGSBwd.cu,Utils.cuh.5.
if constexprguard for the wave64-onlybs64kernel (audit F1/F6). The 3DGSbackward dispatch is changed so the wave64-only
bs64kernel — the one whoserocprim_warpSum<64>silently no-ops on wave32 — is never instantiated on a wave32build:
This is what defuses the silent no-op: on wave32 the compiler never emits the bad
kernel, so there is no path to the corrupt gradient. The surviving
64literals (lines30, 47, 85, 86, 210 of
RasterizeToPixels3DGSBwd.cu) are intentional — they areinside the now-wave64-only
bs64kernel and must stay 64.Edit the
.cu/.cuhsources, not the generated.hip, so a fresh hipifyregenerates correct kernels.
Maintainer-friendliness note
The change is deliberately shaped to be a zero-regression add for the
officially-supported Instinct line: on a CDNA target the
__gfx9__branch resolves toWARP_SIZE == 64and theif constexprkeeps the existingbs64kernel, so thegenerated wave64 device code is byte-identical to today's. The wave32 path is purely
additive. If maintainers prefer a different surface, the same logic maps cleanly onto an
explicit
#if defined(__GFX11__) || defined(__GFX12__)block or a-DGSPLAT_WARP_SIZE=build flag — happy to reshape to fit the project's arch-dispatchstyle.
One caveat to resolve before/at merge (host/device on CDNA): the
__gfx9__macrosare undefined in the host compile pass, so host-side
WARP_SIZEresolves to 32. Thisis benign for gfx1151-only builds (host 32 == device 32) but would be a host/device
mismatch on a CDNA build. For the upstream form, derive the host value from
PYTORCH_ROCM_ARCH/ a-DGSPLAT_WARP_SIZE=flag so host and device agree on Instinct.Test plan
PYTORCH_ROCM_ARCH=gfx1151 pip install --no-build-isolation -e .→gsplat/csrc.soproduced,import gsplatsucceeds. ✅own torch reference (
gsplat/cuda/_torch_impl.py,_torch_impl_2dgs.py) on a tinyfixed scene. Proves the wave32 reductions are correct and the silent-no-op path is
gone. ✅ max abs error 2.0e-6 on the wave32 color/SH gradient (the path that
silently no-op'd before the fix).
gsplat.rasterization(...)on a 100-Gaussian synthetic scene, 64×64 render +backward. ✅
RASTERIZE+BACKWARD OK.examples/simple_trainer.py default --max-steps 7000on a real COLMAP project(4K photogrammetry). ✅ ran to completion — 2,658,384 Gaussians; PSNR 20.94 /
SSIM 0.60 / LPIPS 0.355 over held-out views; no crash, stable memory.
Environment note (not part of this PR)
Early gfx1151 bring-up hit a first-GPU-op segfault in
libhsa-runtime64.so(ROCm #5853, a CWSR/VGPR-count mismatch) — purely a ROCm-runtime issue, unrelated to
these kernel changes. It is fixed in stock ROCm ≥7.2.1, which is why the validated
environment above uses the 7.2.1 base; no special env vars or workarounds are needed on
a current ROCm.