Fix getValidWorkDiv for CUDA and ROCm [I] #2222 #2251
Merged
Conversation
This file contains 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
mehmetyusufoglu
force-pushed
the
fixValidWorkDiv
branch
6 times, most recently
from
e8dca57 to
6400b7f
Compare
mehmetyusufoglu
force-pushed
the
fixValidWorkDiv
branch
6 times, most recently
from
503e26e to
f0dd41e
Compare
mehmetyusufoglu
force-pushed
the
fixValidWorkDiv
branch
3 times, most recently
from
30694dc to
de0aed5
Compare
mehmetyusufoglu
force-pushed
the
fixValidWorkDiv
branch
11 times, most recently
from
589c616 to
3d3322e
Compare
|
offline discussed in the developer meeting:
|
mehmetyusufoglu
force-pushed
the
fixValidWorkDiv
branch
3 times, most recently
from
0a04381 to
5a21705
Compare
|
please rebase against dev to get the fix from #2294 in and remove draft from the PR if it is ready |
Co-authored-by: Andrea Bocci <[email protected]>
mehmetyusufoglu
force-pushed
the
fixValidWorkDiv
branch
from
5a21705 to
b7f84a5
Compare
Done, thanks. |
psychocoderHPC
requested changes
Jun 20, 2024
| { | ||
| // Expected valid workdiv for this kernel. These values might change depending on the GPU type and compiler | ||
| // therefore commented out. Number of registers used by kernel might limit threads per block value differently | ||
| // for different GPUs. CHECK(workDiv == WorkDiv{Vec{8, 729444}, Vec{1, 736}, Vec{1, 1}}); |
There was a problem hiding this comment.
this comment looks outdated with this hard coded values
There was a problem hiding this comment.
Not outdated, but i changed the comment since could be confusing and values would change for different GPU versions.
psychocoderHPC
approved these changes
Jun 24, 2024
2 tasks
psychocoderHPC
added a commit
to psychocoderHPC/alpaka
that referenced
this pull request
Aug 9, 2024
- revert introduced `KernelBundle` in alpaka-group#2251 - change signature of `getValidWorkDivForKernel`,`isValidWorkDivKernel` and `isValidWorkDivKernel` - reuse old naming `getValidWorkDiv` and `isValidWorkDiv`
psychocoderHPC
added a commit
to psychocoderHPC/alpaka
that referenced
this pull request
Aug 9, 2024
- revert introduced `KernelBundle` in alpaka-group#2251 - change signature of `getValidWorkDivForKernel`,`isValidWorkDivKernel` and `isValidWorkDivKernel` - reuse old naming `getValidWorkDiv` and `isValidWorkDiv`
psychocoderHPC
added a commit
to psychocoderHPC/alpaka
that referenced
this pull request
Aug 9, 2024
- revert introduced `KernelBundle` in alpaka-group#2251 - change signature of `getValidWorkDivForKernel`,`isValidWorkDivKernel` and `isValidWorkDivKernel` - reuse old naming `getValidWorkDiv` and `isValidWorkDiv`
psychocoderHPC
added a commit
to psychocoderHPC/alpaka
that referenced
this pull request
Aug 9, 2024
- revert introduced `KernelBundle` in alpaka-group#2251 - change signature of `getValidWorkDivForKernel`,`isValidWorkDivKernel` and `isValidWorkDivKernel` - reuse old naming `getValidWorkDiv` and `isValidWorkDiv`
psychocoderHPC
added a commit
to psychocoderHPC/alpaka
that referenced
this pull request
Aug 9, 2024
- revert introduced `KernelBundle` in alpaka-group#2251 - change signature of `getValidWorkDivForKernel`,`isValidWorkDivKernel` and `isValidWorkDivKernel` - reuse old naming `getValidWorkDiv` and `isValidWorkDiv`
psychocoderHPC
added a commit
to psychocoderHPC/alpaka
that referenced
this pull request
Aug 9, 2024
- revert introduced `KernelBundle` in alpaka-group#2251 - change signature of `getValidWorkDivForKernel`,`isValidWorkDivKernel` and `isValidWorkDivKernel` - reuse old naming `getValidWorkDiv` and `isValidWorkDiv`
psychocoderHPC
added a commit
to psychocoderHPC/alpaka
that referenced
this pull request
Aug 13, 2024
- revert introduced `KernelBundle` in alpaka-group#2251 - change signature of `getValidWorkDivForKernel`,`isValidWorkDivKernel` and `isValidWorkDivKernel` - reuse old naming `getValidWorkDiv` and `isValidWorkDiv`
psychocoderHPC
added a commit
to psychocoderHPC/alpaka
that referenced
this pull request
Aug 14, 2024
- revert introduced `KernelBundle` in alpaka-group#2251 - change signature of `getValidWorkDivForKernel`,`isValidWorkDivKernel` and `isValidWorkDivKernel` - reuse old naming `getValidWorkDiv` and `isValidWorkDiv`
SimeonEhrig
pushed a commit
that referenced
this pull request
Aug 15, 2024
…2349) * remove `KernelBundle`, change signature of `getValidWorkDiv*` - revert introduced `KernelBundle` in #2251 - change signature of `getValidWorkDivForKernel`,`isValidWorkDivKernel` and `isValidWorkDivKernel` - reuse old naming `getValidWorkDiv` and `isValidWorkDiv` * use new interface for `getValidWorkDiv` * fix cheat sheet
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.
getValidWorkDivhas a bug. It only considers device hard properties (TApi::getDeviceProperties()); does not consider the kernel function. Actually kernel function properties can limit number of threads per block.In the case of CUDA and ROCm, the function
getValidWorkDivshould useTApi::funcGetAttributes(&funcAttrs, kernelName)together withTApi::getDeviceProperties().I have added 3 functions.
getFunctionAttributes,getValidWorkDivKernelandisValidWorkDivKernel. One function intrinsic to Alpaka changed:subDivideGridElems. Now has an additional argument; "threads per block", if the value is zero the function uses device hard properties as before.Next PR: The 2 functions
getValidWorkDivKernelandisValidWorkDivKernelwill be used in Examples and Tests instead of getValidWorkDiv and isValidWorkDiv at the next PR. The names can be determined.Credits to @psychocoderHPC for the PR.
Tests: The tests for 1D and 2D workdivs were added.