[PATCH] D123812: [CUDA] Add wrapper code generation for registering CUDA images

Fri May 6 08:08:00 PDT 2022

jdoerfert added a comment.

Are there unresolved concerns we should address in this review?

================
Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:351
+///       required for texture / surface / managed variables.
+Function *createRegisterGlobalsFunction(Module &M) {
+  LLVMContext &C = M.getContext();
----------------
jhuber6 wrote:
> tra wrote:
> > jhuber6 wrote:
> > > tra wrote:
> > > > Do you think generation of the CUDA registration glue could be shared with the front-end?
> > > > 
> > > I was thinking about it, but ultimately decided to keep the noise outside of the new driver to a minimum. Maybe if we move to the offloading entries being a common format we can easily share this code. Keeping it in Clang would have the advantage that it's easier to test directly and ensures we don't de-sync if anything changes. The only downside is that in the future I may want to push this functionality to a linker plugin or similar, which would require pulling it out of Clang again to prevent us needing to link in Clang to build LLVM.
> > > 
> > > Also needing to do this all through the builder API isn't ideal, it would be nice if we had some kind of runtime to call to do this for us, but I didn't feel like adding yet another shared library for CUDA. I considered putting it inside the cuda header wrappers as well, but forcing every CUDA file to have some externally visible weak registration blob didn't sit well with me.
> > Perhaps front-end is not the right place for it, indeed. LLVM itself may be a better choice. We already have some things there for somewhat similar purposes (like lib/WindowsManifest) so adding a helper function to generate runtime glue for CUDA should not be unreasonable.
> I think it's fine here for this patch, but I definitely want to move it into LLVM in the future once I start generalizing more of this stuff.
I'm OK with it being here but the place to consider (IMHO) is `llvm/lib/Frontend`, maybe `/CUDA/Register.cpp`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123812