tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:351
+/// required for texture / surface / managed variables.
+Function *createRegisterGlobalsFunction(Module &M) {
+ LLVMContext &C = M.getContext();
----------------
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.
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
