Bug fix(distance): correct CK pairwise distance on RDNA3 (gfx1100) WMMA - #8
Open
zihaomu wants to merge 1 commit into
Open
Conversation
The Composable-Kernel pairwise-distance path was both unreachable and, once reached, numerically wrong on RDNA3 (gfx11) GPUs. Two independent issues: 1. dispatch_ck.cuh - dead-code 4GB guard. The buffer-descriptor guard compared the THEORETICAL maximum batch (mb_max * nb_max) against the 4GB limit instead of the EFFECTIVE per-batch output size min(mb_max, m) * min(nb_max, n). The theoretical product always exceeds 4GB, so the guard always tripped and every problem size silently fell back to the SM60 kernel - the CK pairwise path was never executed. 2. ck/epilogue.hpp - wrong warp-gemm selected for the C tile distribution. The CShuffle epilogue selected the warp-gemm that defines the C tile-distribution using ODataType (the output type, float) rather than the GEMM input type. On CDNA both resolve to the same MFMA C layout, so the bug is invisible there. On RDNA3, <float,float,float> selects the MFMA warp-gemm (wave64, 4 C registers/thread) while the pipeline accumulator uses the fp16 WMMA warp-gemm (wave32, 8 interleaved C registers/thread). The layout mismatch made slice_acc_tile copy only 4 of 8 C registers, zeroing/scrambling half of the output rows. Fix: - Thread the GEMM input type (DataT) into PairwiseDistanceCkEpilogueProblem and use it (instead of ODataType) to select the warp-gemm, so the epilogue C distribution matches the pipeline accumulator on both CDNA and RDNA. - Guard on the effective per-batch dims so the CK path is actually reached. Validated on Radeon PRO W7900 (gfx1100), ROCm 7.2.4: fp16 L2Expanded and Cosine now match the CPU/SM60 reference exactly across sizes (16..512, D up to 512); fp32 continues to use the SM60 path unchanged. fp16 pairwise distance is 10-19x faster than the SM60 fallback (4096x4096x512: 8.99 ms -> 0.47 ms, 1911 -> 36501 GFLOP/s). Scope: only cuvsPairwiseDistance (L2Expanded/Cosine) is affected; brute-force / CAGRA / IVF do not use this dispatch.
zihaomu
marked this pull request as ready for review
August 21, 2026 02:51
Contributor
|
Thanks @zihaomu for this contribution. I might be missing something but it looks like the CK path is currently only enabled for GCN architectures that start with the gfx9 prefix. https://github.com/AMD-Ecosystem/hipVS/blob/release/rocmds-26.03/cpp/src/distance/detail/pairwise_matrix/dispatch_ck.cuh#L228-L232. Were there other local changes that forced control to take the CK path on your RDNA system? Should we relax that check? |
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.
Summary
The Composable-Kernel (CK) pairwise-distance path was both unreachable and, once reached, numerically wrong on RDNA3 (gfx11) GPUs. This PR fixes two independent issues so that
cuvsPairwiseDistance(L2Expanded / Cosine) correctly and efficiently uses RDNA3 WMMA.Bugs
1. Dead-code 4GB guard (
dispatch_ck.cuh)The buffer-descriptor guard compared the theoretical maximum batch (
mb_max * nb_max) against the 4GB limit instead of the effective per-batch output sizemin(mb_max, m) * min(nb_max, n). Since the theoretical product always exceeds 4GB, the guard always tripped and every problem size silently fell back to the SM60 kernel — the CK pairwise path was never executed.2. Wrong warp-gemm for the C tile distribution (
ck/epilogue.hpp)The CShuffle epilogue selected the warp-gemm that defines the C tile-distribution using
ODataType(the output type,float) rather than the GEMM input type.<float,float,float>and<half,half,float>resolve to the same MFMA C layout, so the bug is invisible.<float,float,float>selects the MFMA warp-gemm (wave64, 4 C regs/thread) while the pipeline accumulator uses the fp16 WMMA warp-gemm (wave32, 8 interleaved C regs/thread). The mismatch madeslice_acc_tilecopy only 4 of 8 C registers, zeroing/scrambling half of the output rows.Fix
DataT) intoPairwiseDistanceCkEpilogueProblemand use it (instead ofODataType) to select the warp-gemm, so the epilogue C distribution matches the pipeline accumulator on both CDNA and RDNA.3 files changed, +16 / −4.
Validation — Radeon PRO W7900 (gfx1100), ROCm 7.2.4
Repo test suite (
DISTANCE_TEST): passes on gfx1100 — all L2Expanded + Cosine cases (101/101), including the fp16halfinstances (DistanceEucExpTestH,DistanceExpCosH, and theirXequalYvariants) that now exercise the CK WMMA path. Differential check: reverting only the epilogue fix (keeping the guard so CK stays engaged) makes those fp16 cases fail — confirming the tests actually hit the CK path and that the fix is load-bearing. fp32/f64 continue on the SM60 path unchanged.Independent harness: fp16 L2Expanded & Cosine also match a CPU fp64 reference exactly (
ratio=1.0000, nbad=0) across sizes {16, 64, 100, 128, 256, 512}².Performance (fp16 L2Expanded):
Scope
Only
cuvsPairwiseDistance(L2Expanded / Cosine) is affected; brute-force / CAGRA / IVF do not use this dispatch.