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

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 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.
----------------
tra wrote:
> 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.
> 
> 
Ah, I see the point you're making now. This is yet another thing that OpenMP did that I just copied. I wouldn't have implemented it this way but I figured it would be simpler to keep them similar. I mostly did it this way because I did some initial tests of registering and accessing CUDA globals in OpenMP and it required using the same flags for the kernels and globals. We could change it for CUDA in the future and I could make that change here if it's valuable. Ideally I would like to rewrite how we do all this registration with the structs but breaking the ABI makes it complicated...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471