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

[Michael Kruse via cfe-commits]

An update:

I just re-tried creating a symbolic link from inside wsl on drvfs
(case 2). It now created a symlink just as "mklink" does. What might
have changed is that I updated to a newer insider preview or that I
enabled developer mode when I tried the first time.

[1] seems to apply for case 1. [2] has a nice overview about what can
happen here.

Michael

[1] https://github.com/libgit2/libgit2/pull/4713
[2] 
https://stackoverflow.com/questions/11662868/what-happens-when-i-clone-a-repository-with-symlinks-on-windows

Am Do., 2. Mai 2019 um 10:17 Uhr schrieb Alexandre Ganea
:
>
> Thanks Michael, it makes sense. I'm still with the old SVN setup on Windows. 
> WSL uses the NTFS partition through drvfs (your case 2.)
> I'll switch to the git monorepo, it looks like the latest git supports [1] 
> symlinks on Windows (with the restrictions you've mentioned) and they should 
> be mapped properly inside WSL in that case.
>
> Should we then assume symlinks will always work for end-users? Should we 
> validate that somehow, during cmake setup?
>
> Alex.
>
> [1] https://github.com/libgit2/libgit2/pull/4713
>
> -Message d'origine-
> De : Michael Kruse 
> Envoyé : 1 mai 2019 17:23
> À : reviews+d42642+public+5dc9c99d2f2d3...@reviews.llvm.org; Alexandre Ganea 
> via Phabricator 
> Cc : hah...@hahnjo.de; jle...@google.com; t...@google.com; Alexandre Ganea 
> ; cfe-commits ; 
> llvm-commits ; sylvestre.le...@gmail.com
> Objet : Re: [PATCH] D42642: [CUDA] Detect installation in PATH
>
> Hi,
>
> I had my own difficulties with this. It depends on how the repository
> containing the symlink has been checked-out. For instance:
>
> 1. Using a windows git (such as git extensions, mingw-git, git for windows)
> 2. Using git inside wsl on a drvfs mount (i.e. a windows folder
> mounted into the Unix filesystem)
> 3. Using git inside wsl on a lxfs mount (I.e. native mount such as '/')
>
> In case 1. git does not create a Ubuntu symlink. Also, there are no
> Unix symlinks in Windows filesystems, so when Unix tools (such as git)
> request to create a symlink, it creates something else (AFAIR it's a
> text file containing the symlink path, but having lost its symlink
> properties). Case 3 should work.
>
> I still managed to make cases 1 and 2 work by deleting the text file
> and re-creating it with windows's "mklink" tool (which requires either
> admin rights or developer mode turned on). It seems the WSL layer
> translates this as a symlink to the Linux environment.
>
> Michael
>
>
>
>
>
> Am Mi., 1. Mai 2019 um 16:59 Uhr schrieb Alexandre Ganea via
> Phabricator via llvm-commits :
> >
> > aganea added a subscriber: rnk.
> > aganea added a comment.
> >
> > So it turns out this is a symlink issue. The file 
> > `clang/trunk/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas` has been 
> > synchronized on my Windows 10 PC as a regular text file, not a symlink. It 
> > looks like TortoiseSVN doesn't implement symlinks. As WSL inherits of my 
> > file system, it will not find the symbolic link. I suppose `REQUIRES: 
> > !system-windows` isn't enough for `cuda-detect-path.cu`, and it would need 
> > something like `REQUIRES: symlinks` along with support in lit. @rnk
> >
> >
> > Repository:
> >   rL LLVM
> >
> > CHANGES SINCE LAST ACTION
> >   https://reviews.llvm.org/D42642/new/
> >
> > https://reviews.llvm.org/D42642
> >
> >
> >
> > ___
> > llvm-commits mailing list
> > llvm-comm...@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexandre Ganea via cfe-commits]

Thanks Michael, it makes sense. I'm still with the old SVN setup on Windows. 
WSL uses the NTFS partition through drvfs (your case 2.)
I'll switch to the git monorepo, it looks like the latest git supports [1] 
symlinks on Windows (with the restrictions you've mentioned) and they should be 
mapped properly inside WSL in that case.

Should we then assume symlinks will always work for end-users? Should we 
validate that somehow, during cmake setup?

Alex.

[1] https://github.com/libgit2/libgit2/pull/4713

-Message d'origine-
De : Michael Kruse  
Envoyé : 1 mai 2019 17:23
À : reviews+d42642+public+5dc9c99d2f2d3...@reviews.llvm.org; Alexandre Ganea 
via Phabricator 
Cc : hah...@hahnjo.de; jle...@google.com; t...@google.com; Alexandre Ganea 
; cfe-commits ; 
llvm-commits ; sylvestre.le...@gmail.com
Objet : Re: [PATCH] D42642: [CUDA] Detect installation in PATH

Hi,

I had my own difficulties with this. It depends on how the repository
containing the symlink has been checked-out. For instance:

1. Using a windows git (such as git extensions, mingw-git, git for windows)
2. Using git inside wsl on a drvfs mount (i.e. a windows folder
mounted into the Unix filesystem)
3. Using git inside wsl on a lxfs mount (I.e. native mount such as '/')

In case 1. git does not create a Ubuntu symlink. Also, there are no
Unix symlinks in Windows filesystems, so when Unix tools (such as git)
request to create a symlink, it creates something else (AFAIR it's a
text file containing the symlink path, but having lost its symlink
properties). Case 3 should work.

I still managed to make cases 1 and 2 work by deleting the text file
and re-creating it with windows's "mklink" tool (which requires either
admin rights or developer mode turned on). It seems the WSL layer
translates this as a symlink to the Linux environment.

Michael





Am Mi., 1. Mai 2019 um 16:59 Uhr schrieb Alexandre Ganea via
Phabricator via llvm-commits :
>
> aganea added a subscriber: rnk.
> aganea added a comment.
>
> So it turns out this is a symlink issue. The file 
> `clang/trunk/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas` has been 
> synchronized on my Windows 10 PC as a regular text file, not a symlink. It 
> looks like TortoiseSVN doesn't implement symlinks. As WSL inherits of my file 
> system, it will not find the symbolic link. I suppose `REQUIRES: 
> !system-windows` isn't enough for `cuda-detect-path.cu`, and it would need 
> something like `REQUIRES: symlinks` along with support in lit. @rnk
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D42642/new/
>
> https://reviews.llvm.org/D42642
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via cfe-commits]

Hi,

I had my own difficulties with this. It depends on how the repository
containing the symlink has been checked-out. For instance:

1. Using a windows git (such as git extensions, mingw-git, git for windows)
2. Using git inside wsl on a drvfs mount (i.e. a windows folder
mounted into the Unix filesystem)
3. Using git inside wsl on a lxfs mount (I.e. native mount such as '/')

In case 1. git does not create a Ubuntu symlink. Also, there are no
Unix symlinks in Windows filesystems, so when Unix tools (such as git)
request to create a symlink, it creates something else (AFAIR it's a
text file containing the symlink path, but having lost its symlink
properties). Case 3 should work.

I still managed to make cases 1 and 2 work by deleting the text file
and re-creating it with windows's "mklink" tool (which requires either
admin rights or developer mode turned on). It seems the WSL layer
translates this as a symlink to the Linux environment.

Michael





Am Mi., 1. Mai 2019 um 16:59 Uhr schrieb Alexandre Ganea via
Phabricator via llvm-commits :
>
> aganea added a subscriber: rnk.
> aganea added a comment.
>
> So it turns out this is a symlink issue. The file 
> `clang/trunk/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas` has been 
> synchronized on my Windows 10 PC as a regular text file, not a symlink. It 
> looks like TortoiseSVN doesn't implement symlinks. As WSL inherits of my file 
> system, it will not find the symbolic link. I suppose `REQUIRES: 
> !system-windows` isn't enough for `cuda-detect-path.cu`, and it would need 
> something like `REQUIRES: symlinks` along with support in lit. @rnk
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D42642/new/
>
> https://reviews.llvm.org/D42642
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a subscriber: rnk.
aganea added a comment.

So it turns out this is a symlink issue. The file 
`clang/trunk/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas` has been 
synchronized on my Windows 10 PC as a regular text file, not a symlink. It 
looks like TortoiseSVN doesn't implement symlinks. As WSL inherits of my file 
system, it will not find the symbolic link. I suppose `REQUIRES: 
!system-windows` isn't enough for `cuda-detect-path.cu`, and it would need 
something like `REQUIRES: symlinks` along with support in lit. @rnk


Repository:
  rL LLVM

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

https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld marked an inline comment as done.
Hahnfeld added a comment.
Herald added a subscriber: ormris.

In D42642#1479013 , @aganea wrote:

> Just a quick heads-up - the `cuda-detect-path.cu` test fails on WSL/Ubuntu 
> 18.04:


Hmm, maybe that's because of symlinks. By chance, can you debug print the 
values of `real_path` and `parent_path`? They are supposed to resolve the links 
which is required for the candidate search to work and the test to pass.




Comment at: cfe/trunk/lib/Driver/ToolChains/Cuda.cpp:98-101
+SmallString<256> ptxasAbsolutePath;
+llvm::sys::fs::real_path(*ptxas, ptxasAbsolutePath);
+
+StringRef ptxasDir = llvm::sys::path::parent_path(ptxasAbsolutePath);

These calls.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.
Herald added a project: LLVM.

Just a quick heads-up - the `cuda-detect-path.cu` test fails on WSL/Ubuntu 
18.04:

  aganea@MTL-BJ842:/mnt/f/svn/buildWSL$ /usr/bin/python3.6 bin/llvm-lit -vv 
../clang/test/Driver/cuda-detect-path.cu
  llvm-lit: /mnt/f/svn/llvm/utils/lit/lit/llvm/config.py:341: note: using 
clang: /mnt/f/svn/buildWSL/bin/clang
  -- Testing: 1 tests, single process --
  FAIL: Clang :: Driver/cuda-detect-path.cu (1 of 1)
   TEST 'Clang :: Driver/cuda-detect-path.cu' FAILED 

  
  (output chopped)
  
  + env PATH=/mnt/f/svn/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin 
/mnt/f/svn/buildWSL/bin/clang -v --target=i386-unknown-linux 
--sysroot=/mnt/f/svn/clang/test/Driver/no-cuda-there
  + /mnt/f/svn/buildWSL/bin/FileCheck 
/mnt/f/svn/clang/test/Driver/cuda-detect-path.cu --check-prefix SYMLINKS
  /mnt/f/svn/clang/test/Driver/cuda-detect-path.cu:82:14: error: SYMLINKS: 
expected string not found in input
  // SYMLINKS: Found CUDA installation: {{.*}}/Inputs/CUDA-symlinks/opt/cuda
   ^
  :1:1: note: scanning from here
  clang version 9.0.0 (trunk)
  ^
  :4:1: note: possible intended match here
  InstalledDir: /mnt/f/svn/buildWSL/bin
  ^
  
  aganea@MTL-BJ842:/mnt/f/svn/buildWSL$ env 
PATH=/mnt/f/svn/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin 
/mnt/f/svn/buildWSL/bin/clang -v --target=i386-unknown-linux 
--sysroot=/mnt/f/svn/clang/test/Driver/no-cuda-there
  clang version 9.0.0 (trunk)
  Target: i386-unknown-linux
  Thread model: posix
  InstalledDir: /mnt/f/svn/buildWSL/bin
  aganea@MTL-BJ842:/mnt/f/svn/buildWSL$ ls -l 
/mnt/f/svn/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin
  total 0
  -rwxrwxrwx 1 aganea aganea 29 Feb  1  2018 ptxas


Repository:
  rL LLVM

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

https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323848: [CUDA] Detect installation in PATH (authored by 
Hahnfeld, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42642?vs=132103=132104#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42642

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains/Cuda.cpp
  cfe/trunk/test/Driver/Inputs/CUDA-nolibdevice/usr/local/cuda/bin/ptxas
  cfe/trunk/test/Driver/Inputs/CUDA-symlinks/opt/cuda/bin/ptxas
  cfe/trunk/test/Driver/Inputs/CUDA-symlinks/opt/cuda/include/.keep
  cfe/trunk/test/Driver/Inputs/CUDA-symlinks/opt/cuda/lib/.keep
  
cfe/trunk/test/Driver/Inputs/CUDA-symlinks/opt/cuda/nvvm/libdevice/libdevice.compute_30.10.bc
  
cfe/trunk/test/Driver/Inputs/CUDA-symlinks/opt/cuda/nvvm/libdevice/libdevice.compute_35.10.bc
  cfe/trunk/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas
  cfe/trunk/test/Driver/Inputs/CUDA/usr/local/cuda/bin/ptxas
  cfe/trunk/test/Driver/cuda-detect-path.cu
  cfe/trunk/test/Driver/cuda-detect.cu
  cfe/trunk/test/Driver/cuda-not-found.cu
  cfe/trunk/test/Driver/cuda-version-check.cu

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -556,6 +556,8 @@
 def no_cuda_noopt_device_debug : Flag<["--"], "no-cuda-noopt-device-debug">;
 def cuda_path_EQ : Joined<["--"], "cuda-path=">, Group,
   HelpText<"CUDA installation path">;
+def cuda_path_ignore_env : Joined<["--"], "cuda-path-ignore-env">, Group,
+  HelpText<"Ignore environment variables to detect CUDA installation">;
 def ptxas_path_EQ : Joined<["--"], "ptxas-path=">, Group,
   HelpText<"Path to ptxas (used for compiling CUDA code)">;
 def fcuda_flush_denormals_to_zero : Flag<["-"], "fcuda-flush-denormals-to-zero">,
Index: cfe/trunk/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas
===
--- cfe/trunk/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas
+++ cfe/trunk/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas
@@ -0,0 +1 @@
+link ../../opt/cuda/bin/ptxas
\ No newline at end of file
Index: cfe/trunk/test/Driver/cuda-detect-path.cu
===
--- cfe/trunk/test/Driver/cuda-detect-path.cu
+++ cfe/trunk/test/Driver/cuda-detect-path.cu
@@ -0,0 +1,83 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+// This tests uses the PATH environment variable.
+// REQUIRES: !system-windows
+
+// RUN: env PATH=%S/Inputs/CUDA/usr/local/cuda/bin \
+// RUN:%clang -v --target=i386-unknown-linux --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s
+// RUN: env PATH=%S/Inputs/CUDA/usr/local/cuda/bin \
+// RUN:%clang -v --target=i386-apple-macosx --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s
+// RUN: env PATH=%S/Inputs/CUDA/usr/local/cuda/bin \
+// RUN:%clang -v --target=x86_64-unknown-linux --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s
+// RUN: env PATH=%S/Inputs/CUDA/usr/local/cuda/bin \
+// RUN:%clang -v --target=x86_64-apple-macosx --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s
+
+
+// Check that we follow ptxas binaries that are symlinks.
+// RUN: env PATH=%S/Inputs/CUDA-symlinks/usr/bin \
+// RUN:%clang -v --target=i386-unknown-linux --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s --check-prefix SYMLINKS
+// RUN: env PATH=%S/Inputs/CUDA-symlinks/usr/bin \
+// RUN:%clang -v --target=i386-apple-macosx --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s --check-prefix SYMLINKS
+// RUN: env PATH=%S/Inputs/CUDA-symlinks/usr/bin \
+// RUN:%clang -v --target=x86_64-unknown-linux --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s --check-prefix SYMLINKS
+// RUN: env PATH=%S/Inputs/CUDA-symlinks/usr/bin \
+// RUN:%clang -v --target=x86_64-apple-macosx --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s --check-prefix SYMLINKS
+
+
+// We only take a CUDA installation from PATH if it contains libdevice.
+// RUN: env PATH=%S/Inputs/CUDA-nolibdevice/usr/local/cuda/bin \
+// RUN:%clang -v --target=i386-unknown-linux --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s --check-prefix NOCUDA
+// RUN: env PATH=%S/Inputs/CUDA-nolibdevice/usr/local/cuda/bin \
+// RUN:%clang -v --target=i386-apple-macosx --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s --check-prefix NOCUDA
+// RUN: env PATH=%S/Inputs/CUDA-nolibdevice/usr/local/cuda/bin \
+// RUN:%clang -v --target=x86_64-unknown-linux --sysroot=%S/no-cuda-there \
+// RUN:2>&1 | FileCheck %s --check-prefix NOCUDA
+// RUN: env PATH=%S/Inputs/CUDA-nolibdevice/usr/local/cuda/bin \
+// RUN:%clang -v --target=x86_64-apple-macosx --sysroot=%S/no-cuda-there \
+// 

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 132103.
Hahnfeld marked 3 inline comments as done.
Hahnfeld added a comment.

Add some `sysroot` arguments to new test to make sure it doesn't accidentally 
find CUDA installations in the system.


https://reviews.llvm.org/D42642

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/Inputs/CUDA-nolibdevice/usr/local/cuda/bin/ptxas
  test/Driver/Inputs/CUDA-symlinks/opt/cuda/bin/ptxas
  test/Driver/Inputs/CUDA-symlinks/opt/cuda/include/.keep
  test/Driver/Inputs/CUDA-symlinks/opt/cuda/lib/.keep
  
test/Driver/Inputs/CUDA-symlinks/opt/cuda/nvvm/libdevice/libdevice.compute_30.10.bc
  
test/Driver/Inputs/CUDA-symlinks/opt/cuda/nvvm/libdevice/libdevice.compute_35.10.bc
  test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas
  test/Driver/Inputs/CUDA/usr/local/cuda/bin/ptxas
  test/Driver/cuda-detect-path.cu
  test/Driver/cuda-detect.cu
  test/Driver/cuda-not-found.cu
  test/Driver/cuda-version-check.cu

Index: test/Driver/cuda-version-check.cu
===
--- test/Driver/cuda-version-check.cu
+++ test/Driver/cuda-version-check.cu
@@ -2,50 +2,50 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // The installation at Inputs/CUDA is CUDA 7.0, which doesn't support sm_60.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // This should only complain about sm_60, not sm_35.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_35 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=OK_SM35
 
 // We should get two errors here, one for sm_60 and one for sm_61.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_61 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=ERR_SM61
 
 // We should still get an error if we pass -nocudainc, because this compilation
 // would invoke ptxas, and we do a version check on that, too.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // If with -nocudainc and -E, we don't touch the CUDA install, so we
 // shouldn't get an error.
 // RUN: %clang --target=x86_64-linux -v -### -E --cuda-device-only --cuda-gpu-arch=sm_60 -nocudainc \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // --no-cuda-version-check should suppress all of these errors.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 \
 // RUN:--no-cuda-version-check %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // We need to make sure the version check is done only for the device toolchain,
 // therefore we should not get an error in host-only mode. We use the -S here
 // to avoid the error being produced in case by the assembler tool, which does
 // the same check.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --cuda-path=%S/Inputs/CUDA/usr/local/cuda -S 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-device-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \

[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 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld marked 5 inline comments as done.
Hahnfeld added a subscriber: sylvestre.ledru.
Hahnfeld added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:96-105
+  if (llvm::ErrorOr 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")

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...


https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:96-105
+  if (llvm::ErrorOr 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")

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.



https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 132029.
Hahnfeld added a comment.

Follow symlinked `ptxas` executables.


https://reviews.llvm.org/D42642

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/Inputs/CUDA-nolibdevice/usr/local/cuda/bin/ptxas
  test/Driver/Inputs/CUDA-symlinks/opt/cuda/bin/ptxas
  test/Driver/Inputs/CUDA-symlinks/opt/cuda/include/.keep
  test/Driver/Inputs/CUDA-symlinks/opt/cuda/lib/.keep
  
test/Driver/Inputs/CUDA-symlinks/opt/cuda/nvvm/libdevice/libdevice.compute_30.10.bc
  
test/Driver/Inputs/CUDA-symlinks/opt/cuda/nvvm/libdevice/libdevice.compute_35.10.bc
  test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas
  test/Driver/Inputs/CUDA/usr/local/cuda/bin/ptxas
  test/Driver/cuda-detect-path.cu
  test/Driver/cuda-detect.cu
  test/Driver/cuda-not-found.cu
  test/Driver/cuda-version-check.cu

Index: test/Driver/cuda-version-check.cu
===
--- test/Driver/cuda-version-check.cu
+++ test/Driver/cuda-version-check.cu
@@ -2,50 +2,50 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // The installation at Inputs/CUDA is CUDA 7.0, which doesn't support sm_60.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // This should only complain about sm_60, not sm_35.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_35 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=OK_SM35
 
 // We should get two errors here, one for sm_60 and one for sm_61.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_61 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=ERR_SM61
 
 // We should still get an error if we pass -nocudainc, because this compilation
 // would invoke ptxas, and we do a version check on that, too.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // If with -nocudainc and -E, we don't touch the CUDA install, so we
 // shouldn't get an error.
 // RUN: %clang --target=x86_64-linux -v -### -E --cuda-device-only --cuda-gpu-arch=sm_60 -nocudainc \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // --no-cuda-version-check should suppress all of these errors.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 \
 // RUN:--no-cuda-version-check %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // We need to make sure the version check is done only for the device toolchain,
 // therefore we should not get an error in host-only mode. We use the -S here
 // to avoid the error being produced in case by the assembler tool, which does
 // the same check.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --cuda-path=%S/Inputs/CUDA/usr/local/cuda -S 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-device-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-device-only 

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D42642#992137, @tra wrote:

> I've thought a bit more about this and there's another quirk -- symlinks.
>
> What if we've found /usr/bin/ptxas and is a symlink pointing to the real 
> ptxas in the CUDA installation? If we add /usr to the list of candidates it 
> will not help us at all. We should probably find the real path and add 
> another candidate path derived from it.


Yes, this might be a valid use case, for example linking `/usr/bin/ptxas` to 
`/opt/cuda/bin/ptxas` or something. I think there should be an LLVM function to 
get the `realpath`, let me check that.
(In fact I have symlinks in my environment but it doesn't matter in this case 
because we are just linking the complete CUDA installation to another path...)




Comment at: lib/Driver/ToolChains/Cuda.cpp:206
 // -nocudalib hasn't been specified.
-if (LibDeviceMap.empty() && !Args.hasArg(options::OPT_nocudalib))
+if (CheckLibDevice && LibDeviceMap.empty())
   continue;

tra wrote:
> Hahnfeld wrote:
> > tra wrote:
> > > I think this may be too strict.
> > > 
> > > Checking directory structure for the purposes of detecting CUDA SDK 
> > > should work well enough to weed out false detection for 'split' CUDA 
> > > installation and we've verified libdevice directory presence above. 
> > > 
> > > Checking for libdevice bitcode is somewhat orthogonal to this. IMO, 
> > > regardless of how we've found the installation directory, whether we have 
> > > suitable libdevice version there should not matter if user explicitly 
> > > passed -nocudalib. Insisting on libdevice presence in this situation 
> > > would be somewhat surprising. 
> > > 
> > > 
> > > 
> > > 
> > > 
> > So you are suggesting to revert the change to this line, right?
> Yes, if you agree with my reasoning.
> 
I generally agree that the working bitcode files are orthogonal to the 
directory structure. However I would argue that there won't be a real world 
scenario of an empty directory called `nvvm/libdevice` without any libdevice 
files in there. That said it probably doesn't make a difference, so I'll follow 
the separation of concerns.


https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I've thought a bit more about this and there's another quirk -- symlinks.

What if we've found /usr/bin/ptxas and is a symlink pointing to the real ptxas 
in the CUDA installation? If we add /usr to the list of candidates it will not 
help us at all. We should probably find the real path and add another candidate 
path derived from it.




Comment at: lib/Driver/ToolChains/Cuda.cpp:206
 // -nocudalib hasn't been specified.
-if (LibDeviceMap.empty() && !Args.hasArg(options::OPT_nocudalib))
+if (CheckLibDevice && LibDeviceMap.empty())
   continue;

Hahnfeld wrote:
> tra wrote:
> > I think this may be too strict.
> > 
> > Checking directory structure for the purposes of detecting CUDA SDK should 
> > work well enough to weed out false detection for 'split' CUDA installation 
> > and we've verified libdevice directory presence above. 
> > 
> > Checking for libdevice bitcode is somewhat orthogonal to this. IMO, 
> > regardless of how we've found the installation directory, whether we have 
> > suitable libdevice version there should not matter if user explicitly 
> > passed -nocudalib. Insisting on libdevice presence in this situation would 
> > be somewhat surprising. 
> > 
> > 
> > 
> > 
> > 
> So you are suggesting to revert the change to this line, right?
Yes, if you agree with my reasoning.



https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:206
 // -nocudalib hasn't been specified.
-if (LibDeviceMap.empty() && !Args.hasArg(options::OPT_nocudalib))
+if (CheckLibDevice && LibDeviceMap.empty())
   continue;

tra wrote:
> I think this may be too strict.
> 
> Checking directory structure for the purposes of detecting CUDA SDK should 
> work well enough to weed out false detection for 'split' CUDA installation 
> and we've verified libdevice directory presence above. 
> 
> Checking for libdevice bitcode is somewhat orthogonal to this. IMO, 
> regardless of how we've found the installation directory, whether we have 
> suitable libdevice version there should not matter if user explicitly passed 
> -nocudalib. Insisting on libdevice presence in this situation would be 
> somewhat surprising. 
> 
> 
> 
> 
> 
So you are suggesting to revert the change to this line, right?


https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:206
 // -nocudalib hasn't been specified.
-if (LibDeviceMap.empty() && !Args.hasArg(options::OPT_nocudalib))
+if (CheckLibDevice && LibDeviceMap.empty())
   continue;

I think this may be too strict.

Checking directory structure for the purposes of detecting CUDA SDK should work 
well enough to weed out false detection for 'split' CUDA installation and we've 
verified libdevice directory presence above. 

Checking for libdevice bitcode is somewhat orthogonal to this. IMO, regardless 
of how we've found the installation directory, whether we have suitable 
libdevice version there should not matter if user explicitly passed -nocudalib. 
Insisting on libdevice presence in this situation would be somewhat surprising. 







https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D42642#991154, @tra wrote:

> In https://reviews.llvm.org/D42642#991127, @Hahnfeld wrote:
>
> > In https://reviews.llvm.org/D42642#990976, @tra wrote:
> >
> > > Some linux distributions integrate CUDA into the standard directory 
> > > structure. I.e. binaries go into /usr/bin, headers into /usr/include, 
> > > bitcode goes somewhere else, etc. ptxas will be found, but we would still 
> > > fail to detect CUDA. I'd add  one more test case to make sure that's 
> > > still the case.
> >
> >
> > I'm not sure how this can work, we only require `bin/` and `include/` to 
> > exist, and `nvvm/libdevice/` if `-nocudalib` isn't specified. I agree this 
> > can be a problem because the defaults might detect an invalid 
> > installation...
>
>
> Good point. Perhaps we want to be more strict about CUDA installation 
> checking if we've found it indirectly via PATH (as opposed to explicit 
> --cuda-path or a known common install path).
>  Should we always check for nvvm/libdevice directory? It's unlikely to be 
> under /usr or /usr/local and it will be always present in a CUDA installation 
> of all currently supported CUDA versions.


That sounds like a sane check. I updated the implementation and added 
corresponding checks.


https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 131963.
Hahnfeld added a comment.

Check for `libdevice` in candidates from `PATH`.


https://reviews.llvm.org/D42642

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/Inputs/CUDA-nolibdevice/usr/local/cuda/bin/ptxas
  test/Driver/Inputs/CUDA/usr/local/cuda/bin/ptxas
  test/Driver/cuda-detect-path.cu
  test/Driver/cuda-detect.cu
  test/Driver/cuda-not-found.cu
  test/Driver/cuda-version-check.cu

Index: test/Driver/cuda-version-check.cu
===
--- test/Driver/cuda-version-check.cu
+++ test/Driver/cuda-version-check.cu
@@ -2,50 +2,50 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // The installation at Inputs/CUDA is CUDA 7.0, which doesn't support sm_60.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // This should only complain about sm_60, not sm_35.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_35 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=OK_SM35
 
 // We should get two errors here, one for sm_60 and one for sm_61.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_61 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=ERR_SM61
 
 // We should still get an error if we pass -nocudainc, because this compilation
 // would invoke ptxas, and we do a version check on that, too.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // If with -nocudainc and -E, we don't touch the CUDA install, so we
 // shouldn't get an error.
 // RUN: %clang --target=x86_64-linux -v -### -E --cuda-device-only --cuda-gpu-arch=sm_60 -nocudainc \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // --no-cuda-version-check should suppress all of these errors.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 \
 // RUN:--no-cuda-version-check %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // We need to make sure the version check is done only for the device toolchain,
 // therefore we should not get an error in host-only mode. We use the -S here
 // to avoid the error being produced in case by the assembler tool, which does
 // the same check.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --cuda-path=%S/Inputs/CUDA/usr/local/cuda -S 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-device-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-device-only --cuda-path=%S/Inputs/CUDA/usr/local/cuda -S 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // OK-NOT: error: GPU arch
Index: test/Driver/cuda-not-found.cu
===
--- test/Driver/cuda-not-found.cu
+++ test/Driver/cuda-not-found.cu
@@ -3,10 +3,10 @@
 // Check that we raise an error if we're trying to compile CUDA code but can't
 // find 

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

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In https://reviews.llvm.org/D42642#991127, @Hahnfeld wrote:

> In https://reviews.llvm.org/D42642#990976, @tra wrote:
>
> > Some linux distributions integrate CUDA into the standard directory 
> > structure. I.e. binaries go into /usr/bin, headers into /usr/include, 
> > bitcode goes somewhere else, etc. ptxas will be found, but we would still 
> > fail to detect CUDA. I'd add  one more test case to make sure that's still 
> > the case.
>
>
> I'm not sure how this can work, we only require `bin/` and `include/` to 
> exist, and `nvvm/libdevice/` if `-nocudalib` isn't specified. I agree this 
> can be a problem because the defaults might detect an invalid installation...


Good point. Perhaps we want to be more strict about CUDA installation checking 
if we've found it indirectly via PATH (as opposed to explicit --cuda-path or a 
known common install path).
Should we always check for nvvm/libdevice directory? It's unlikely to be under 
/usr or /usr/local and it will be always present in a CUDA installation of all 
currently supported CUDA versions.


Repository:
  rC Clang

https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D42642#990976, @tra wrote:

> Some linux distributions integrate CUDA into the standard directory 
> structure. I.e. binaries go into /usr/bin, headers into /usr/include, bitcode 
> goes somewhere else, etc. ptxas will be found, but we would still fail to 
> detect CUDA. I'd add  one more test case to make sure that's still the case.


I'm not sure how this can work, we only require `bin/` and `include/` to exist, 
and `nvvm/libdevice/` if `-nocudalib` isn't specified. I agree this can be a 
problem because the defaults might detect an invalid installation...

In https://reviews.llvm.org/D42642#991013, @jlebar wrote:

> Can we document this behavior in 
> https://llvm.org/docs/CompileCudaWithLLVM.html (in the LLVM repo)?  Totally 
> fine if you want to do this in a separate patch.


Sure, will do!


Repository:
  rC Clang

https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

Can we document this behavior in https://llvm.org/docs/CompileCudaWithLLVM.html 
(in the LLVM repo)?  Totally fine if you want to do this in a separate patch.


Repository:
  rC Clang

https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

Some linux distributions integrate CUDA into the standard directory structure. 
I.e. binaries go into /usr/bin, headers into /usr/include, bitcode goes 
somewhere else, etc. ptxas will be found, but we would still fail to detect 
CUDA. I'd add  one more test case to make sure that's still the case.

Otherwise - LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D42642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: jlebar, tra.
Herald added a subscriber: cfe-commits.

If the CUDA toolkit is not installed to its default locations
in /usr/local/cuda, the user is forced to specify --cuda-path.
This is tedious and the driver can be smarter if well-known tools
(like ptxas) can already be found in the PATH environment variable.

Add option --cuda-path-ignore-env if the user wants to ignore
set environment variables. Also use it in the tests to make sure
the driver always finds the same CUDA installation, regardless
of the user's environment.


Repository:
  rC Clang

https://reviews.llvm.org/D42642

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/Inputs/CUDA/usr/local/cuda/bin/ptxas
  test/Driver/cuda-detect-path.cu
  test/Driver/cuda-detect.cu
  test/Driver/cuda-not-found.cu
  test/Driver/cuda-version-check.cu

Index: test/Driver/cuda-version-check.cu
===
--- test/Driver/cuda-version-check.cu
+++ test/Driver/cuda-version-check.cu
@@ -2,50 +2,50 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // The installation at Inputs/CUDA is CUDA 7.0, which doesn't support sm_60.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // This should only complain about sm_60, not sm_35.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_35 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=OK_SM35
 
 // We should get two errors here, one for sm_60 and one for sm_61.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_61 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=ERR_SM61
 
 // We should still get an error if we pass -nocudainc, because this compilation
 // would invoke ptxas, and we do a version check on that, too.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // If with -nocudainc and -E, we don't touch the CUDA install, so we
 // shouldn't get an error.
 // RUN: %clang --target=x86_64-linux -v -### -E --cuda-device-only --cuda-gpu-arch=sm_60 -nocudainc \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // --no-cuda-version-check should suppress all of these errors.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 \
 // RUN:--no-cuda-version-check %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // We need to make sure the version check is done only for the device toolchain,
 // therefore we should not get an error in host-only mode. We use the -S here
 // to avoid the error being produced in case by the assembler tool, which does
 // the same check.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --cuda-path=%S/Inputs/CUDA/usr/local/cuda -S 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-device-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60