[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jhuber6 closed this revision. jhuber6 added a comment. Screwed up and put the wrong revision in the commit message. Closed in rGd564409946a5a13cb6391fc0fec54dcbd6f6d249 Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

JonChesterfield added a comment. Nice patch. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88929/new/ https://reviews.llvm.org/D88929 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo accepted this revision. ye-luo added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88929/new/ https://reviews.llvm.org/D88929 ___

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jdoerfert added a comment. @ye-luo good? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88929/new/ https://reviews.llvm.org/D88929 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jhuber6 updated this revision to Diff 296792. jhuber6 added a comment. Implementing Ye's code. I changed it to putput the architecture as a dependency and then set the value where we define the default architecture. I did a full build from scratch using this and got the sm_70 libraries for my

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo added inline comments. Comment at: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt:80 + else() +list(APPEND compute_capabilities ${CMAKE_MATCH_1}) + endif() jhuber6 wrote: > ye-luo wrote: > > 1. Doesn't work right now. Missing comma

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo added inline comments. Comment at: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt:92 foreach(sm ${nvptx_sm_list}) set(CUDA_ARCH ${CUDA_ARCH} -gencode arch=compute_${sm},code=sm_${sm}) endforeach() my point 2 refers to here CUDA_ARCH which

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jhuber6 added inline comments. Comment at: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt:80 + else() +list(APPEND compute_capabilities ${CMAKE_MATCH_1}) + endif() ye-luo wrote: > 1. Doesn't work right now. Missing comma

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo requested changes to this revision. ye-luo added inline comments. This revision now requires changes to proceed. Comment at: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt:80 + else() +list(APPEND compute_capabilities ${CMAKE_MATCH_1}) + endif()

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jhuber6 updated this revision to Diff 296577. jhuber6 added a comment. Removing redundant call to `find_package`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88929/new/ https://reviews.llvm.org/D88929 Files: clang/CMakeLists.txt

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo added a comment. In D88929#2315640 , @JonChesterfield wrote: > An alternative approach is to build the deviceRTL for multiple cuda versions > and then pick whichever one is the best fit when compiling application code. > That has advantages when

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

JonChesterfield added a comment. An alternative approach is to build the deviceRTL for multiple cuda versions and then pick whichever one is the best fit when compiling application code. That has advantages when building the deviceRTL libraries on a different machine to the one that intends to

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo added a comment. In D88929#2315538 , @jhuber6 wrote: > In D88929#2315519 , @ye-luo wrote: > >> Probably not messing with `enable_language(CUDA)` at the moment, just add >>

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jhuber6 added a comment. In D88929#2315519 , @ye-luo wrote: > Probably not messing with `enable_language(CUDA)` at the moment, just add > `cuda_select_nvcc_arch_flags(CUDA_ARCH_FLAGS)` to >

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo added a comment. In D88929#2315513 , @jhuber6 wrote: > In D88929#2315451 , @ye-luo wrote: > >> I just realized that this patch affects clang and libomptarget. >> I cannot comment on clang. Regarding

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jhuber6 added a comment. In D88929#2315451 , @ye-luo wrote: > I just realized that this patch affects clang and libomptarget. > I cannot comment on clang. Regarding libomptarget, Could you explain why the > detection is not put together with other cuda

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo added a comment. I just realized that this patch affects clang and libomptarget. I cannot comment on clang. Regarding libomptarget, Could you explain why the detection is not put together with other cuda stuff in `openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake`

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jhuber6 added a comment. In D88929#2315402 , @ye-luo wrote: > The link I posted indicated that independent feature is merged since 3.12. > Better to avoid deprecated stuff when introducing new cmake lines even though > some existing lines may still rely

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo added a comment. 3.18 introduces CMAKE_CUDA_ARCHITECTURES. Does 3.18 supports detection? If we know a new way works since 3.18, I think putting both with if-else makes sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88929/new/

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo added a comment. The link I posted indicated that independent feature is merged since 3.12. Better to avoid deprecated stuff when introducing new cmake lines even though some existing lines may rely on deprecated cmake. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jhuber6 added a comment. In D88929#2315378 , @ye-luo wrote: > FindCUDA has been deprecated. > Please explore the following feature without directly calling FindCUDA. > https://gitlab.kitware.com/cmake/cmake/-/merge_requests/1856 Finding architectures

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

ye-luo added a comment. FindCUDA has been deprecated. Please explore the following feature without directly calling FindCUDA. https://gitlab.kitware.com/cmake/cmake/-/merge_requests/1856 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88929/new/

[PATCH] D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default

jhuber6 created this revision. jhuber6 added a reviewer: jdoerfert. jhuber6 added projects: clang, OpenMP. Herald added subscribers: openmp-commits, cfe-commits, guansong, yaxunl, mgorny. jhuber6 requested review of this revision. Herald added a subscriber: sstefan1. This patch changes the CMake