[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 6 15:42:40 PDT 2022


tra added inline comments.


================
Comment at: clang/lib/CodeGen/CGCUDARuntime.h:58
+      /// Mark the entry as a global variable.
+      OffloadGlobalEntry = 0x0,
+      /// Mark the entry as a managed global variable.
----------------
We're still using the same numeric value for two different kinds of entities.
Considering that it's the third round we're making around this point, I'm starting to suspect that I may be missing something.
Is there a particular reason kernels and global unmanaged variables have to have the same 'kind'?

It's possible that I didn't do a good job explaining my enthusiastic nitpicking here.
My suggestion to have unified enum for **all** entities we register is based on a principle of separation of responsibilities. If we want to know what kind of entry we're dealing with, checking the 'kind' field should be sufficient. The 'size' field should only indicate the size of the entity. Having to consider both kind and size to determine what you're dealing with just muddies things and should not be done unless there's a good reason for that. E.g. it might be OK if we were short on flag bits.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471



More information about the cfe-commits mailing list