Conversation
omlins
self-requested a review
August 20, 2026 09:07
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.
This PR fixes the issue that
compute_nthreadscould return more thannthreads_maxthreads per block, making kernel launches fail, e.g. formaxsize = (256, 5, 128):Any 3-D call is affected whose largest array argument has an extent of 3, 5, 6 or 7 in y (e.g. a 256x4x128 grid, where a y-staggered field is of size 256x5x128). The memopt path is affected as well, as it uses the same heuristic (with
nthreads_max = 128for CUDA).The thread budget of each dimension was rounded up (
ceil). Whenever a dimension was clamped to a smallermaxsize, the next dimension's share was no longer an integer and rounding it up exceeded the remaining budget;prod(nthreads) <= nthreads_maxwas never verified afterwards.Fix
Rounding the budget of each dimension down instead, which makes
prod(nthreads) <= nthreads_maxhold by construction:The heuristic is unchanged for all inputs that did not overflow: a sweep over maxsize in [1,300]x[1,80]x[1,80] for the CUDA, AMDGPU and memopt parameterisations shows that the differing inputs are exactly the previously overflowing ones (267198 of them for CUDA), and that the maximum total is now exactly
nthreads_max. The example above now gives(32, 5, 1).I've added a new
compute_nthreadstestset intest/ParallelKernel/test_parallel.jlcovering the documented cases, the regression itself, and sweeps assertingprod(nthreads) <= nthreads_maxandnthreads >= 1for the CUDA, AMDGPU and memopt parameterisations.cc @ChristianSchuler