mojca added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:131
+ std::initializer_list<const char *> Versions = {
+ "11.5", "11.4", "11.3", "11.2", "11.1", "11.0", "10.2", "10.1",
+ "10.0", "9.2", "9.1", "9.0", "8.0", "7.5", "7.0"};
----------------
tra wrote:
> mojca wrote:
> > kadircet wrote:
> > > looks like the list is getting big and hard to maintain. considering that
> > > this is done only once per compiler invocation (and we check for
> > > existence of directories down in the loop anyway). what about throwing in
> > > an extra directory listing to base-directories mentioned down below and
> > > populate `Candidates` while preserving the newest-version-first order?
> > I totally agree with the sentiment, and that was my initial thought as
> > well, but with zero experience I was too scared to make any more
> > significant changes.
> >
> > I can try to come up with a new patch (that doesn't need further
> > maintenance whenever a new CUDA version gets released) if that's what you
> > are suggesting. I would nevertheless merge this one, and prepare a new more
> > advanced patch separately, but that's finally your call.
> >
> > What's your suggestion about D.SysRoot + "Program Files/..."? At the time
> > when this function gets called it looks like D.SysRoot is empty (or at
> > least my debugger says so) and in my case it resolves to D: while the CUDA
> > support is installed under C:.
> >
> > Is there any special LLVM-specific/preferrable way to iterate through
> > directories?
> >
> > (What I also miss a bit in the whole process in an option to simply say "I
> > want CUDA 11.1" without the need to explicitly spell out the full path.)
> >
> > If you provide me give some general guidelines, I'll prepare another,
> > hopefully more future-proof patch.
> >
> > (Side note: I'm not sure if I'm calling clang-format correctly, but if I
> > call it, it keeps reformatting the rest of this file.)
> This whole list may no longer be particularly useful. The most common use
> case on Linux, AFAICT, is to install only one CUDA version using
> system-provided package manager.
> E.g. https://packages.ubuntu.com/focal/amd64/nvidia-cuda-toolkit/filelist
>
> TBH, I'm tempted to limit autodetection to only that one system-default
> version and require user to use --cuda-path if they need something else.
On Windows this is certainly not the case. Unless the installation is changed
manually, one always gets the new version installed into a new directory.
I really do need multiple versions on Windows (and the ability to pick an older
one) if I want to compile a binary that works on someone else's computer (if I
compile against the latest CUDA, users need "the latest" drivers that may
sometimes not even be available for their machine).
(That said, at least when using CMake, the selection needs to be done by CMake
anyway, and maybe CMake could be forced to specify the correct flag
automatically.)
So even if the functionality gets removed from non-Windows platforms, it would
be really nice to keep it for Windows.
Now, there are three "conflicting" feedbacks/suggestions above. I can try to
improve support, but it would be really helpful to reach the consensus of what
needs to be done before coding it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114326/new/
https://reviews.llvm.org/D114326
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
