tra added inline comments.

================
Comment at: lib/Driver/ToolChains/Cuda.h:90
-  }
 };
 
----------------
gtbercea wrote:
> gtbercea wrote:
> > tra wrote:
> > > gtbercea wrote:
> > > > gtbercea wrote:
> > > > > I would also like to keep the spirit of this code if not in this 
> > > > > exact form at least something that performs the same functionality.
> > > > @tra what's your opinion on this code? Should this stay, stay but 
> > > > modified to be more robust or taken out completely?
> > > There are currently no users for this. In general, I would rather not 
> > > have magically-changing default GPU based on how broken your CUDA 
> > > installation is. IMO it would be better to keep defaults static and fail 
> > > if prerequisites are not met.
> > I would have thought that it is up to the compiler to select, as default, 
> > the lowest viable compute capability. This is what this code aims to do 
> > (whether it actually does that's a separate issue :) ).
> > 
> The reason I added this code in the first place was to overcome the fact that 
> something like a default of sm_30 may work on the K40 but once you go to 
> newer Pascal, Volta GPUs then you need a new minimum compute capability that 
> is supported.
> Should this stay, stay but modified to be more robust or taken out completely?

I'd take it out, at least for now as you've removed the only user of that 
function.

In general, though, compilers tend to use conservative defaults and for CUDA 
that would be the lowest GPU variant supported by compiler. In case of CUDA 
it's determined by the CUDA SDK version. Figuring lowers supported version via 
libdevice mapping we've created is wrong. E.g. with this patch and -nocudalib 
you may end up using CUDA-9 without any libdevice and would return sm_20.

If/when we need to figure out minimum supported version, it should be based 
directly on the value returned by version().


https://reviews.llvm.org/D38883



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to