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

[Artem Belevich via cfe-commits]

tra added inline comments.

================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:32
@@ -29,1 +31,3 @@
+  "Use --cuda-path to specify a different CUDA install, or pass "
+  "--nocuda-version-check.">;
 def err_drv_invalid_thread_model_for_target : Error<
----------------
Is it supposed to be two dashes or one?


================
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">;
----------------
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.

================
Comment at: lib/Driver/ToolChains.cpp:1704
@@ -1702,1 +1703,3 @@
 
+// Parses the contents of version.txt in an CUDA installation.
+static CudaVersion ParseCudaVersionFile(llvm::StringRef V) {
----------------
Might add an example of what's in that file, so it's easier to understand what the code below is doing.

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

================
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)) {
----------------
-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.


http://reviews.llvm.org/D21869