jhuber6 added a comment. In D125904#3537872 <https://reviews.llvm.org/D125904#3537872>, @tra wrote:
> Is this patch in its current form blocking any of your other work? no-cuid > approach, even if we figure out how to do it, will likely take some time. Do > you need an interim solution until then? > > In D125904#3537385 <https://reviews.llvm.org/D125904#3537385>, @jhuber6 wrote: > >> Also, for the OpenMP case, we already pass the host-IR as a dependency for >> the device compilation. So it would be relatively easy for us to just >> generate these names on the host and then read them from the IR for the >> device. The problem is that CUDA / HIP doesn't use this approach so it >> wouldn't be a great solution to have two different ways to do this. So we >> would either need to make CUDA / HIP take the host IR and use that, or move >> OpenMP to use the driver. The benefit of passing the IR is that we can much >> more stably generate some arbitrary string to mangle these names and we're >> guarunteed to have them match up because we read them from the host. The >> downside is that it'd be somewhat of a regression because now we have an >> extra call to Clang for CUDA / HIP when we previously didn't need to. > > Yeah. The different compilation flows are a bit of a problem. So is the > closeness of NVIDIA's binary format, which limits what we can do with them. > E.g. we can't currently modify GPU binary and rename of add new symbols. > > I'll need to think about the no-cuid solution. If we can solve it, not > deviating from C++ linking would be a valuable benefit and would save us some > headaches down the road. Extra clang invocation may be worth it, but it's too > early to tell. It's blocking the new driver from handling static variables correctly because I haven't written the CUID support there yet. I could go ahead and copy over some necessary code to get it to work there, but a no-CUID solution would definitely be ideal. Personally, I think this is fine as a fallback so clang at least generates **something** that works rather than just leaving it completely blank. The code changed in this patch is pretty minimal. But I will probably get rid of the environment variable check, since I can't verify exactly that it will be the same between the host and device if we were to land this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125904/new/ https://reviews.llvm.org/D125904 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
