[libomptarget] Make libomptarget.devicertl.a built in all cases.

[ye-luo]

Summary

Make libomptarget.device.a built when using -DLLVM_ENABLE_PROJECTS=openmp
Use add_custom_command.

[jhuber6]

So this changes the compilation to just use the Clang binary found in the install directory and hope it works like we did previously. It's not ideal, I was planning on deleting all the code above and deprecating the old bitcode library. As I understand the reason we'd want to do this is to support compiling the OpenMP runtime with a different compiler than the clang that's necessary to build the device runtime. I wonder if instead it's possible to make the RUNTIMES portion of the build only apply to the device runtime, since realistically that's all we need it for.

Also with this method, we no longer inherit the -DCMAKE_BUILD_TYPE=Release and instead use the old clang-opt-flags which only did -O1. This removes a lot of the benefits users will see using this library for LTO over the traditional bitcode library. We don't do O3 on the bitcode library because last I checked it broke everything.

[ye-luo]

In D128130#3594408, @jhuber6 wrote:

So this changes the compilation to just use the Clang binary found in the install directory and hope it works like we did previously.

We don't do "hope it works" but we are sure it works and I don't think we found undesired clang. If undesired clang gets picked up. Please report issues.
Actually in the case of ENABLE_PROJECTS=openmp, there is no clang binary searching but directly referencing the clang cmake target. The just-built clang is guaranteed to be picked up.
In the case of ENABLE_RUNTIMES=openmp, LLVM_DIR is passed in, I would consider it closer to "hope it works". Even relying on CMAKE_CXX_COMPILER in runtime builds, you actually hope the RUNTIMES setup passes in the desired compiler, it is just a different scripting magic managed by ENABLE_RUNTIMES.

It's not ideaI was planning on deleting all the code above and deprecating the old bitcode library.

I've no problem depercating old bitcode library only if the devicertl static library can be built when OpenMP project can be built standalone or as ENABLE_PROJECTS with GCC as the host compiler for the host library part. RUNTIMES has the downside of getting contaminated by clang development. It doesn't cover my needs. The intention of this PR is to make my requirement to work. It is not "ideal" but on the other hand "ideal" solution only exits when it has actually been made. To make the deprecation and transition fast. We should consider intermediate solutions before an "ideal" solution is found.

As I understand the reason we'd want to do this is to support compiling the OpenMP runtime with a different compiler than the clang that's necessary to build the device runtime. I wonder if instead it's possible to make the RUNTIMES portion of the build only apply to the device runtime, since realistically that's all we need it for.

Correct, building libomptarget host part with GCC and only the DeviceRTL with clang. I'm requesting this compilation mode to work.
A potentially better solution you may try is to spin off DeviceRTL as a separate RUNTIME project. The ENABLE_PROJECTS=openmp and ENABLE_RUNTIMES=openmp-devicertl. However, I'm not sure how soon this can be made working and how drastic changes are acceptable.

Also with this method, we no longer inherit the -DCMAKE_BUILD_TYPE=Release and instead use the old clang-opt-flags which only did -O1. This removes a lot of the benefits users will see using this library for LTO over the traditional bitcode library. We don't do O3 on the bitcode library because last I checked it broke everything.

I just made an initial patch and chose to use existing clang-opt-flags and thus inherit '-O1', no problem just make a another clang-opt-flags-for-static with '-O3'. There are anyway many flags being hard-coded, I don't see the issue of hard-coding more flags. For my understanding, RUNTIMES itself is not necessary a perfect feature. It is a compromise. RUNTIME doesn't allow precise control of libraries neither. RUNTIME="libcxx;openmp" I don't see easy way of configuring two runtime libraries compiled with different compile type. RUNTIME only brings in some benefit but it also carries limitations.

[ye-luo]

Use -O3 in DeviceRTL static library build.

[jhuber6]

I still much prefer the regular CMake approach, but if this solves a problem you have with building the runtime it's fine. We can revisit this later if needed.

openmp/libomptarget/DeviceRTL/CMakeLists.txt

foreach(sm ${nvptx_sm_list})
   compileDeviceRTLLibrary(sm_${sm} nvptx -target nvptx64-nvidia-cuda -DLIBOMPTARGET_BC_TARGET -Xclang -target-feature -Xclang +ptx61 "-D__CUDA_ARCH__=${sm}0")
 endforeach()
 
 add_custom_target(omptarget.devicertl.amdgpu)
 foreach(mcpu ${amdgpu_mcpus})
   compileDeviceRTLLibrary(${mcpu} amdgpu -target amdgcn-amd-amdhsa -DLIBOMPTARGET_BC_TARGET -D__AMDGCN__ -nogpulib)
 endforeach()
 
-set(LIBOMPTARGET_LLVM_VERSION "${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")
-if(NOT (CMAKE_CXX_COMPILER_ID MATCHES "[Cc]lang" AND CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL LIBOMPTARGET_LLVM_VERSION))
-  libomptarget_say("Not building static library, CMake compiler '${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}' is not 'Clang ${LIBOMPTARGET_LLVM_VERSION}'.")
-  libomptarget_say("  Use the 'LLVM_ENABLE_RUNTIMES=openmp' option instead")
-  return()
-endif()
-
 # Set the flags to build the device runtime from clang.
-set(clang_lib_flags -fopenmp -fopenmp-cuda-mode -foffload-lto -fvisibility=hidden -Xopenmp-target=nvptx64-nvidia-cuda --cuda-feature=+ptx61 -mllvm -openmp-opt-disable -nocudalib -nogpulib -nostdinc -DSHARED_SCRATCHPAD_SIZE=512)
+set(clang_lib_flags -fopenmp -fopenmp-cuda-mode -foffload-lto -fvisibility=hidden -Xopenmp-target=nvptx64-nvidia-cuda --cuda-feature=+ptx61 -mllvm -openmp-opt-disable -nocudalib -nogpulib -nostdinc -DSHARED_SCRATCHPAD_SIZE=512 -O3)
 foreach(arch ${nvptx_sm_list})
   set(clang_lib_flags ${clang_lib_flags} --offload-arch=sm_${arch})
 endforeach()
 foreach(arch ${amdgpu_mcpus})
   set(clang_lib_flags ${clang_lib_flags} --offload-arch=${arch})
 endforeach()
 
 # Build the static library version of the device runtime.
+# first create an object target
+add_library(omptarget.devicertl.all_objs OBJECT IMPORTED)
+foreach(src ${src_files})
+  get_filename_component(infile ${src} ABSOLUTE)
+  get_filename_component(outfile ${src} NAME)
+  set(outfile "${outfile}.o")
+  set(outfile_full_path "${CMAKE_CURRENT_BINARY_DIR}/${outfile}")
+
+  add_custom_command(OUTPUT ${outfile_full_path}
+    COMMAND ${CLANG_TOOL} ${clang_lib_flags} --std=c++17 -c
+            -o ${outfile_full_path}
+            -I${include_directory}
+            -I${devicertl_base_directory}/../include
+            ${LIBOMPTARGET_LLVM_INCLUDE_DIRS_DEVICERTL}
+            ${infile}
+    DEPENDS ${infile} ${include_files}
+    IMPLICIT_DEPENDS CXX ${infile}
+    COMMENT "Building device code ${outfile}"
+    VERBATIM
+  )
+  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_full_path}
+      DEPENDS clang
+      APPEND
+    )
+  endif()
+  set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${outfile_full_path})
+
+  set_property(TARGET omptarget.devicertl.all_objs APPEND PROPERTY IMPORTED_OBJECTS ${outfile_full_path})
+endforeach()
+
+# second archive all the object files into a static library
 add_library(omptarget.devicertl STATIC)
-target_compile_features(omptarget.devicertl PRIVATE cxx_std_17)
-target_include_directories(omptarget.devicertl PRIVATE ${include_directory} ${devicertl_base_directory}/../include)
-target_compile_options(omptarget.devicertl PRIVATE ${clang_lib_flags})
+set_target_properties(omptarget.devicertl PROPERTIES LINKER_LANGUAGE CXX)
+target_link_libraries(omptarget.devicertl PRIVATE omptarget.devicertl.all_objs)
 
 install(TARGETS omptarget.devicertl ARCHIVE DESTINATION ${OPENMP_INSTALL_LIBDIR})
-
-add_subdirectory(src)