[Openmp-commits] [PATCH] D95155: [libomptarget] Build cuda plugin without cuda installed locally

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added inline comments.


================
Comment at: openmp/libomptarget/plugins/cuda/dynamic_cuda/cuda.cpp:90
+  return dlwrap_cuInit(arg);
+}
----------------
jdoerfert wrote:
> I would feel better if we 
> 1) use some mechanism to avoid racing on the `checkForCUDA` call, e.g., call once and a boolean global as in D95104.
> 2) Don't overload cuInit but instead call it from the plugin constructor, or make it a constructor. 
> If 2) has inherent benefits we cannot get any other way, I guess we can keep it. I'm worried it might clash with a cuda otherwise linked into the application or confuse people, or something else bad.
This is called once, from a global constructor, so is correct as-is. I'll add the locking now though so that can't cause trouble later.

The cuInit and other symbols here aren't visible outside of the plugin (there's an exports list applied when linking it). Otherwise yeah, that would be bad.

Doing the extra setup in this cuInit means the RTL.cpp doesn't know whether it's linked against cuda or against this shim. This way it is an implementation of cuda, with the implementation detail that it forwards everything to a .so with the right name.

I think that's much better than passing in a C macro or adding a weak function. It might be a surprising solution to the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95155