[OpenMP] Dependency on "opt" during runtimes-configure may be missing #85933

uweigand · 2024-03-20T14:01:22Z

The openmp-s390x-linux builder is sporadically failing with:

# | clang: error: no such file or directory: '/home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./lib/libomptarget.devicertl.a'

when running the test. It turns out that library isn't being built due the following configure decision:

-- LIBOMPTARGET: Not building DeviceRTL. Missing clang: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/bin/clang, llvm-link: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/bin/llvm-link, opt: OPT_TOOL-NOTFOUND, or clang-offload-packager: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/bin/clang-offload-packager

(note the OPT_TOOL-NOTFOUND).

And even in successful builds, I sometimes see the line:

381.426 [3/2/4993] Linking CXX executable bin/opt

after

375.273 [42/4/4951] Performing configure step for 'runtimes'

So it looks like there needs to be a dependency on opt for the runtimes-configure target, but this may be missing. Looking at the llvm/runtimes/CMakeLists.txt file, I see this block:

  if("openmp" IN_LIST LLVM_ENABLE_RUNTIMES)
    foreach(dep opt llvm-link llvm-extract clang clang-offload-packager)
      if(TARGET ${dep} AND OPENMP_ENABLE_LIBOMPTARGET)
        list(APPEND extra_deps ${dep})
      endif()
    endforeach()
  endif()

and this extra_deps variable is used when configuring the default runtimes target:

  if(NOT LLVM_RUNTIME_TARGETS)
    runtime_default_target(
      DEPENDS ${builtins_dep} ${extra_deps}
      CMAKE_ARGS ${libc_cmake_args}
      PREFIXES ${prefixes})
    set(test_targets check-runtimes)

But when only some runtimes components are enabled, extra_deps doesn't appear to be used:

    foreach(name ${LLVM_RUNTIME_TARGETS})
      if(builtins_dep)
        if (LLVM_BUILTIN_TARGETS)
          set(builtins_dep_name "${builtins_dep}-${name}")
        else()
          set(builtins_dep_name ${builtins_dep})
        endif()
      endif()

      check_apple_target(${name} runtime)

      runtime_register_target(${name}
        DEPENDS ${builtins_dep_name} ${hdrgen_deps}
        CMAKE_ARGS -DLLVM_DEFAULT_TARGET_TRIPLE=${name} ${libc_cmake_args}
        EXTRA_ARGS TARGET_TRIPLE ${name})
    endforeach()

(Note that hdrgen_deps is also present in extra_deps, but the latter may include many more dependencies.)

Should extra_deps be used here as well?

CC @jhuber6 @petrhosek

The text was updated successfully, but these errors were encountered:

llvmbot · 2024-03-20T14:41:49Z

@llvm/issue-subscribers-openmp

Author: Ulrich Weigand (uweigand)

The openmp-s390x-linux builder is sporadically failing with: ``` # | clang: error: no such file or directory: '/home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./lib/libomptarget.devicertl.a' ``` when running the test. It turns out that library isn't being built due the following configure decision: ``` -- LIBOMPTARGET: Not building DeviceRTL. Missing clang: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/bin/clang, llvm-link: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/bin/llvm-link, opt: OPT_TOOL-NOTFOUND, or clang-offload-packager: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/bin/clang-offload-packager ``` (note the `OPT_TOOL-NOTFOUND`).

And even in successful builds, I sometimes see the line:

381.426 [3/2/4993] Linking CXX executable bin/opt

after

375.273 [42/4/4951] Performing configure step for 'runtimes'

So it looks like there needs to be a dependency on opt for the runtimes-configure target, but this may be missing. Looking at the llvm/runtimes/CMakeLists.txt file, I see this block:

  if("openmp" IN_LIST LLVM_ENABLE_RUNTIMES)
    foreach(dep opt llvm-link llvm-extract clang clang-offload-packager)
      if(TARGET ${dep} AND OPENMP_ENABLE_LIBOMPTARGET)
        list(APPEND extra_deps ${dep})
      endif()
    endforeach()
  endif()

and this extra_deps variable is used when configuring the default runtimes target:

  if(NOT LLVM_RUNTIME_TARGETS)
    runtime_default_target(
      DEPENDS ${builtins_dep} ${extra_deps}
      CMAKE_ARGS ${libc_cmake_args}
      PREFIXES ${prefixes})
    set(test_targets check-runtimes)

But when only some runtimes components are enabled, extra_deps doesn't appear to be used:

    foreach(name ${LLVM_RUNTIME_TARGETS})
      if(builtins_dep)
        if (LLVM_BUILTIN_TARGETS)
          set(builtins_dep_name "${builtins_dep}-${name}")
        else()
          set(builtins_dep_name ${builtins_dep})
        endif()
      endif()

      check_apple_target(${name} runtime)

      runtime_register_target(${name}
        DEPENDS ${builtins_dep_name} ${hdrgen_deps}
        CMAKE_ARGS -DLLVM_DEFAULT_TARGET_TRIPLE=${name} ${libc_cmake_args}
        EXTRA_ARGS TARGET_TRIPLE ${name})
    endforeach()

(Note that hdrgen_deps is also present in extra_deps, but the latter may include many more dependencies.)

Should extra_deps be used here as well?

CC @jhuber6 @petrhosek

jhuber6 · 2024-03-20T14:43:54Z

I really hate the handling here, unfortunately it's a little difficult to set up the library in the way we need it without fishing a few of these tools out of the build directory. I think in the future I should be able to rewrite a lot of this, but for now does it work if you force the opt dependency on everything?

uweigand · 2024-03-20T14:45:44Z

This patch seems to work for me:

diff --git a/llvm/runtimes/CMakeLists.txt b/llvm/runtimes/CMakeLists.txt
index 623c43d564cc..b0d428881fde 100644
--- a/llvm/runtimes/CMakeLists.txt
+++ b/llvm/runtimes/CMakeLists.txt
@@ -531,7 +531,7 @@ if(runtimes)
       check_apple_target(${name} runtime)
 
       runtime_register_target(${name}
-        DEPENDS ${builtins_dep_name} ${hdrgen_deps}
+        DEPENDS ${builtins_dep_name} ${extra_deps}
         CMAKE_ARGS -DLLVM_DEFAULT_TARGET_TRIPLE=${name} ${libc_cmake_args}
         EXTRA_ARGS TARGET_TRIPLE ${name})
     endforeach()

uweigand · 2024-03-20T17:01:37Z

Actually, I take it back; that seems to have been just happenstance. Looks like I confused LLVM_RUNTIME_TARGETS with LLVM_ENABLE_RUNTIMES. Need to look into this some more.

uweigand · 2024-03-20T17:22:46Z

Now I see. While executing this block of code:

  if("openmp" IN_LIST LLVM_ENABLE_RUNTIMES)
    foreach(dep opt llvm-link llvm-extract clang clang-offload-packager)
      if(TARGET ${dep} AND OPENMP_ENABLE_LIBOMPTARGET)
        list(APPEND extra_deps ${dep})
      endif()
    endforeach()
  endif()

nothing ever happens because OPENMP_ENABLE_LIBOMPTARGET is not set. Looks like this would be set in openmp/CMakeLists.txt, but that file isn't parsed at initial cmake time. Maybe this broke when moving openmp from LLVM_ENABLE_PROJECTS to LLVM_ENABLE_RUNTIMES?

Do we even need that check here? We might as well add those dependencies always ... This patch does fix the problem for me:

diff --git a/llvm/runtimes/CMakeLists.txt b/llvm/runtimes/CMakeLists.txt
index 623c43d564cc..b4664b4baa23 100644
--- a/llvm/runtimes/CMakeLists.txt
+++ b/llvm/runtimes/CMakeLists.txt
@@ -435,7 +435,7 @@ if(runtimes)
       list(APPEND extra_deps "flang-new")
     endif()
     foreach(dep opt llvm-link llvm-extract clang clang-offload-packager)
-      if(TARGET ${dep} AND OPENMP_ENABLE_LIBOMPTARGET)
+      if(TARGET ${dep})
         list(APPEND extra_deps ${dep})
       endif()
     endforeach()

(The previous patch is probably also needed in case people use a non-default LLVM_RUNTIME_TARGETS ...)

jhuber6 · 2024-03-20T17:24:26Z

That seems correct, that probably just wasn't thought through correctly when written and moving to runtimes. If you make a PR for that I'll approve it.

When building the OpenMP runtime with libomptarget support, the runtimes configure step needs to have a dependency on various tools, in particular opt, so that cmake configure checks yield the correct results. This did not work correctly, as the dependencies were only added if the OPENMP_ENABLE_LIBOMPTARGET was set - but that variable is only set by the openmp/CMakeLists.txt file, which isn't even parsed during the initial cmake run (in fact, it is only parsed when executing the runtimes configure step itself, but then it is too late). Fixed by just adding those dependencies always. In addition, the list of dependencies collected in ${extra_deps}, including those required for OpenMP, was only actually used when configuring runtimes for the default set of targets - when the user specifies a non-default LLVM_RUNTIME_TARGETS, those extra dependencies were ignored (with the exception of ${hdrgen_deps}). Fixed by passing the full ${extra_deps} in this case as well. Fixes: llvm#85933

When building the OpenMP runtime with libomptarget support, the runtimes configure step needs to have a dependency on various tools, in particular opt, so that cmake configure checks yield the correct results. This did not work correctly, as the dependencies were only added if the OPENMP_ENABLE_LIBOMPTARGET was set - but that variable is only set by the openmp/CMakeLists.txt file, which isn't even parsed during the initial cmake run (in fact, it is only parsed when executing the runtimes configure step itself, but then it is too late). Fixed by just adding those dependencies always. In addition, the list of dependencies collected in ${extra_deps}, including those required for OpenMP, was only actually used when configuring runtimes for the default set of targets - when the user specifies a non-default LLVM_RUNTIME_TARGETS, those extra dependencies were ignored (with the exception of ${hdrgen_deps}). Fixed by passing the full ${extra_deps} in this case as well. Fixes: #85933

When building the OpenMP runtime with libomptarget support, the runtimes configure step needs to have a dependency on various tools, in particular opt, so that cmake configure checks yield the correct results. This did not work correctly, as the dependencies were only added if the OPENMP_ENABLE_LIBOMPTARGET was set - but that variable is only set by the openmp/CMakeLists.txt file, which isn't even parsed during the initial cmake run (in fact, it is only parsed when executing the runtimes configure step itself, but then it is too late). Fixed by just adding those dependencies always. In addition, the list of dependencies collected in ${extra_deps}, including those required for OpenMP, was only actually used when configuring runtimes for the default set of targets - when the user specifies a non-default LLVM_RUNTIME_TARGETS, those extra dependencies were ignored (with the exception of ${hdrgen_deps}). Fixed by passing the full ${extra_deps} in this case as well. Fixes: llvm#85933

github-actions bot added the new issue label Mar 20, 2024

EugeneZelenko added cmake Build system in general and CMake in particular openmp and removed new issue labels Mar 20, 2024

uweigand mentioned this issue Mar 20, 2024

[runtimes] Fix OpenMP dependencies #85977

Merged

uweigand closed this as completed in #85977 Mar 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[OpenMP] Dependency on "opt" during runtimes-configure may be missing #85933

[OpenMP] Dependency on "opt" during runtimes-configure may be missing #85933

uweigand commented Mar 20, 2024

llvmbot commented Mar 20, 2024

jhuber6 commented Mar 20, 2024

uweigand commented Mar 20, 2024

uweigand commented Mar 20, 2024

uweigand commented Mar 20, 2024

jhuber6 commented Mar 20, 2024

[OpenMP] Dependency on "opt" during runtimes-configure may be missing #85933

[OpenMP] Dependency on "opt" during runtimes-configure may be missing #85933

Comments

uweigand commented Mar 20, 2024

llvmbot commented Mar 20, 2024

jhuber6 commented Mar 20, 2024

uweigand commented Mar 20, 2024

uweigand commented Mar 20, 2024

uweigand commented Mar 20, 2024

jhuber6 commented Mar 20, 2024