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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
