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

Fri May 6 11:33:32 PDT 2022

tra added a comment.

In D123471#3497224 <https://reviews.llvm.org/D123471#3497224>, @jhuber6 wrote:

> It's a little tough, I chose that format because it's exactly the same as we use with OpenMP with a few different flags. I wish whoever initially designed the struct made the ``reserved`` field 64-bits so it could conceivably hold a pointer to some additional information, but that ship has sailed. I originally chose to have this match the OpenMP struct because it will heavily simplify things if every language uses this same method for registering their globals. I would like to change it, but I'm not sure how well it would be received considering backwards compatibility. I'm not sure what the best path forward is on that front.

If it's exiting format that's already in use, then sticking with it is fine. We'll deal with this if/when we'll need to change it.

================
Comment at: clang/lib/CodeGen/CGCUDARuntime.h:58-70
+      OffloadRegionKernelEntry = 0x0,
+    };
+
+    /// The kind flag of the global variable entry.
+    enum OffloadVarEntryKindFlag : uint32_t {
+      /// Mark the entry as a global variable.
+      OffloadGlobalVarEntry = 0x0,
----------------
I'm a bit puzzled by this arrangement. Are those actually flags (i.e. can be set independently) or are they enumerating specific offload kinds (i.e. only one of these values is intended to be set)?

I think we want the latter. If that's the case I'd propose to enumerate kernel and data together, so each kind gets a distinct value and is easy to tell when one needs to examine the offload table manually. Right now both kernels and global vars set the flags to 0.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471