Add __restrict to some unified kernels.
#1910
Open
Conversation
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
ginkgo-bot
added
mod:core
MarcelKoch
commented
Aug 13, 2025
Comment on lines
+244
to
+249
| * @tparam T the underlying type being mapped. Any references or const | ||
| * qualifiers have to be resolved before passing the type. | ||
| * The distinction between const/mutable objects is done by | ||
| * overloading the map_to_device function. | ||
| * @tparam PtrWrapper the pointer type. By default, it's just `T*`, but it may | ||
| * be set to restricted_ptr. |
Member
Author
There was a problem hiding this comment.
I've changed a bit how to_device_type_impl<T> is implemented, since it was easier for me to reason about it after removing all cv/ref.
If wanted I can also revert this change.
Member
|
should we just use it to omp_restrict now if cuda almost have slowdown by adding that? |
In benchmarks the (add|sub)_diag operations could experience significant slowdowns (up to 30% for single-threaded n=20k on intel), while the benefit in other cases was not as significant.
MarcelKoch
force-pushed
the
benchmark-add_sub_diag
branch
from
8205148 to
ac150d4
Compare
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 adds the ability to annotate pointers used in the unified kernels with the
__restrictkeyword. To add the keyword, the object has to be wrapped byas_restrict, when passed to the kernel. (This does not apply to the solver kernels. I would need more work to fix those, which could be done at a later point if necessary.)Currently this has been added only to the
(add|sub)_scaleddense kernel. I briefly also added it to the(add|sub)_scaled_diag, but in a few cases the performance dropped significantly.I ran some benchmarks on the coma-cluster using this input file:
in.json
These are the results, already translated into speedup/slowdown:
blas.json
The cuda and intel-cpu machine was gpu-nvidia-h100, the amd-cpu machine was rocinante.
Here are also only the cases where a slowdown occured.
blas.slowdown.json
As mentioned before, I removed the largest slowdown again. The other slowdowns are ~5% for cuda for the smallest sizes. I think it is reasonable to still continue, since the openmp speedup is quite significant, and small sizes are less relevant for cuda than they are for cpus.