[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Joseph Huber via Phabricator via cfe-commits]

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