Use Legion with CMake's native CUDA language#725
Use Legion with CMake's native CUDA language#725trxcllnt wants to merge 99 commits intonv-legate:branch-23.09from
Conversation
trxcllnt
added
the
category:improvement
| LIBRARY_OUTPUT_DIRECTORY lib) | ||
|
|
||
| if(Legion_USE_CUDA) | ||
| set_property(TARGET legate_core PROPERTY CUDA_ARCHITECTURES ${Legion_CUDA_ARCH}) |
There was a problem hiding this comment.
It seems like we now have a potential conflict between CMAKE_CUDA_ARCHITECTURES and Legion_CUDA_ARCH. If someone were to specify -DCMAKE_CUDA_ARCHITECTURES=80, e.g., it will just be silently ignored because of this line.
Or is this okay because of the new line in Legion:
if(CMAKE_CUDA_ARCHITECTURES)
set(Legion_CUDA_ARCH ${CMAKE_CUDA_ARCHITECTURES})
?
There was a problem hiding this comment.
Yeah right now if someone sets -DCMAKE_CUDA_ARCHITECTURES= (or has set it before add_subdirectory(./legion)), then I made Legion use it instead of -DLegion_CUDA_ARCH=. It feels a bit backward at first since -DLegion_CUDA_ARCH seems more project-specific than -DCMAKE_CUDA_ARCHITECTURES=.
But Legion_CUDA_ARCH is parsed (commas -> semis) and defaults to all-major if unset, so I wanted to have an "escape hatch" if someone wants to force an exact list of GPU architectures that skips (or resets) all of Legion's arch logic.
There was a problem hiding this comment.
Oh but you're right in the find_package(Legion) case. Now legate.core ignores the -DCMAKE_CUDA_ARCHITECTURES argument and use whatever archs Legion used. This mirrors how if we find a Legion that built with -DLegion_USE_CUDA=OFF, legate.core uses that value even if the user configures with -DLegion_USE_CUDA=ON.
There was a problem hiding this comment.
Hmm... I think that's fine. I guess we might at least want a warning in that situation. Or do we want to FATAL_ERROR if someone tries to set CMAKE_CUDA_ARCHITECTURES in that situation and it's totally disallowed?
cmake/versions.json
Outdated
| "git_url" : "https://gitlab.com/StanfordLegion/legion.git", | ||
| "git_tag" : "915f481e5daa9485344866062441d4ce4d7fb365" | ||
| "git_url" : "https://gitlab.com/inlineptx/legion.git", | ||
| "git_tag" : "0de432af68dc8a8274dd5412945e946cea08f3bc" |
There was a problem hiding this comment.
Just putting a reminder here to reset this before merging with mainline.
…fea/legion-with-cmake-cuda-language
* run on pull_request events if PR is to a fork * use rapids-sccache-devs when building on a fork * test prerelease devcontainer
…fea/legion-with-cmake-cuda-language
trxcllnt
force-pushed
the
fea/legion-with-cmake-cuda-language
branch
from
3b328a4 to
15f1359
Compare
…fea/legion-with-cmake-cuda-language
trxcllnt
force-pushed
the
fea/legion-with-cmake-cuda-language
branch
from
15f1359 to
15cf543
Compare
for more information, see https://pre-commit.ci
trxcllnt
force-pushed
the
fea/legion-with-cmake-cuda-language
branch
from
151c1b8 to
018a200
Compare
…fea/legion-with-cmake-cuda-language
…fea/legion-with-cmake-cuda-language
…fea/legion-with-cmake-cuda-language
…fea/legion-with-cmake-cuda-language
…fea/legion-with-cmake-cuda-language
|
Closing in favor of #828. |
3 participants
This PR updates legate.core to use a Legion branch that incorporates the changes in the drop-find-cuda PR.