Fix 2D parallel_for coalescing and remove per-reduce fill! (CUDA/AMDGPU) - #401
Open
miniskar wants to merge 1 commit into
Open
Fix 2D parallel_for coalescing and remove per-reduce fill! (CUDA/AMDGPU)#401miniskar wants to merge 1 commit into
miniskar wants to merge 1 commit into
Conversation
…PU): 2D parallel_for swapped grid axes for every M<N array, mapping threadIdx().x to the strided (column-major) dimension and killing memory coalescing. Now swap only when the basic grid would overflow the y-limit, and shape the block so a warp walks the stride-1 dimension. Reductions seed `init` as a kernel argument instead of pre-filling buffers with fill!, so a reduce is a pure sequence of async launches. On A100: wide arrays ~5-19x faster (41-153 -> ~770 GB/s, matching tall/square); small reductions ~25-31% faster from the two eliminated memset launches. Correctness unchanged; adds non-square 2D regression tests. Added regression tests.
Collaborator
|
Test this please |
Collaborator
Author
|
The two CI/CD failures are due to unavailability of CUDA runtime for CUDA.jl package and another is some file permission issue. |
Collaborator
|
@miniskar thanks! Indeed, it's just CI hiccups. Thanks for the PR contribution! |
Collaborator
|
Test this please |
Collaborator
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.
What
Two GPU-backend changes to
ext/CUDAExt/CUDAExt.jlandext/AMDGPUExt/AMDGPUExt.jl:parallel_forcoalescing — swap grid axes only when necessary, and keep awarp on the stride-1 dimension.
initseeding — passinitas a kernel argument instead ofpre-filling buffers with
fill!.Why
The
M < Nswap was a launchability workaround (the y-grid limit is 65535 vs. 2³¹for x), but it fired for every wide array — including the vast majority that fit the
coalesced path — turning a bandwidth-bound kernel into a strided one. The
fill!sare extra kernel launches on the reduce critical path that only existed to place an
identity value the kernel can hold itself.
Issue 1 — 2D
parallel_forloses coalescing on wide arraysA 2D launch maps one array dimension to the grid x-axis and the other to y.
yis limited to 65535 blocks;xto 2³¹. To launch a wide array (M < N) whoseNwould overflowgrid.y, the backend swaps axes soNlands onx.The two index mappings differ in which array element each warp touches. Arrays are
column-major (
iis stride-1):Before (the bug)
The swap fired for every
M < Narray, even the majority whose basic grid fitsunder the
ylimit — so a coalesced kernel was needlessly turned into a strided one:Worse, the block-shape heuristic then starved the
x(coalesced) dimension for widearrays, collapsing the block to
(1, threads):After (the fix)
Swap only when the coalesced grid would actually overflow
grid.y, and shape theblock so a full warp walks the stride-1 dimension:
Reproduce (NVIDIA A100)
Issue 2 — reductions do a synchronous
fill!between kernel launchesThe op-identity for out-of-range threads was placed by pre-filling the partial-results
buffer with
fill!— two extra launches sitting between the reduction kernels, and theUnmanagedworkspace relied on the caller having pre-filled the buffers.Before
After
Reproduce (NVIDIA A100)
Performance (NVIDIA A100)
parallel_for: ~5–19× faster — e.g. 256×262144 went 41 → ~770 GB/s,1024×65536 149 → ~770 GB/s — now matching tall/square arrays.
launches), e.g. reused reducer N=1000 44.5 → 30.8 µs. Large bandwidth-bound
reductions unchanged.
parallel_forunchanged (within noise).Correctness / tests
Behavior is unchanged for the supported contract (identity
init). Validated on A100(CUDA) and CPU threads —
reduce15,reduce-ND20,LaunchSpec14,parallel_for non-square16,parallel_reduce non-square8, all pass. Addsnon-square 2D regression tests to
test/unittests.jl; the existing 2D tests onlyused square matrices, so they never exercised the axis-swap path:
Notes
parallel_reduceon 2D still has no swap path (hard-coded 16×16),so it overflows the y-grid for
N > ~1.05M— a separate follow-up.