[Openmp-commits] [PATCH] D107156: [libomptarget][amdcgn] Add build dependency for opt

[Khem Raj via Phabricator via Openmp-commits]

raj.khem added a comment.

In D107156#2970143 <https://reviews.llvm.org/D107156#2970143>, @JonChesterfield wrote:

> The current trunk cmake guards whether to build this with:
>
>   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}")
>   elseif (LLVM_TOOL_CLANG_BUILD AND NOT CMAKE_CROSSCOMPILING AND NOT OPENMP_STANDALONE_BUILD)
>     # 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")
>   else()
>     libomptarget_say("Not building AMDGCN device RTL. No appropriate clang found")
>     return()
>   endif()
>
>
>
>> standalone cross-build
>
> Which path are you hitting? Can grep for 'Building AMDGCN device RTL' to see which.

I am on 13.x branch, This patch got backported to 13.x recently. I am not sure if trunk piece you are mentioning is also there in 13.x as well.

> Guessing somewhat, we're specifying opt as a dependency whenever building this library, but perhaps the builds using pre-installed LLVM using LLVM_DIR must not specify it as a dependency, since it isn't supposed to be built as part of the current cmake invocation.
>
> I'd still rather we only attempt to compile this as part of ENABLE_RUNTIMES. Pre-installed LLVM isn't especially likely to build an openmp offloading toolchain that works. If you can confirm that it's the if (LLVM_DIR) predicate that is passing, perhaps the direct fix is to replace find_program() with return()

LLVM_DIR is set for me

CMakeCache.txt:LLVM_DIR:PATH=/mnt/b/yoe/master/build/tmp/work/cortexa72-cortexa53-crypto-yoe-linux-musl/openmp/13.0.0-r0/recipe-sysroot/usr/lib/cmake/llvm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107156