[PATCH] D21869: [CUDA] Check that our CUDA install supports the requested architectures.

[Justin Lebar via cfe-commits]

jlebar added inline comments.

================
Comment at: include/clang/Driver/Options.td:1722-1724
@@ -1721,2 +1721,5 @@
 def nocudalib : Flag<["-"], "nocudalib">;
+def nocuda_version_check : Flag<["-"], "nocuda-version-check">,
+  HelpText<"Don't error out if the detected version of the CUDA install is "
+           "too low for the requested CUDA gpu architecture.">;
 def nodefaultlibs : Flag<["-"], "nodefaultlibs">;
----------------
tra wrote:
> IMO we should use double-dash for this option. 
> 
> -nocudalib/-nocudainc were mimicking existing -nostdlib/-nostdinc and thus ended with single dash in front.
> --nocuda-version-check is not constrained by this.
I guess I have to do a double dash because --no-cuda-noopt-device-debug is thus spelled.  Notice there's also a dash after "no".

...this is so screwed up.

================
Comment at: lib/Driver/ToolChains.cpp:1793
@@ +1792,3 @@
+        FS.getBufferForFile(InstallPath + "/version.txt");
+    if (!VersionFile) {
+      // CUDA 7.0 doesn't have a version.txt, so guess that's our version if
----------------
tra wrote:
> What if it's cuda-8, but we've failed to read the file due to permissions.
> Perhaps we should differentiate no-such-file from other errors.
Urgh.

I feel like simpler is probably better.  Like, I would rather say:

- We consider /usr/local/cuda, /usr/local/cuda-8.0, ...
- We pick the first one that has a few key files
- We infer its version by looking for version.txt.  If we can't read it, for whatever reason, we assume 7.0.

Than the more complicated

- We infer its version by looking for version.txt.  If it's present but we can't read it, we try to look at the file path (/usr/local/cuda-8.0/...).

Especially because as we make things more complicated, our next step will probably be to change the algorithm to something like:

- For each directory D in /usr/local/cuda, /usr/local/cuda-8.0, ...
  - Infer D's version using the algorithm above.
  - If D is not compatible with all of the cuda_archs specified, break and try the next one...

I do agree that if someone hits a permission problem on version.txt, that would be very difficult for them to debug.  I suppose we could raise a proper error if version.txt is present but not readable?  I'm not sure it's worth doing that, though...

================
Comment at: lib/Driver/ToolChains.cpp:4711
@@ +4710,3 @@
+  // Check our CUDA version if we're going to include the CUDA headers.
+  if (!DriverArgs.hasArg(options::OPT_nocudainc) &&
+      !DriverArgs.hasArg(options::OPT_nocuda_version_check)) {
----------------
tra wrote:
> -nocudainc should not preclude CUDA version check, IMO.
> Primary use of version check is to make sure we can compile for a given GPU architecture. I think we still want to do that even if -nocudainc is passed.
We also do a check if you invoke ptxas.  So the only circumstances under which we don't do a check is -nocudainc and no invocation of ptxas.  In which case the only thing we're using from the CUDA install is fatbinary, which doesn't care about the sm version (afaik).  We could add a version check on the fatbin invocation too if you want.  My goal was just that if we don't use anything from the CUDA install, we should not check its version.


http://reviews.llvm.org/D21869