[PATCH] D38883: [CMake][OpenMP] Customize default offloading arch

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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