Update OpenMP Kernels #211
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
jyoung3131
reviewed
Sep 19, 2024
| tl[j] = sl[pattern[j]]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| assert(dense_perthread[rand()%omp_threads][rand()%pattern_length]!=0); | ||
|
|
||
| std::atomic_thread_fence(std::memory_order_release); |
There was a problem hiding this comment.
Hi @radelja - do we need to remove the atomic thread fence op here? I think this was added as a new "feature" for 2.0.
There was a problem hiding this comment.
This was added in PR #12 to Jered's fork, and I assumed it was included as part of testing the gather kernel. Should I add this to the other kernels using the dense_perthread buffer?
There was a problem hiding this comment.
We should remove it for now, I think. It needs to be behind a flag when it is added. And should be on every kernel
|
Looks good and runs fine on my machine. Once Jeff's comment about atomics is resolved that we can merge |
plavin
approved these changes
Sep 23, 2024
3 tasks
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.
Overview
This PR updates the OpenMP kernels to address an issue with the gather kernel and aligns them closer to their v1.1 implementations. As mentioned in #189, there is still a gap in performance between the current scatter, multiscatter, and sg OpenMP kernels on certain platforms.
✨ Change Description/Rationale
dense_perthreadbuffers in the scatter and multiscatter OpenMP kernels👀 Reviewer Checklist
✅ PR Checklist
spatter-devel🚀 TODOs
📌 Future Work