In the OpenMC app we saw omp target update spending an awful lot of
time in the shadow map traversal without ever doing any update there.
There are two cases that allow us to avoid the traversal completely.
The simplest thing is that small updates cannot (reasonably) contain
an attached pointer part. The other case requires to track in the
mapping table if an entry might contain an attached pointer as part.
Given that we have a single location shadow map entries are created,
the latter is actually fairly easy as well.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM with two minor nits.
openmp/libomptarget/src/device.cpp | ||
---|---|---|
343 | Can you add a comment explaining that IsNew is always going to be false because getTgtPtrBegin is called from targetDataEnd, targetDataContiguous and processDataBefore, i.e. for data which is expected to be mapped already. And maybe it's more elegant to hardcode false in the return statement directly, thus avoiding defining a variable which is never used in the body of this function. | |
openmp/libomptarget/src/omptarget.cpp | ||
619 | an be --> can be |
Ping. I'm preparing some more ShadowPtrMap optimizations which I'd like to base on top of this patch. Is there any planned change or can we land the patch?
This seems to have broken some x64 openmp tests, e.g. https://lab.llvm.org/buildbot/#/builders/193/builds/2940
i see the same failures in my local build, and when i revert this commit, and rebuild/test, failures go away
Can you add a comment explaining that IsNew is always going to be false because getTgtPtrBegin is called from targetDataEnd, targetDataContiguous and processDataBefore, i.e. for data which is expected to be mapped already. And maybe it's more elegant to hardcode false in the return statement directly, thus avoiding defining a variable which is never used in the body of this function.