<div dir="ltr">I don't really understand why having to change the test when we change the code it test changes...<div><br></div><div>We have several fake install trees in the driver tests to check pretty much exactly these kinds of things?</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Apr 19, 2016 at 11:31 AM Justin Lebar via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Yes, in general our testing story around the CUDA installs needs work.<br>
In particular, our wrapper headers are complicated and fragile, and<br>
have zero coverage at the moment.  That's why Art is working on<br>
getting CUDA tests into the test-suite.<br>
<br>
It's possible to test this particular change without access to a full<br>
CUDA installation.  However, I was hesitant to do so, because such a<br>
test would have to create a fake cuda installation.  Our test would<br>
then be fragile with respect to exactly how clang detects that a CUDA<br>
install is "real".  If we changed those heuristics, we'd have to<br>
change our test.<br>
<br>
On Tue, Apr 19, 2016 at 11:21 AM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> wrote:<br>
> This commit is missing a test.<br>
><br>
><br>
> On Fri, Apr 15, 2016 at 5:16 PM Justin Lebar via cfe-commits<br>
> <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> Author: jlebar<br>
>> Date: Fri Apr 15 19:11:11 2016<br>
>> New Revision: 266496<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=266496&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=266496&view=rev</a><br>
>> Log:<br>
>> [CUDA] Raise an error if the CUDA install can't be found.<br>
>><br>
>> Summary:<br>
>> Without this change, we silently proceed on without including<br>
>> __clang_cuda_runtime_wrapper.h.  This leads to very strange behavior --<br>
>> you say you're compiling CUDA code, but e.g. __device__ is not defined!<br>
>><br>
>> Reviewers: tra<br>
>><br>
>> Subscribers: cfe-commits<br>
>><br>
>> Differential Revision: <a href="http://reviews.llvm.org/D19180" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19180</a><br>
>><br>
>> Modified:<br>
>>     cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td<br>
>>     cfe/trunk/lib/Driver/ToolChains.cpp<br>
>><br>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=266496&r1=266495&r2=266496&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=266496&r1=266495&r2=266496&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)<br>
>> +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Apr 15<br>
>> 19:11:11 2016<br>
>> @@ -23,6 +23,9 @@ def err_drv_unknown_language : Error<"la<br>
>>  def err_drv_invalid_arch_name : Error<<br>
>>    "invalid arch name '%0'">;<br>
>>  def err_drv_cuda_bad_gpu_arch : Error<"Unsupported CUDA gpu architecture:<br>
>> %0">;<br>
>> +def err_drv_no_cuda_installation : Error<<br>
>> +  "cannot find CUDA installation.  Provide its path via --cuda-path, or<br>
>> pass "<br>
>> +  "-nocudainc to build without CUDA includes.">;<br>
>>  def err_drv_invalid_thread_model_for_target : Error<<br>
>>    "invalid thread model '%0' in '%1' for this target">;<br>
>>  def err_drv_invalid_linker_name : Error<<br>
>><br>
>> Modified: cfe/trunk/lib/Driver/ToolChains.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=266496&r1=266495&r2=266496&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=266496&r1=266495&r2=266496&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- cfe/trunk/lib/Driver/ToolChains.cpp (original)<br>
>> +++ cfe/trunk/lib/Driver/ToolChains.cpp Fri Apr 15 19:11:11 2016<br>
>> @@ -4118,11 +4118,14 @@ void Linux::AddCudaIncludeArgs(const Arg<br>
>>    if (DriverArgs.hasArg(options::OPT_nocudainc))<br>
>>      return;<br>
>><br>
>> -  if (CudaInstallation.isValid()) {<br>
>> -    addSystemInclude(DriverArgs, CC1Args,<br>
>> CudaInstallation.getIncludePath());<br>
>> -    CC1Args.push_back("-include");<br>
>> -    CC1Args.push_back("__clang_cuda_runtime_wrapper.h");<br>
>> +  if (!CudaInstallation.isValid()) {<br>
>> +    getDriver().Diag(diag::err_drv_no_cuda_installation);<br>
>> +    return;<br>
>>    }<br>
>> +<br>
>> +  addSystemInclude(DriverArgs, CC1Args,<br>
>> CudaInstallation.getIncludePath());<br>
>> +  CC1Args.push_back("-include");<br>
>> +  CC1Args.push_back("__clang_cuda_runtime_wrapper.h");<br>
>>  }<br>
>><br>
>>  bool Linux::isPIEDefault() const { return<br>
>> getSanitizerArgs().requiresPIE(); }<br>
>><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>