tra added inline comments.

================
Comment at: lib/Driver/ToolChains.cpp:4773
@@ +4772,3 @@
+  StringRef GpuArch =
+      DriverArgs.getLastArgValue(options::OPT_march_EQ, "sm_20");
+  std::string LibDeviceFile = CudaInstallation.getLibDeviceFile(GpuArch);
----------------
jlebar wrote:
> I *think* march is never empty here.  We should add it when we create the 
> CudaActions.
> 
> Same below.
> 
> If I'm right or if I'm wrong, let's make sure there are tests.  :)
It's guaranteed to be non-empty here only when we're constructing device-side 
pipeline which *is* normally the case.

Alas, it's possible for user to specify a nonsensical set of arguments that 
would trigger this assertion:

For instance: `clang --cuda-host-only -target nvptx-nvidia-cuda -x cuda` used 
by test/Preprocessor/cuda-types.cu.

Because target is CUDA, we do pick CudaToolchain, but because it's a host 
compilation, we never get to add -march=sm_XX (TC->TranslateArgs is called with 
BoundArch=nullptr).

Dafaulting to somewhat safe sm_20 on user error seems better than crashing 
compiler with an assertion.

================
Comment at: test/Driver/cuda-detect.cu:66
@@ -63,2 +65,2 @@
 // NOCUDAINC-NOT: "-include" "__clang_cuda_runtime_wrapper.h"
 // COMMON-SAME: "-x" "cuda"
----------------
jlebar wrote:
> Should we test the new sm_XX --> libdevice file mappings added?
OK.


https://reviews.llvm.org/D23037



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

Reply via email to