[Openmp-commits] [PATCH] D108534: [OpenMP][Docs] add clang to LLVM_ENABLE_PROJECTS in build instructions

[Shivam Gupta via Phabricator via Openmp-commits]

xgupta added a comment.

In D108534#2959955 <https://reviews.llvm.org/D108534#2959955>, @JonChesterfield wrote:

> Building openmp offloading via LLVM_ENABLE_PROJECTS makes it difficult to get the deviceRTL and clang to exactly match so it's not as heavily tested as it might be.

Thanks, I understood.

> Corresponding part of the CMakeLists is:
>
>   if (LLVM_DIR)
>     # Builds that use pre-installed LLVM have LLVM_DIR set.
>     find_program(CLANG_TOOL clang PATHS ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
>     find_program(LINK_TOOL llvm-link PATHS ${LLVM_TOOLS_BINARY_DIR}
>       NO_DEFAULT_PATH)
>     find_program(OPT_TOOL opt PATHS ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
>     libomptarget_say("Building AMDGCN device RTL. Using clang: ${CLANG_TOOL}")
>   else()
>     # LLVM in-tree builds may use CMake target names to discover the tools.
>     set(CLANG_TOOL $<TARGET_FILE:clang>)
>     set(LINK_TOOL $<TARGET_FILE:llvm-link>)
>     set(OPT_TOOL $<TARGET_FILE:opt>)
>     libomptarget_say("Building AMDGCN device RTL. Using clang from in-tree build")
>   endif()
>
>
> Nvptx has done something different. Things like
>
>   if (LLVM_TOOL_CLANG_BUILD AND NOT CMAKE_CROSSCOMPILING)
>
>   if (NOT OPENMP_STANDALONE_BUILD AND NOT CMAKE_CROSSCOMPILING)
>
> Looks like that was introduced by D101265 <https://reviews.llvm.org/D101265>.
>
> Mismatch between clang and the deviceRTL is really easy to do by accident with the ENABLE_PROJECTS approach and we don't do anything like burn a git hash into the deviceRTL to check at runtime that they match. I'm inclined to say that we should totally disable openmp offloading when not building via ENABLE_RUNTIMES.

Ok, So we also need to update LLVM CMake.rst doc where openmp is in the list of ENABLE_PROJECTS.

> Anyone know how to spell that in cmake?

I guess anyone is me :) sure I need to think a little, we do.

> edit: regarding
>
>> And with enabling clang in the list of llvm projects to build, build was successful.
>
> There's a fairly good chance that the successful build created a deviceRTL that wouldn't work with the clang used, which is fine if the user doesn't try to use openmp offloading but is otherwise bad

Agree, I didn't (never?) use openmp so can't say this successful build work successfully.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108534/new/

https://reviews.llvm.org/D108534