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

Jon Chesterfield via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 7 16:47:38 PDT 2020


JonChesterfield added a comment.

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83269





More information about the llvm-commits mailing list