[CUDA] Download instead of having the user install it.#35336
[CUDA] Download instead of having the user install it.#35336Neumann-A wants to merge 51 commits intomicrosoft:masterfrom
Conversation
|
Tested the installation of |
BillyONeal
left a comment
There was a problem hiding this comment.
I am concerned that by effectively creating our own CUDA distribution that isn't identical to real CUDA (with CUDA_HOME and all that stuff) that this behavior is likely to be ... 'surprising'.
Also, this copies stuff from CUDA into the installed tree; I'm not sure that's desirable.
(I'm not 'request changes' here because these are closer to 'questions' than 'this needs to change')
| set(platform linux) | ||
| endif() | ||
|
|
||
| set(components |
There was a problem hiding this comment.
Where did these lists come from?
There was a problem hiding this comment.
Asking this questions 5 months after the fact is a bit hard. But probably just open https://developer.download.nvidia.com/compute/cuda/redist/ in a web browser + look at the redistrib_x.y.z.json files there and also checking conda-forge recipes.
| ) | ||
|
|
||
| set(util | ||
| #nsight_compute |
There was a problem hiding this comment.
My understanding is that nsight is some kind of developer gui tool so its OK that it isn't included here, but it does highlight a concern: If folks were using vcpkg with something nsight, before they were getting the matching bits for their copy of CUDA, now they might get mismatched bits. Do we have folks who actually use cuda regularly to common on the practicality of this idea?
There was a problem hiding this comment.
Nobody stops you from installing the exact same version of cuda vcpkg would install. (If it even matters. From my understanding Nsight is basically a profiler like vtune to figure out bottlenecks. )
|
|
||
| file(READ "${cuda_redist_json}" cuda_json) | ||
|
|
||
| file(MAKE_DIRECTORY "${CURRENT_PACKAGES_DIR}/tools/cuda") |
There was a problem hiding this comment.
Unfortunately I think this implicates the tool ports thing since this is going to try to provide nvcc...
There was a problem hiding this comment.
It is not trying to provide nvcc it is providing nvcc. (but you could also use the installation with clang to compile cuda)
ports/cuda/portfile.cmake
Outdated
| # Don't really know why this renaming is required. | ||
| file(RENAME "${CURRENT_PACKAGES_DIR}/tools/cuda/lib" "${CURRENT_PACKAGES_DIR}/tools/cuda/lib64") |
There was a problem hiding this comment.
In general lib vs. lib64 depends on one's distro...
There was a problem hiding this comment.
tuned to what nvcc expects.
| foreach(pc_file IN LISTS pc_files) | ||
| file(READ "${pc_file}" contents) | ||
| string(REGEX REPLACE "cudaroot=[^\n]+" "cudaroot=\${prefix}/tools/cuda" contents "${contents}") | ||
| #string(REGEX REPLACE "/targets/x86_64-linux" "" contents "${contents}") |
There was a problem hiding this comment.
There's a lot of commented out code in here; are you still working on this?
There was a problem hiding this comment.
Hmm below 5% lines are comments in this file is considered a lot?
I probably expected this to be needed to be changed but there was no pc file consumer for cuda yet so it couldn't be properly tested.
ports/cuda/portfile.cmake
Outdated
| set(VCPKG_POLICY_SKIP_ARCHITECTURE_CHECK enabled) | ||
| set(VCPKG_POLICY_ONLY_RELEASE_CRT enabled) | ||
|
|
||
| set(VCPKG_POLICY_EMPTY_INCLUDE_FOLDER enabled) |
There was a problem hiding this comment.
CUDA has headers so if you need this skip it seems like something is wrong? I would get this if it only made bits pointing to a separate cuda install somewhere, but it isn't doing that.
There was a problem hiding this comment.
headers are not located in /include but /tools/cuda/... (whatever nvcc expects)
I think there needs to be a description in this port describing the differences between how it installs things and how things are in a 'CUDA_HOME'. |
|
Can you describe this in a comment in the port? |
Added a comment in 6573e17 |
|
One issue here is that the license files seem to change over time. |
|
It looks like they stopped with the 'sign one's life away' to download cudnn; https://developer.download.nvidia.com/compute/cudnn/redist/cudnn/windows-x86_64/cudnn-windows-x86_64-9.2.0.82_cuda12-archive.zip looks like a normal URI now. |
|
(I'm just observing changes relevant to this review as I'm updating the build lab to CUDA 12.5, sorry for the spam) |
# Conflicts: # ports/cudnn/FindCUDNN.cmake # ports/cudnn/portfile.cmake # ports/cudnn/vcpkg.json # versions/baseline.json # versions/c-/cudnn.json

(Only tested with windows currently.)