[libomptarget][DeviceRTL] Generalise and simplify cmakelists

[JonChesterfield]

Summary

Step towards building the DeviceRTL for amdgpu.

Mostly replaces cuda-specific toolchain finding logic with the
generic logic currently found in the amdgpu deviceRTL cmake. Also
deletes dead code and changes the default to build on systems
without cuda installed, as the library doesn't use cuda and the
amdgpu-only systems generally won't have cuda installed.

[JonChesterfield]

• add hook to disable bitcode build as requested on the previous amdgpu lib

[JonChesterfield]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

12

Built it by default - only depends on clang (doesn't even use freestanding libc headers at present). This was a feature request on amdgpu/deviceRTL and it's cheap to keep that alive here. Someone building libomptarget with a clang which is too old to compile this library may want to opt out.

21–22

This is copy&paste from the amdgpu cmakelists with the debug print slightly changed.

93–94

MAX_SM is cruft from the old deviceRTL, not used in the new one

97–98

these messages are redundant with the ones emitted above

178

I think it reads better to sort out the dependency for both tools before using them

186

This looks dubious - the tailing _opt corresponds to the following custom command, but we're calling opt as opt file.bc -o file.bc. Probably want to link to file.link.bc or similar then opt to file.bc so we've got both on disk after the build. I vaguely recall problems reading & writing to the same file on windows. Leaving for a later change.

[JonChesterfield]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

142

^ this looks wrong, can anyone confirm?

I think it says that the target of all bitcode libraries depends on opt and llvm-link, however each of the libraries already depends on opt and llvm-link, using code that carefully only adds that dependency if they're part of the build.

If I'm reading this right, this line:

add_dependencies(omptarget-new-nvptx-bc opt llvm-link)

is redundant in the best case and will break the build in the worst case

[JonChesterfield]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

212

like the above comment, I don't think we want to add opt and llvm-link as dependencies here

[JonChesterfield]

Aside from changing to building by default and some changes to debug messages, this is intended to be a non-functional change.

[Meinersbur]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

34–37

This removes support for LIBOMPTARGET_NVPTX_CUDA_COMPILER/LIBOMPTARGET_NVPTX_BC_LINKER settings? Instead, it uses clang found in LLVM_TOOLS_BINARY_DIR, which very much prefer over ${CMAKE_C_COMPILER}. This this work with an LLVM_DIR to a pre-installed LLVM in /usr or /usr/local?

[JonChesterfield]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

34–37

Changes the flag yeah. This is openmp code - no particular reason why NVPTX_CUDA_COMPILER is a good name for which one to use.

The cmake will have a go with whatever LLVM_DIR is set, whether that will succeed or not depends on whether that compiler is recent enough to deal with the openmp features used. Variant is probably the limiting factor.

We're trying to encourage people to build this via ENABLE_RUNTIMES as than ensures that the clang used can actually build it, but there are still these hooks if people want to try with a different one.

[Meinersbur]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

34–37

Have you seen https://lists.llvm.org/pipermail/llvm-dev/2021-October/153238.html ?

I tried the "Default" build with openmp and it did not work:

CMake Error at /home/meinersbur/src/llvm-project/openmp/runtime/src/CMakeLists.txt:314 (string):
  string sub-command REGEX, mode MATCH needs at least 5 arguments total to
  command.

[JonChesterfield]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

34–37

I read that earlier today but haven't tried to work out the implications for openmp.

string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION ${PACKAGE_VERSION})

I don't know offhand what that's trying to do. Seems unrelated to this patch?

[Meinersbur]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

34–37

Just asking whether we have a plan to make this work if we want to support "Default builds" as well. I think it does, since it uses the same <root>/runtimes/CMakeLists.txt that sets LLVM_DIR using:

find_package(LLVM PATHS "${LLVM_BINARY_DIR}" NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
find_package(Clang PATHS "${LLVM_BINARY_DIR}" NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)

This will import LLVM/Clang's *Targets.cmake. Instead of find_program, one can get the executable path directly using IMPORTED_LOCATION.

[Meinersbur]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

34–37

The documentation also mentions that imported add_executable can be used in add_custom_command. Maybe

add_custom_command(
  clang <args> ...

would just work as well? (this would also work for in-tree clang instead of $<TARGET_FILE:clang>).

186

The OUTPUT must match the OUTPUT of the other add_custom_command on line 192 to add a DEPENDS dependency to the generation of that output dependency. Thus add_custom_command itself does not execute anything.

With the APPEND option I think this must come after the main add_custom_command line on line 192. Alternatively, you could add the DEPENDS opt to that line as well, but it would result in an error if the opt target does not exist. It actually seem to (either the in-tree target or the imported target), since otherwise add_dependencies(${bclib_target_name} opt llvm-link) would give an error es well.

[JonChesterfield]

I'm fairly confident that this change is correct as written because:

• The complicated part if (LLVM_DIR)... is copied directly from amdgpu where it was debugged into existence by other people and has built by default for a long time
• MAX_SM is unused
• The rest is carefully renaming variable and slightly reordering clauses into a sequence that seems more likely to work than the current one

The dependency description in the current file looks inconsistent, there's a top level target that seems to be unused, and I think it's optimising a bitcode library then installing the non-optimised one. That I'm not confident to change, especially at the same time as this refactor, as the odds are high that I'm missing some aspect of how cmake or the rest of the build system behave.

I mistrust cmake's documentation as it correlates poorly with observed behaviour. I also anticipate openmp breaking in a fashion I am ill equipped to repair when the cmake changes to libc++ et al land. Simplifying the current script before semantics change seems a good move.

[tianshilei1992]

IMO the dependence should be, if we are using out of tree build, check the three binaries ahead of time and return error directly if anything is wrong. We don't need to set dependence from custom target to the three binaries in this mode. For in tree build, we use the target file to for the custom command, and use the target name for dependences.

[Meinersbur]

In D111983#3071248, @JonChesterfield wrote:

Simplifying the current script before semantics change seems a good move.

This was what I am suggestion. If the current patch is working, I am ok with keeping it as-is.

However, regarding the correctness of the this patch it is required to have any

add_custom_command(<target> ... APPEND)

occur after

add_custom_command(<target> ...  COMMAND)

as by CMake's documentation:

APPEND
Append the COMMAND and DEPENDS option values to the custom command for the first output specified. There must have already been a previous call to this command with the same output.

[JonChesterfield]

• restore original order of custom command / add depends

[JonChesterfield]

In D111983#3071816, @Meinersbur wrote:

as by CMake's documentation:

APPEND
Append the COMMAND and DEPENDS option values to the custom command for the first output specified. There must have already been a previous call to this command with the same output.

Thanks! I misread that part of your earlier comment. Restored the corresponding parts to the order they were in previously. Curious that append before define was not reported as an error by cmake.

openmp/libomptarget/DeviceRTL/CMakeLists.txt

178

^turns out dependencies come after use, not before, in this context

[JonChesterfield]

Ping. I'm probably going to land this series on rocm in the near future so some amd-internal people can do some testing on it. I'll have an easier time in the merges if some of it can go on trunk first

[JonChesterfield]

• rebase

[Meinersbur]

LGTM

[Meinersbur]

Missed a comment at the previous Submit button press.

openmp/libomptarget/DeviceRTL/CMakeLists.txt

142

Redundant dependencies have no consequences, but forgetting them may result in non-deterministic failures.

I don't see how it could break the build as long as all targets exist. opt and llvm-link targets exist in both build configs, LLVM_ENABLE_PROJECTS and LLVM_ENABLE_RUNTIMES.

[JonChesterfield]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

195

Is this stuff not here to work around opt / llvm-link sometimes not being targets?

[Meinersbur]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

195

Mostly to only add a dependency only when configured to use in-tree llvm-link/opt. llvm-link os a target in both, the LLVM_ENABLE_PROJECTS and LLVM_ENABLE_RUNTIMES (an "import target" from find_package(LLVM)) build.

[tianshilei1992]

openmp/libomptarget/DeviceRTL/CMakeLists.txt

195

That's why I said this here.

openmp/libomptarget/DeviceRTL/CMakeLists.txt

 ##===----------------------------------------------------------------------===##
 #
 # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 ##===----------------------------------------------------------------------===##
 #
-# Build the Device RTL for all toolchains that are available
+# Build the DeviceRTL for all toolchains that are available
 #
 ##===----------------------------------------------------------------------===##
 
-# TODO: copied from NVPTX, need to be generalized.
+set(LIBOMPTARGET_BUILD_DEVICERTL_BCLIB TRUE CACHE BOOL
+  "Can be set to false to disable building this library.")
 
-# By default we will not build NVPTX deviceRTL on a CUDA free system
-set(LIBOMPTARGET_BUILD_NVPTX_BCLIB FALSE CACHE BOOL
-  "Whether build NVPTX deviceRTL on CUDA free system.")
-
-if (NOT (LIBOMPTARGET_DEP_CUDA_FOUND OR LIBOMPTARGET_BUILD_NVPTX_BCLIB))
-  libomptarget_say("Not building NVPTX deviceRTL by default on CUDA free system.")
+if (NOT LIBOMPTARGET_BUILD_DEVICERTL_BCLIB)
+  libomptarget_say("Not building DeviceRTL: Disabled by LIBOMPTARGET_BUILD_DEVICERTL_BCLIB")
   return()
 endif()
 
 if (NOT LIBOMPTARGET_LLVM_INCLUDE_DIRS)
-  libomptarget_say("Not building device RTL: Missing definition for LIBOMPTARGET_LLVM_INCLUDE_DIRS")
-  return()
-endif()
-
-
-# Check if we can create an LLVM bitcode implementation of the runtime library
-# that could be inlined in the user application. For that we need to find
-# a Clang compiler capable of compiling our CUDA files to LLVM bitcode and
-# an LLVM linker.
-set(LIBOMPTARGET_NVPTX_CUDA_COMPILER "" CACHE STRING
-  "Location of a CUDA compiler capable of emitting LLVM bitcode.")
-set(LIBOMPTARGET_NVPTX_BC_LINKER "" CACHE STRING
-  "Location of a linker capable of linking LLVM bitcode objects.")
-
-if (NOT LIBOMPTARGET_NVPTX_CUDA_COMPILER STREQUAL "")
-  set(cuda_compiler ${LIBOMPTARGET_NVPTX_CUDA_COMPILER})
-elseif (LLVM_TOOL_CLANG_BUILD AND NOT CMAKE_CROSSCOMPILING)
-  # Compile the deviceRTL with the clang that is built in the project.
-  set(cuda_compiler "$<TARGET_FILE:clang>")
-elseif(${CMAKE_C_COMPILER_ID} STREQUAL "Clang")
-  set(cuda_compiler ${CMAKE_C_COMPILER})
-else()
-  libomptarget_say("Not building deviceRTL: clang not found")
+  libomptarget_say("Not building DeviceRTL: Missing definition for LIBOMPTARGET_LLVM_INCLUDE_DIRS")
   return()
 endif()
 
-# Get compiler directory to try to locate a suitable linker.
-get_filename_component(compiler_dir ${cuda_compiler} DIRECTORY)
-
-set(bc_linker_candidate "${compiler_dir}/llvm-link")
-if (NOT LIBOMPTARGET_NVPTX_BC_LINKER STREQUAL "")
-  set(bc_linker ${LIBOMPTARGET_NVPTX_BC_LINKER})
-elseif (EXISTS "${bc_linker_candidate}" AND NOT IS_DIRECTORY "${bc_linker_candidate}")
-  # Try to use the linker consistent with the CUDA compiler unless explicitly
-  # set to a different linker.
-  set(bc_linker "${bc_linker_candidate}")
-elseif (NOT OPENMP_STANDALONE_BUILD AND NOT CMAKE_CROSSCOMPILING)
-  # Use the linker also built in the same project.
-  set(bc_linker "$<TARGET_FILE:llvm-link>")
+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 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 DeviceRTL. Using clang from in-tree build")
 else()
-  libomptarget_say("Not building deviceRTL: llvm-link not found")
-  return()
-endif()
-
-set(opt_candidate "${compiler_dir}/opt")
-if (EXISTS "${opt_candidate}" AND NOT IS_DIRECTORY "${opt_candidate}")
-  # Try to use the opt consistent with the CUDA compiler.
-  set(opt "${opt_candidate}")
-elseif (NOT OPENMP_STANDALONE_BUILD AND NOT CMAKE_CROSSCOMPILING)
-  # Use opt that is also built in the same project.
-  set(opt "$<TARGET_FILE:opt>")
-else()
-  libomptarget_say("Not building deviceRTL: opt not found")
+  libomptarget_say("Not building DeviceRTL. No appropriate clang found")
   return()
 endif()
 
 # TODO: This part needs to be refined when libomptarget is going to support
 # Windows!
 # TODO: This part can also be removed if we can change the clang driver to make
 # it support device only compilation.
 if(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "x86_64")
   set(aux_triple x86_64-unknown-linux-gnu)
 elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "ppc64le")
   set(aux_triple powerpc64le-unknown-linux-gnu)
 elseif(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "aarch64")
   set(aux_triple aarch64-unknown-linux-gnu)
 else()
-  libomptarget_say("Not building CUDA offloading device RTL: unknown host arch: ${CMAKE_HOST_SYSTEM_PROCESSOR}")
+  libomptarget_say("Not building DeviceRTL: unknown host arch: ${CMAKE_HOST_SYSTEM_PROCESSOR}")
   return()
 endif()
 
 set(devicertl_base_directory ${CMAKE_CURRENT_SOURCE_DIR})
 set(include_directory ${devicertl_base_directory}/include)
 set(source_directory ${devicertl_base_directory}/src)
 
 set(all_capabilities 35 37 50 52 53 60 61 62 70 72 75 80)

 endif()
 
 # Check all SM values
 foreach(sm ${nvptx_sm_list})
   if (NOT ${sm} IN_LIST all_capabilities)
     libomptarget_warning_say("[NVPTX] Compute capability ${sm} is not supported. Make sure clang can work with it.")
   endif()
 endforeach()
 
-# Override default MAX_SM in src/target_impl.h if requested
-if (DEFINED LIBOMPTARGET_NVPTX_MAX_SM)
-  set(MAX_SM_DEFINITION "-DMAX_SM=${LIBOMPTARGET_NVPTX_MAX_SM}")
-endif()
-
 # Activate RTL message dumps if requested by the user.
 set(LIBOMPTARGET_DEVICE_DEBUG FALSE CACHE BOOL
-  "Activate NVPTX device RTL debug messages.")
+  "Activate DeviceRTL debug messages.")
 
-if ("${cuda_compiler}" STREQUAL "$<TARGET_FILE:clang>")
-  libomptarget_say("Building LLVM bitcode offloading device RTL using in-tree clang.")
-else ()
-  libomptarget_say("Building LLVM bitcode offloading device RTL using ${cuda_compiler}")
-endif ()
 
 set(src_files
   ${source_directory}/Configuration.cpp
   ${source_directory}/Debug.cpp
   ${source_directory}/Kernel.cpp
   ${source_directory}/Mapping.cpp
   ${source_directory}/Misc.cpp
   ${source_directory}/Parallelism.cpp
   ${source_directory}/Reduction.cpp

 if(${LIBOMPTARGET_DEVICE_DEBUG})
   list(APPEND bc_flags -DOMPTARGET_DEBUG=-1)
 else()
   list(APPEND bc_flags -DOMPTARGET_DEBUG=0)
 endif()
 
 # Create target to build all Bitcode libraries.
 add_custom_target(omptarget-new-nvptx-bc)
 add_dependencies(omptarget-new-nvptx-bc opt llvm-link)
 
 # Generate a Bitcode library for all the compute capabilities the user requested
 foreach(sm ${nvptx_sm_list})
   # TODO: replace this with declare variant and isa selector.
   set(cuda_flags -Xclang -target-cpu -Xclang sm_${sm} "-D__CUDA_ARCH__=${sm}0")
   set(bc_files "")
   foreach(src ${src_files})
     get_filename_component(infile ${src} ABSOLUTE)
     get_filename_component(outfile ${src} NAME)
     set(outfile "${outfile}-sm_${sm}.bc")
 
     add_custom_command(OUTPUT ${outfile}
-      COMMAND ${cuda_compiler} ${bc_flags}
-        ${cuda_flags} ${MAX_SM_DEFINITION} ${infile} -o ${outfile}
+      COMMAND ${CLANG_TOOL} ${bc_flags}
+        ${cuda_flags} ${infile} -o ${outfile}
       DEPENDS ${infile}
       IMPLICIT_DEPENDS CXX ${infile}
       COMMENT "Building LLVM bitcode ${outfile}"
       VERBATIM
     )
-    if("${cuda_compiler}" STREQUAL "$<TARGET_FILE:clang>")
+    if("${CLANG_TOOL}" STREQUAL "$<TARGET_FILE:clang>")
       # Add a file-level dependency to ensure that clang is up-to-date.
       # By default, add_custom_command only builds clang if the
       # executable is missing.
       add_custom_command(OUTPUT ${outfile}
         DEPENDS clang
         APPEND
       )
     endif()
     set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${outfile})
 
     list(APPEND bc_files ${outfile})
   endforeach()
 
   set(bclib_name "libomptarget-new-nvptx-sm_${sm}.bc")
 
   # Link to a bitcode library.
   add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}
-      COMMAND ${bc_linker}
+      COMMAND ${LINK_TOOL}
         -o ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name} ${bc_files}
       DEPENDS ${bc_files}
       COMMENT "Linking LLVM bitcode ${bclib_name}"
   )
-  if("${bc_linker}" STREQUAL "$<TARGET_FILE:llvm-link>")
-    # Add a file-level dependency to ensure that llvm-link is up-to-date.
-    # By default, add_custom_command only builds llvm-link if the
-    # executable is missing.
-    add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}
-      DEPENDS llvm-link
-      APPEND
-    )
-  endif()
 
   add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}_opt
-      COMMAND ${opt} ${link_opt_flags} ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}
+      COMMAND ${OPT_TOOL} ${link_opt_flags} ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}
                       -o ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}
       DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}
       COMMENT "Optimizing LLVM bitcode ${bclib_name}"
   )
-  if("${opt}" STREQUAL "$<TARGET_FILE:opt>")
-    # Add a file-level dependency to ensure that opt is up-to-date.
-    # By default, add_custom_command only builds opt if the
-    # executable is missing.
+
+  # Add a file-level dependency to ensure that llvm-link and opt are up-to-date.
+  # By default, add_custom_command only builds the tool if the executable is missing
+  if("${LINK_TOOL}" STREQUAL "$<TARGET_FILE:llvm-link>")
+    add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}
+      DEPENDS llvm-link
+      APPEND)
+  endif()
+  if("${OPT_TOOL}" STREQUAL "$<TARGET_FILE:opt>")
     add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}_opt
       DEPENDS opt
-      APPEND
-    )
+      APPEND)
   endif()
+
   set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${bclib_name})
 
   set(bclib_target_name "omptarget-new-nvptx-sm_${sm}-bc")
 
   add_custom_target(${bclib_target_name} ALL DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}_opt)
   add_dependencies(omptarget-new-nvptx-bc ${bclib_target_name})
   add_dependencies(${bclib_target_name} opt llvm-link)
 
   # Copy library to destination.
   add_custom_command(TARGET ${bclib_target_name} POST_BUILD
                     COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}
                     ${LIBOMPTARGET_LIBRARY_DIR})
 
   # Install bitcode library under the lib destination folder.
   install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name} DESTINATION "${OPENMP_INSTALL_LIBDIR}")
 endforeach()