gtbercea added inline comments.
================
Comment at: lib/Driver/ToolChains/Cuda.cpp:170-182
- // This code prevents IsValid from being set when
- // no libdevice has been found.
- bool allEmpty = true;
- std::string LibDeviceFile;
- for (auto key : LibDeviceMap.keys()) {
- LibDeviceFile = LibDeviceMap.lookup(key);
- if (!LibDeviceFile.empty())
----------------
gtbercea wrote:
> tra wrote:
> > tra wrote:
> > > gtbercea wrote:
> > > > gtbercea wrote:
> > > > > Hahnfeld wrote:
> > > > > > tra wrote:
> > > > > > > Hahnfeld wrote:
> > > > > > > > tra wrote:
> > > > > > > > > I'd keep this code. It appears to serve useful purpose as it
> > > > > > > > > requires CUDA installation to have at least some libdevice
> > > > > > > > > library in it. It gives us a change to find a valid
> > > > > > > > > installation, instead of ailing some time later when we ask
> > > > > > > > > for a libdevice file and fail because there are none.
> > > > > > > > We had some internal discussions about this after I submitted
> > > > > > > > the patch here.
> > > > > > > >
> > > > > > > > The main question is: Do we want to support CUDA installations
> > > > > > > > without libdevice and are there use cases for that? I'd say
> > > > > > > > that the user should be able to use a toolchain without
> > > > > > > > libdevice together with `-nocudalib`.
> > > > > > > Sounds reasonable. How about keeping the code but putting it
> > > > > > > under `if(!hasArg(nocudalib))`?
> > > > > > >
> > > > > > Ok, I'll do that in a separate patch and keep the code here for now.
> > > > > The problem with nocudalib is that if for example you write a test,
> > > > > which looks to verify some device facing feature that requires a
> > > > > libdevice to be found (so you don't want to use nocudalib), it will
> > > > > probably work on your machine which has the correct CUDA setup but
> > > > > fail on another machine which does not (which is where you want to
> > > > > use nocudalib). You can see the contradiction there.
> > > > Just to be clear I am arguing for keeping this code :)
> > > @gtbercea: I'm not sure I follow your example. If you're talking about
> > > clang tests, we do have fake CUDA installation setup under
> > > test/Driver/Inputs which removes dependency on whatever CUDA you may or
> > > may not have installed on your machine. I also don't see a contradiction
> > > -- you you do need libdevice, it makes no point picking a broken CUDA
> > > installation which does not have any libdevice files. If you explicitly
> > > tell compiler that you don't need libdevice, that would make CUDA w/o
> > > libdevice acceptable. With --cuda-path you do have a way to tell clang
> > > which installation you want it to use. What do I miss?
> > >
> > >
> > Ah, you were arguing with Hahnfeld@'s -nocudalib example. Then I guess
> > we're in violent agreement.
> I fully agree with this: "you do need libdevice, it makes no point picking a
> broken CUDA installation which does not have any libdevice files. If you
> explicitly tell compiler that you don't need libdevice, that would make CUDA
> w/o libdevice acceptable."
>
> I was trying to show an example of a situation where you have your code
> compiled using nocudalib on one machine and then the same code will error on
> a machine which requires the nocudalib flag to be passed to make up for the
> absence of libdevice.
>
>
Yes it was a counter argument to that! :)
https://reviews.llvm.org/D38883
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
