Page MenuHomePhabricator

[OpenMP][Docs] add clang to LLVM_ENABLE_PROJECTS in build instructions
AbandonedPublic

Authored by xgupta on Aug 23 2021, 12:59 AM.

Details

Summary

Without enabling clang following error messagesa are generated -

CMake Error at /home/user/project/llvm-project/openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt:134 (add_custom_command):

Error evaluating generator expression:

  $<TARGET_FILE:clang>

No target "clang"

Call Stack (most recent call first):

/home/user/project/llvm-project/openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt:154 (add_cuda_bc_library)

And with enabling clang in the list of llvm projects to build, build was successful.

Diff Detail

Event Timeline

xgupta created this revision.Aug 23 2021, 12:59 AM
xgupta requested review of this revision.Aug 23 2021, 12:59 AM
Herald added a project: Restricted Project. · View Herald Transcript
JonChesterfield added a subscriber: JonChesterfield.

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. 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.

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. Anyone know how to spell that in cmake?

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

xgupta added a comment.EditedAug 23 2021, 9:11 AM

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.

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 & llvm main CMakeLists.txt 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.

At least it should not affect standalone build.

xgupta added a comment.EditedAug 23 2021, 10:52 AM

So I looked at how buildbot built openmp here - https://github.com/llvm/llvm-zorg/blob/main/buildbot/osuosl/master/config/builders.py#L1377.
And did `cmake ../llvm -DLLVM_ENABLE_RUNTIMES="openmp" -DLLVM_ENABLE_PROJECTS="clang" -DLLVM_ENABLE_LLD=ON -DCMAKE_BUILD_TYPE=Release and make openmp to build it and faced a newly reported issue - https://bugs.llvm.org/show_bug.cgi?id=51579

In configure stage pthread.h header not found and I get the following error but I have pthread in usr/include/pthread.h

CMake Error at /usr/share/cmake-3.21/Modules/FindPackageHandleStandardArgs.cmake:230 (message):

Could NOT find Threads (missing: Threads_FOUND)

Call Stack (most recent call first):

/usr/share/cmake-3.21/Modules/FindPackageHandleStandardArgs.cmake:594 (_FPHSA_FAILURE_MESSAGE)
/usr/share/cmake-3.21/Modules/FindThreads.cmake:238 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
/home/xgupta/project/llvm-project/openmp/runtime/cmake/config-ix.cmake:155 (find_package)
/home/xgupta/project/llvm-project/openmp/runtime/CMakeLists.txt:240 (include)

So can someone please share the recipe for building in-tree openmp or it is me who has something missing?

Edit, Note-
Building is configuring two times while running make openmp-

  1. first during llvm build where -- Looking for pthread.h - found
  1. and second for runtime -- Looking for pthread.h - not found
jdoerfert requested changes to this revision.Aug 23 2021, 11:18 AM

This webpage is about to go away. If anything we need to update the docs on openmp.llvm.org/docs. Basically, feel free to improve the FAQ in https://openmp.llvm.org/docs/SupportAndFAQ.html#q-how-to-build-an-openmp-gpu-offload-capable-compiler .

This revision now requires changes to proceed.Aug 23 2021, 11:18 AM

cmake failing to find pthread seems bad. That it finds it for projects and not for runtimes suggests that the clang built under runtimes can't find pthread.

@jdoerfert do you think detecting the non-runtimes build and skipping the offloading libraries would be reasonable? Perhaps with a cmake override for the determined

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. Anyone know how to spell that in cmake?

I think it is the other way around ENABLE_PROJECTS=clang;openmp ensures that the deviceRTL is built with with same clang as the clang the user will end up with. Problems occur when OpenMP's CMakeLists.txt tries to use the host compiler for the deviceRTL just because its CMAKE_C_COMPILER_ID is clang. Unfortunately this is also the mechanism how the LLVM_ENABLE_RUNTIMES=openmp builds finds its deviceRTL compiler. My patch to avoid this behaviour was controversial: D101663.

I agree that compiling deviceRTL (and by extension libomptarget) should be disabled if no suitable compiler is found. A suitable compiler might either be specified using LIBOMPTARGET_NVPTX_CUDA_COMPILER or $<TARGET_FILE:clang> if available. I don't see a good reason to disable building libomptarget entirely with LLVM_ENABLE_PROJECTS=openmp builds if clang is found in the same build.

The challenge is mostly that clang and deviceRTL (and host runtime to some extent) need to agree on the internal api. E.g. whether a function returns i32 or i64, as something I changed a few days ago for nvptx. That internal API is not stable, documented or versioned. That lets code move across the boundary and generally evolve.

However, that only runs reliably if clang uses the deviceRTL from the same monorepo hash. Some other clang will have different assumptions about the library. It's not sufficient to build the deviceRTL with the clang the user will run on host code. It really has to be the deviceRTL that came from the same checkout as the clang the user will use.

This is difficult to specify reliably without requiring an exact match (e.g. burn the git sha into all the pieces and refuse to run if some differ) or bootstrapping. Enable_runtimes makes the bootstrap easy to get right.

This is difficult to specify reliably without requiring an exact match (e.g. burn the git sha into all the pieces and refuse to run if some differ) or bootstrapping. Enable_runtimes makes the bootstrap easy to get right.

+1

This webpage is about to go away. If anything we need to update the docs on openmp.llvm.org/docs. Basically, feel free to improve the FAQ in https://openmp.llvm.org/docs/SupportAndFAQ.html#q-how-to-build-an-openmp-gpu-offload-capable-compiler .

This file is deleted as part of D108588. Apologies for the inconvenience.

I originally reported: https://bugs.llvm.org/show_bug.cgi?id=51579
Seeing the discussion here, it looks like the official instructions to build LLVM openmp in-tree (at openmp.llvm.)org won't work for anyone? And it wasn't something in my cmake conf or setup.

Seeing the discussion here, it looks like the official instructions to build LLVM openmp in-tree (at openmp.llvm.)org won't work for anyone? And it wasn't something in my cmake conf or setup.

I'd guess cmake failing to find pthread is local to your system.

Target offloading can't build with GCC. It will build with any recent clang but the result won't necessarily work for offloading unless the library is from the same checkout as clang.

The offload libs should default to build under enable_runtimes (as they'll work) and to not build under enable_projects. That needs a fix to the cmake to detect which is happening.

bondhugula added a comment.EditedAug 24 2021, 4:04 AM

Seeing the discussion here, it looks like the official instructions to build LLVM openmp in-tree (at openmp.llvm.)org won't work for anyone? And it wasn't something in my cmake conf or setup.

I'd guess cmake failing to find pthread is local to your system.

This would be strange because I do have it at /usr/include/pthread.h. Looking at the cmake error log pasted at https://bugs.llvm.org/show_bug.cgi?id=51579 the missing pthread.h error appears to be a symptom of something else but I'm not sure. I think the compile command being used there isn't right? (cmake error log snippet pasted below)

Determining if the include file pthread.h exists failed with the following output:
Change Dir: /home/uday/llvm-project-upstream/build/runtimes/runtimes-bins/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/ninja cmTC_f7224 && [1/2] Building C object CMakeFiles/cmTC_f7224.dir/CheckIncludeFile.c.o
FAILED: CMakeFiles/cmTC_f7224.dir/CheckIncludeFile.c.o 
/usr/bin/cc   -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wno-comment -fdiagnostics-color  -nostdinc++ -nostdlib++ -o CMakeFiles/cmTC_f7224.dir/CheckIncludeFile.c.o   -c CheckIncludeFile.c
cc: error: unrecognized command line option ‘-nostdlib++’; did you mean ‘-nostdlib’?
ninja: build stopped: subcommand failed.

It's using the wrong compiler?

Seeing the discussion here, it looks like the official instructions to build LLVM openmp in-tree (at openmp.llvm.)org won't work for anyone? And it wasn't something in my cmake conf or setup.

I'd guess cmake failing to find pthread is local to your system.

This would be strange because I do have it at /usr/include/pthread.h. Looking at the cmake error log pasted at https://bugs.llvm.org/show_bug.cgi?id=51579 the missing pthread.h error appears to be a symptom of something else but I'm not sure. I think the compile command being used there isn't right? (cmake error log snippet pasted below)

Determining if the include file pthread.h exists failed with the following output:
Change Dir: /home/uday/llvm-project-upstream/build/runtimes/runtimes-bins/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/ninja cmTC_f7224 && [1/2] Building C object CMakeFiles/cmTC_f7224.dir/CheckIncludeFile.c.o
FAILED: CMakeFiles/cmTC_f7224.dir/CheckIncludeFile.c.o 
/usr/bin/cc   -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wno-comment -fdiagnostics-color  -nostdinc++ -nostdlib++ -o CMakeFiles/cmTC_f7224.dir/CheckIncludeFile.c.o   -c CheckIncludeFile.c
cc: error: unrecognized command line option ‘-nostdlib++’; did you mean ‘-nostdlib’?
ninja: build stopped: subcommand failed.

It's using the wrong compiler?

I'd guess /usr/bin/cc is very old.

Meinersbur added a comment.EditedAug 24 2021, 9:43 AM

However, that only runs reliably if clang uses the deviceRTL from the same monorepo hash. Some other clang will have different assumptions about the library. It's not sufficient to build the deviceRTL with the clang the user will run on host code. It really has to be the deviceRTL that came from the same checkout as the clang the user will use.

This is difficult to specify reliably without requiring an exact match (e.g. burn the git sha into all the pieces and refuse to run if some differ) or bootstrapping. Enable_runtimes makes the bootstrap easy to get right.

At the risk of repeating myself:

  • $<TARGET_FILE:clang> (if clang is compiled alongside) is the correct clang with the same monorepo hash.
  • The current implementation of libomptarget's LLVM_ENABLE_RUNTIMES makes any compiler with CMAKE_C_COMPILER_ID equals "clang" compile the deviceRTL.
xgupta abandoned this revision.Sep 15 2021, 4:06 AM