-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove AdaptiveCpp workaround in dpl_shim.h to allow for automatic prefetch optimization #177
Conversation
…efetch optimization
Thanks, looks good. For the record, do I need a specific commit or branch for prefetch to work or is the latest commit OK? |
Please update the CHANGELOG, and consider if this needs to go to |
It's on develop. But I'm still working on optimizing. I think for your software it's all good, but e.g. LULESH is submitting a ton of small kernels where the additional overhead is observable for small problems.
Ok, I can do that. What is your release schedule? The workaround on current main is only relevant for an earlier prerelease version of AdaptiveCpp stdpar, and is already unnecessary for AdaptiveCpp 23.10.0 release. And it has detrimental effect now. So I'd be much in favor if the |
The changes in CHANGELOG are grouped by version. Am I supposed to create a new section for the next version? If so, what is the next version going to be? |
Re Release Schedule: we haven't done one for a while, but I think we're collecting lots of minor things that would warrant a 5.1 release soon. I normally just add an |
Ok, that's great, thanks :)
The The github-generated changelog depends on the title of PRs, so before merging it would be good to check the PR titles and potentially rename them if they are not precise enough. On the plus side, github changelogs also handle correct attribution of contributions. |
…efetch optimization (UoB-HPC#177)
…matic prefetch optimization (UoB-HPC#177)" This reverts commit 06c3d53.
To my knowledge, the AdaptiveCpp-specific workaround in
dpl_shim.h
is no longer necessary as the linking issue was fixed some time ago.Additionally, this workaround prevents a new optmization in AdaptiveCpp stdpar from taking place: AdaptiveCpp stdpar can now emit automatic data prefetches. But this only works, if the data is managed by the stdpar runtime, which tracks pointers it knows. If
sycl::malloc_shared
is used directly, the stdpar optimizations do not recognize the pointer and won't apply the optimization.(the reasoning being that if someone calls SYCL functionality directly, they might want to exert more control and emit prefetches or
mem_advise
operations by themselves).Other projects which also use this header like cloverleaf are also affected. Will create a PR there too.