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