[Openmp-commits] [PATCH] D83269: [OpenMP] Identify GPU kernels (aka. OpenMP target regions)

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.

In D83269#2137745 <https://reviews.llvm.org/D83269#2137745>, @JonChesterfield wrote:

> I think there's slightly more code here than is necessary.
>
> Specifically, I think identifyKernels should return SmallPtrSetImpl<Kernel> instead of populating a member variable which can later be accessed. With a rename, proposing:
>  `SmallPtrSetImpl<Kernel> getKernels(Module &M){/*roughly contents of current identifyKernels */}`
>
> The cache then stores the set by value instead of by reference. Less state lying around, can't accidentally add multiple copies of the name to a single set. Depending on the control flow we might look up the metadata more than once, but that seems fine given it usually goes in a cache.
>
> Thoughts?


We will end up looking at it once per SCC in the program, per invocation of the pass. I would prefer to cache module wide information explicitly and this was the "smallest" solution for this for now.
I can do recompute but the `nvvm.annotations` has ~100 (non-kernel) entries from the device runtime we'll have to go through every time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83269/new/

https://reviews.llvm.org/D83269