<div dir="ltr">Thanks!</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Apr 19, 2016 at 11:58 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">Committed a test in r266796.<br>
<br>
On Tue, Apr 19, 2016 at 11:42 AM, Justin Lebar <<a href="mailto:jlebar@google.com" target="_blank">jlebar@google.com</a>> wrote:<br>
>> I don't really understand why having to change the test when we change the code it test changes...<br>
><br>
> My thought was, this code isn't really testing how we detect a CUDA<br>
> installation. That's a separate matter, involving --cuda-path,<br>
> various default locations we check, and so on.<br>
><br>
> Anyway I now see that we're already biting that cost with existing<br>
> tests, so adding a test for this shouldn't be a big deal.<br>
><br>
> On Tue, Apr 19, 2016 at 11:33 AM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> wrote:<br>
>> I don't really understand why having to change the test when we change the<br>
>> code it test changes...<br>
>><br>
>> We have several fake install trees in the driver tests to check pretty much<br>
>> exactly these kinds of things?<br>
>><br>
>> On Tue, Apr 19, 2016 at 11:31 AM 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>
>>> 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>><br>
>>> 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>
>>> >><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>
>>> >> ==============================================================================<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<br>
>>> >> architecture:<br>
>>> >> %0">;<br>
>>> >> +def err_drv_no_cuda_installation : Error<<br>
>>> >> + "cannot find CUDA installation. Provide its path via --cuda-path,<br>
>>> >> 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>
>>> >><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>
>>> >> ==============================================================================<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>
_______________________________________________<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>