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()) ---------------- 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. https://reviews.llvm.org/D38883 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits