Fix ROCm builds on 6.4.3+ and on GPU-less builders - #414
Open
kashif wants to merge 3 commits into
Open
Conversation
Signed-off-by: Kashif Rasul <kashif.rasul@gmail.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
Signed-off-by: Kashif Rasul <kashif.rasul@gmail.com>
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.
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::Halfoverloads inutils.cuhno 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.pygates the HIP extension build ontorch.cuda.is_available(), which isFalseon 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 theinclude_package_datawork-around that's there specifically for hipify's absolute paths. Both checks now key offtorch.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 theSHFL_*_SYNCmacros expand to the maskless__shfl*intrinsics, whosewidthdefaults to the wavefront size — 32 on RDNA, but 64 on CDNA. The reductions look fine either way: inspmm_value_bw_kernellane 0 only ever reads lanes that had aggregated within 0..31 at the time of the read. But the broadcast inspmm_kernellooks wrong on a 64-wide wave:With
width == 64,iaddresses the absolute lane in the full wave, so lanes 32-63 — which belong to a differentrow— would read the wrong row'smat_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 (
ReduceSum32ingroup_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).