jhuber6 added inline comments.
================
Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:351
+/// required for texture / surface / managed variables.
+Function *createRegisterGlobalsFunction(Module &M) {
+ LLVMContext &C = M.getContext();
----------------
tra wrote:
> jdoerfert wrote:
> > 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`.
> OK. I'm fine keeping it all here for now.
> Please add a comment pointing towards the origin of this code. and, maybe, a
> TODO item to consolidate and move it into a better place.
Will do, thanks for the reviews. I'll land these tomorrow morning and see if
anything breaks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123812/new/
https://reviews.llvm.org/D123812
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
