r266496 - [CUDA] Raise an error if the CUDA install can't be found.

Justin Lebar via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 19 11:42:43 PDT 2016


> 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 <chandlerc at 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 at 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 <chandlerc at 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 at 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 at lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


More information about the cfe-commits mailing list