Skip to content

Fix ROCm builds on 6.4.3+ and on GPU-less builders - #414

Open
kashif wants to merge 3 commits into
rusty1s:masterfrom
kashif:rocm-build-fixes
Open

Fix ROCm builds on 6.4.3+ and on GPU-less builders#414
kashif wants to merge 3 commits into
rusty1s:masterfrom
kashif:rocm-build-fixes

Conversation

@kashif

@kashif kashif commented Aug 20, 2026

Copy link
Copy Markdown

Three small things needed to build and run this correctly on ROCm.

The first commit is @Looong01's fix from #405, cherry-picked unchanged. ROCm 6.4.3+ static_asserts that the warp shuffle mask is 64-bit, so the at::Half overloads in utils.cuh no longer compile. That PR has been open a while — I hit the same wall on ROCm 7.2 and can confirm the fix works. Happy to drop the commit if you'd rather merge #405 on its own.

The second is a build fix. setup.py gates the HIP extension build on torch.cuda.is_available(), which is False on any machine without a visible GPU — including CI wheel builders. On a ROCm build of PyTorch that silently produces a CPU-only wheel, and it also skips the include_package_data work-around that's there specifically for hipify's absolute paths. Both checks now key off torch.version.hip, which describes the PyTorch build rather than the machine doing the building. CUDA behaviour is unchanged.

The third one I'm less sure about, so please treat it as a question rather than a fix.

These kernels assume 32-lane warps throughout (FULL_MASK, lane_idx = thread_idx & 31, row = thread_idx >> 5). On ROCm the SHFL_*_SYNC macros expand to the maskless __shfl* intrinsics, whose width defaults to the wavefront size — 32 on RDNA, but 64 on CDNA. The reductions look fine either way: in spmm_value_bw_kernel lane 0 only ever reads lanes that had aggregated within 0..31 at the time of the read. But the broadcast in spmm_kernel looks wrong on a 64-wide wave:

int row = thread_idx >> 5;
...
for (int i = 0; i < 32; i++)
  mat_rows[i] = SHFL_SYNC(FULL_MASK, mat_row, i);

With width == 64, i addresses the absolute lane in the full wave, so lanes 32-63 — which belong to a different row — would read the wrong row's mat_row. Silently wrong results rather than a crash.

The commit just passes an explicit width of 32 to the three ROCm macros, which is what PyTorch itself does when a kernel needs a 32-lane shuffle on wave64 (ReduceSum32 in group_norm_kernel.cu).

I can't verify this part: my GPU is gfx1150, which is wave32, so the change is a no-op there and the bug isn't reproducible. It builds clean and the suite still passes, but if someone with MI-series hardware can confirm the before/after, that would settle it. Happy to split it into its own PR if you'd prefer to merge the two build fixes first.

Tested on ROCm 7.2 / gfx1150 with PyTorch 2.13: builds from source and the test suite passes (285 passed, 4 skipped — the skips are all METIS).

Looong01 and others added 2 commits August 20, 2026 15:36
@codecov-commenter

codecov-commenter commented Aug 20, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.09%. Comparing base (2d55981) to head (1fe08aa).
⚠️ Report is 15 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #414   +/-   ##
=======================================
  Coverage   73.09%   73.09%           
=======================================
  Files          29       29           
  Lines        1171     1171           
=======================================
  Hits          856      856           
  Misses        315      315           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Kashif Rasul <kashif.rasul@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants