[PATCH] D42642: [CUDA] Detect installation in PATH

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added a comment.

LGTM.



================
Comment at: lib/Driver/ToolChains/Cuda.cpp:96-105
+      if (llvm::ErrorOr<std::string> ptxas =
+              llvm::sys::findProgramByName("ptxas")) {
+        SmallString<256> ptxasAbsolutePath;
+        llvm::sys::fs::real_path(*ptxas, ptxasAbsolutePath);
+
+        StringRef ptxasDir = llvm::sys::path::parent_path(ptxasAbsolutePath);
+        if (llvm::sys::path::filename(ptxasDir) == "bin")
----------------
Hahnfeld wrote:
> tra wrote:
> > Another corner case:
> > Debian scatters CUDA install all over the filesystem. To make it work with clang it has a 'shim' package which re-creates complete CUDA install using symlinks to its scattered bits. https://bugs.llvm.org/show_bug.cgi?id=35249.  If PATH includes such a shim with a symlink pointing to location somewhere else in the filesystem, this variant of the patch will not work.
> > 
> > I'd add another candidate derived from the path returned by find. This should cover all reasonable scenarios I can think of.
> > 
> > Caveat: clang on Debian already has a special case to add this shim to the list of candidates ( D40453 ), so this patch should not affect it. Still, it's possible for the similar case to happen somewhere else where we do not have any explicit workarounds in clang.
> > 
> > BTW, should this heuristic apply on Windows, too? IIRC cuda installer does add CUDA's bin dir to PATH.
> > 
> I'd rather not complicate this further. I asked on D40453 and the reply was
> >>! In D40453#936114, @sylvestre.ledru wrote:
> > Debian packages don't update the PATH and we are aiming at providing packages working out of the box.
> 
> So IMO if distributions opt to fully diverge from CUDA's standard directory layout they should add special cases here to make it work.
> 
> I'm not really sure regarding Windows:
> 1. From what I recall the `PATH` variable was somewhat dubious under Windows...
> 2. Does the installer actually allow to customize the installation path? If not, how likely is it that users move the directory manually?
> I can't test on Windows, so I'd generally prefer if you adjust the code and add tests in a separate patch...
I don't have windows either.
OK, we can handle oddball CUDA installations and exotic platforms until someone wants it.




https://reviews.llvm.org/D42642