feat: support build on gfx1250 (Part2)#423
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is part 2 of adding gfx1250 (CDNA5) support by making MXFP4/MXFP8 quantize/dequantize kernels run correctly with 32-lane wavefronts, and by adjusting the turbo (MFMA) FP8 GEMM binding to link more uniformly across builds.
Changes:
- Parameterizes quantization kernels by wavefront size (
THREADS_PER_WARP), updating block sizing and converting some fixed thread↔work mappings into grid-stride loops. - Adds gfx1250-specific conversion paths for MXFP4/MXFP8 (quantize/dequantize) using AMDGCN compiler builtins and removes “not implemented on gfx1250” runtime guards.
- Moves the gfx1250 “turbo GEMM unsupported” runtime guard from the PyTorch binding into
turbo_gemm_mxfp8_impland removes the previous stub-branch approach.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| csrc/pytorch/gemm/turbo_gemm.cpp | Removes the entry-point gfx1250 guard so the binding relies on the lower-level impl to reject gfx1250. |
| csrc/kernels/quantization/quantization_mxfp8.cu | Makes MXFP8 quantization wavefront-size aware; adds gfx1250 builtin conversion path; updates colwise write-out mapping. |
| csrc/kernels/quantization/quantization_mxfp4.cu | Makes MXFP4 quantization wavefront-size aware; adds gfx1250 builtin conversion paths; updates LDS init + colwise write-out mapping. |
| csrc/kernels/quantization/dequantization_mxfp8.cu | Removes gfx1250 “not implemented” guards for MXFP8 dequantization paths. |
| csrc/kernels/quantization/dequantization_mxfp4.cu | Adds gfx1250 builtin-based FP4 unpack path and removes gfx1250 “not implemented” guard. |
| csrc/kernels/gemm/turbo_gemm.cu | Removes BUILD_TURBO_BACKEND stub branch and moves gfx1250 rejection to runtime check inside turbo_gemm_mxfp8_impl. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -67,9 +67,6 @@ at::Tensor turbo_gemm_fp8(at::Tensor A, at::Tensor scaleA_inv, at::Tensor B, at: | |||
| const at::ScalarType out_dtype, bool transA, bool transB, bool transC, | |||
| const std::string &granularity) { | |||
| // Runtime guard: the turbo (MFMA) backend is not compiled on gfx1250. | |||
Comment on lines
1732
to
1734
| dim3 grid((total_M_padded + BLOCK_M - 1) / BLOCK_M, (N_pad + BLOCK_N - 1) / BLOCK_N); | ||
| dim3 block(THREADS_PER_BLOCK); | ||
| dim3 block(warp_size() * WARPS_PER_BLOCK); | ||
|
|
Comment on lines
1142
to
1144
| dim3 grid((M_pad + BLOCK_M - 1) / BLOCK_M, (N_pad + BLOCK_N - 1) / BLOCK_N, G); | ||
| dim3 block(THREADS_PER_BLOCK); | ||
| dim3 block(warp_size() * WARPS_PER_BLOCK); | ||
| const uint32_t sr_seed = global_sr_counter.fetch_add(1, std::memory_order_relaxed); |
xiaobochen-amd
approved these changes
Jul 21, 2026
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.
Description
This PR is Part 2 of enabling Primus-Turbo on gfx1250 (CDNA5). Part 1 made the project build for gfx1250; this part makes the MXFP4 / MXFP8 quantization and de-quantization kernels actually run correctly on gfx1250, and unblocks the turbo (MFMA) FP8 GEMM host binding so it links uniformly.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Parameterized the wavefront size in the MXFP4 / MXFP8 quantization kernels.
WARP_SIZEnow resolves to the per-archTHREADS_PER_WARP(64 on gfx942/gfx950, 32 on gfx1250),WARPS_PER_BLOCKis fixed at 4 so each warp still owns exactly one 32x32 chunk, andTHREADS_PER_BLOCKscales toWARP_SIZE * WARPS_PER_BLOCK(256 on gfx950, 128 on gfx1250).This keeps a 1-warp-per-chunk mapping, which avoids both the extra
__syncthreadsdeadlock and the LDS blow-up that a fixed 256-thread block would cause with 32-lane wavefronts.Fixed the host launch geometry for the fat-binary case.
Because the host translation unit is compiled once but device code is compiled per
--offload-arch, the block dimension is now computed from the runtimewarp_size()(dim3 block(warp_size() * WARPS_PER_BLOCK)) instead of the compile-timeTHREADS_PER_BLOCK.Generalized the coalesced colwise FP4 / FP8 write-out and the LDS zero-init into grid-stride loops.
These previously assumed a one-item-per-thread mapping tied to a 256-thread block; they now cover the buffer for both 256-thread (gfx950) and 128-thread (gfx1250) blocks, and the corresponding
static_asserts were relaxed from strict equality to divisibility.Added gfx1250 conversion paths using compiler builtins instead of inline asm.
Quantization uses
__builtin_amdgcn_cvt_scalef32_pk8_fp4_f32,__builtin_amdgcn_cvt_scalef32_sr_pk8_fp4_f32(stochastic rounding), and__builtin_amdgcn_cvt_scalef32_pk8_{fp8,bf8}_f32, padding the unused upper lanes and keeping the low bits so the element-to-nibble/byte order and the divide-by-scale semantics match the gfx950 path exactly.De-quantization uses
__builtin_amdgcn_cvt_scale_pk8_f32_fp4purely as an FP4-to-F32 unpack (scale byte0x7f, opsel 0) and applies the per-thread scale in registers, sidestepping the cross-lane scale selection of the PK8 de-quant converter.MXFP8 de-quantization needed no intrinsic change (it is a pure C++ cast plus scale multiply); only the gfx1250 guard was removed.
Removed all the temporary
is_gfx1250()"not implemented" guards from the MXFP4 / MXFP8 quantize / de-quantize entry points added in Part 1.Unblocked the turbo (MFMA) FP8 GEMM binding.
Dropped the
#ifdef BUILD_TURBO_BACKENDstub branch inturbo_gemm.cuand moved the gfx1250 runtime guard down intoturbo_gemm_mxfp8_impl, so the host binding inturbo_gemm.cpplinks uniformly and fails clearly at call time on gfx1250 rather than at build configuration.Verification
Note:
tests/conftest.pystill carries a blanket "not yet supported on gfx1250" skip from Part 1, so the above was validated with standalone scripts using the same ops and tolerances. Relaxing that skip is left out of this PR because other ops (e.g. GEMM) are still unsupported on gfx1250.Checklist: