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

Wed May 25 15:26:55 PDT 2022

jhuber6 added a comment.

In D125904#3538714 <https://reviews.llvm.org/D125904#3538714>, @tra wrote:

> It would be great to have some compile-time checks for that, if possible. Otherwise it will only manifest at run-time and the end user will have no clue what's going on.

Not sure how we could check it at compile-time, if we knew what it was supposed to be we could just set it properly right?

================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6855
+      if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID))
+        SM.getDiagnostics().Report(diag::err_cannot_open_file)
+            << PLoc.getFilename() << EC.message();
----------------
tra wrote:
> If we're compiling an already preprocessed file with `#line` directives, I assume that the source location will be pointing to the file specified by `#line`, not the input file itself.
> If that file is not available (and it does not have to. E.g. I someone has preprocess the original CUDA source and use creduce on the preprocessed code) then we'll get an error when we should not have.
> 
> It also results in a different CUIDs being generated for different identifiers, which is different from one CUID for everything generated during the compilation. I can not tell whether it matters in practice. As long as they are in sync betwee the host and the device, it should be OK and it may even have the benefit of allowing commoning things that come from headers, while driver-set common CUID would make all such instances unique.
> 
> 
This should try to use the line directive first, and the current file second if that's not available. The only thing that could change this for the host to device is macros, but we check those currently so it should always be in sync. The downside is if the user somehow only passes a macro to the device side it'll somewhat silently fail when we try to register it.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125904/new/

https://reviews.llvm.org/D125904