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



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

Reply via email to