Page MenuHomePhabricator

[OpenMP][CMake] Use in-project clang as CUDA->IR compiler for new DeviceRTL.
ClosedPublic

Authored by Meinersbur on Sep 22 2021, 7:07 AM.

Details

Summary

Use the in-project clang, llvm-link and opt if available and unless CMake cache variables specify to use a different compiler. This applies D101265 to the new DeviceRTL's CMakeLists.txt which was copied before D101265 was applied.

Fixes the openmp-offloading-cuda-runtime builder failing since D110006.

Diff Detail

Event Timeline

Meinersbur created this revision.Sep 22 2021, 7:07 AM
Meinersbur requested review of this revision.Sep 22 2021, 7:07 AM
ye-luo added inline comments.Sep 22 2021, 8:02 AM
openmp/libomptarget/DeviceRTL/CMakeLists.txt
57–63

I got empty ${llvm_link} on my machine and use ${compiler_dir}/llvm-link solves the problem.

Meinersbur added inline comments.Sep 22 2021, 8:13 AM
openmp/libomptarget/DeviceRTL/CMakeLists.txt
57–63

Thanks for noticing. I removed ${llvm_link} to avoid confusion with ${bc_linker}. Missed this occurrence.

  • Replaced missed occurance of ${llvm_link}

Generally looks better than previous patch. Thanks for that. This patch assumes those targets must be there in runtime build, which might not be the case. I think it could be better if we first check if those targets are there. If not, raise an error directly, saying runtime build requires clang, etc.

openmp/libomptarget/DeviceRTL/CMakeLists.txt
46

This is the case with LLVM_ENABLE_RUNTIMES=openmp.

This is not always the case. Standalone build also falls to this branch.

69

${compiler_dir}/opt appears three times. Can we set a variable for that?

Meinersbur added a comment.EditedSep 22 2021, 9:59 AM

Generally looks better than previous patch. Thanks for that. This patch assumes those targets must be there in runtime build, which might not be the case. I think it could be better if we first check if those targets are there.

The clang target does not exist at this point since the CMake script for openmp is processed before clang.

If not, raise an error directly, saying runtime build requires clang, etc.

The error already exists: "Not building deviceRTL: clang not found", unless a CUDA->IR compiler is explicitly specified with LIBOMPTARGET_NVPTX_CUDA_COMPILER .

  • Use temporary variable for candidates
Meinersbur marked 3 inline comments as done.Sep 22 2021, 10:08 AM
openmp/libomptarget/DeviceRTL/CMakeLists.txt
41

LLVM_TOOL_CLANG_BUILD seems already obsolete. I searched the whole LLVM project but didn't find where it is defined. The only two places it is still being used is in polly and libomptarget.

Meinersbur added inline comments.Sep 25 2021, 10:36 PM
openmp/libomptarget/DeviceRTL/CMakeLists.txt
41

It is defined here: https://github.com/llvm/llvm-project/blob/e21b0ba8c9378bca01d2311be4e1b6ccd3397bc4/llvm/CMakeLists.txt#L141

It is deprecated to be set by the user, as the comment before is explaining. It is still an integral part of the entire external project mechanism: https://github.com/llvm/llvm-project/blob/e21b0ba8c9378bca01d2311be4e1b6ccd3397bc4/llvm/cmake/modules/AddLLVM.cmake#L1355

There are not many literal LLVM_TOOL_CLANG_BUILD strings because its implementation is mostly generic for all tools, not just clang.

This revision is now accepted and ready to land.Sep 26 2021, 1:30 PM