[libomptarget][nfc] Move some source into common from nvptx

[JonChesterfield]

Summary

[libomptarget][nfc] Move some source into common from nvptx

Moves some source that compiles cleanly under amdgcn into a common subdirectory
Includes some non-trivial files and some headers. Keeps the cuda file extension.

The build systems for different architectures seem unlikely to have much in
common. The idea is therefore to set include paths such that files under
common/src compile as if they were under arch/src as the mechanism for sharing.
In particular, files under common/src need to be able to include target_impl.h.

The corresponding -Icommon is left out in favour of explicit includes on the
basis that the it makes it clearer which files under common are used by a given
architecture.

[ABataev]

It breaks the build:

[547/1109] Building NVCC intermediate link file projects/openmp/libomptarget/deviceRTLs/nvptx/CMakeFiles/omptarget-nvptx.dir/omptarget-nvptx_intermediate_link.o 
FAILED: projects/openmp/libomptarget/deviceRTLs/nvptx/CMakeFiles/omptarget-nvptx.dir/omptarget-nvptx_intermediate_link.o
projects/openmp/libomptarget/deviceRTLs/nvptx/CMakeFiles/omptarget-nvptx.dir/src/./omptarget-nvptx_generated_data_sharing.cu.o:1:8: warning: null character(s) ignored [enabled by default]
ELF´)}xK#9_Z20isRuntimeI?dv
...

[JonChesterfield]

• Remove whitespace change to data_sharing

[JonChesterfield]

In D70328#1748116, @ABataev wrote:

It breaks the build:

...omptarget-nvptx_generated_data_sharing.cu.o:1:8: warning: null character(s) ignored [enabled by default]
 ...

That's interesting. I see the same failure on one machine but not on a second so thought it was a quirk of the local setup. Please could you try the new revision? It leaves data_sharing.cu totally unchanged (the first revision ran that file through a remove-non-ascii perl fragment intended to patch up whatever character nvcc didn't like).

[ABataev]

In D70328#1748187, @JonChesterfield wrote:

In D70328#1748116, @ABataev wrote:

It breaks the build:

...omptarget-nvptx_generated_data_sharing.cu.o:1:8: warning: null character(s) ignored [enabled by default]
 ...

That's interesting. I see the same failure on one machine but not on a second so thought it was a quirk of the local setup. Please could you try the new revision? It leaves data_sharing.cu totally unchanged (the first revision ran that file through a remove-non-ascii perl fragment intended to patch up whatever character nvcc didn't like).

Nope, still the same. I think the reason is -x cu option in nvcc invocation command, obj file is treated as cuda source file with this option

[JonChesterfield]

In D70328#1748208, @ABataev wrote:

Nope, still the same. I think the reason is -x cu option in nvcc invocation command, obj file is treated as cuda source file with this option

Yep, good spot. -x cu is right for compiling the source but not for linking.

There's an alternative '-Xcompiler -x -Xcompiler cu', or '--compiler-options -x --compiler-options cu', but that also passes the arguments to gcc which errors with 'language not recognised'

Which I think means -x cu is not usable via cmake. How do you feel about shims like:

nvptx/loop.cu:
#include "common/src/loop.cpp"

Testing that approach now

[JonChesterfield]

• Work around nvcc's limited support for overriding language

[jdoerfert]

One request but I'm fine with the workaround (includes) if they solve the problem

openmp/libomptarget/deviceRTLs/common/debug.h

1

Please add the - C++ - annotation to header files

openmp/libomptarget/deviceRTLs/nvptx/cancel.cu

1 ↗

(On Diff #229634)

This is not pretty but also not problematic. It's not like we have to maintain any more code, just a single file.

[JonChesterfield]

• Add C++ annotation to leading comment in header files

[JonChesterfield]

Reinstating the unity.cu patch would equivalently work around the language override challenge. Happy to go that way instead if the single line task.cu et al are considered too ugly.

[JonChesterfield]

Simplified initial suggestion by dropping the .cu => .cpp rename. If we agree on moving the source to a common location, I can follow up with some cmake that builds the corresponding files for amdgcn. Making the common code look less like cuda can happen later.

[JonChesterfield]

• - drop the contentious file rename part

[ABataev]

The patch does not break the build.

[JonChesterfield]

In D70328#1750148, @ABataev wrote:

The patch does not break the build.

Which patch do you refer to?

Neither the task.cu #include common/task.cpp nor moving task.cu without renaming break the build for me, but it's worth noting which configuration you've checked.

[ABataev]

In D70328#1750166, @JonChesterfield wrote:

In D70328#1750148, @ABataev wrote:

The patch does not break the build.

Which patch do you refer to?

Neither the task.cu #include common/task.cpp nor moving task.cu without renaming break the build for me, but it's worth noting which configuration you've checked.

I was just saying that the patch is good and it does not(!) break the build. SO, if everybody else is fine with it, you can land it.

[JonChesterfield]

That's great, thanks. We will imminently have (a subset of) deviceRTL available for a second architecture. I'll look at the CMake next.

openmp/libomptarget/deviceRTLs/common/src/cancel.cu

 //===------ cancel.cu - NVPTX OpenMP cancel interface ------------ CUDA -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // Interface to be used in the implementation of OpenMP cancel.
 //
 //===----------------------------------------------------------------------===//
 
 #include "interface.h"
-#include "debug.h"
+#include "common/debug.h"
 
 EXTERN int32_t __kmpc_cancellationpoint(kmp_Ident *loc, int32_t global_tid,
                                         int32_t cancelVal) {
   PRINT(LD_IO, "call kmpc_cancellationpoint(cancel val %d)\n", (int)cancelVal);
   // disabled
   return 0;
 }
 
 EXTERN int32_t __kmpc_cancel(kmp_Ident *loc, int32_t global_tid,
                              int32_t cancelVal) {
   PRINT(LD_IO, "call kmpc_cancel(cancel val %d)\n", (int)cancelVal);
   // disabled
   return 0;
 }

openmp/libomptarget/deviceRTLs/common/src/critical.cu

 //===------ critical.cu - NVPTX OpenMP critical ------------------ CUDA -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains the implementation of critical with KMPC interface
 //
 //===----------------------------------------------------------------------===//
 
 #include "interface.h"
-#include "debug.h"
+#include "common/debug.h"
 
 EXTERN
 void __kmpc_critical(kmp_Ident *loc, int32_t global_tid,
                      kmp_CriticalName *lck) {
   PRINT0(LD_IO, "call to kmpc_critical()\n");
   omp_set_lock((omp_lock_t *)lck);
 }
 
 EXTERN
 void __kmpc_end_critical(kmp_Ident *loc, int32_t global_tid,
                          kmp_CriticalName *lck) {
   PRINT0(LD_IO, "call to kmpc_end_critical()\n");
   omp_unset_lock((omp_lock_t *)lck);
 }

openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt

if(NOT LIBOMPTARGET_NVPTX_ALTERNATE_GCC_HOST_COMPILER)
     return()
   endif()
   set(CUDA_HOST_COMPILER "${LIBOMPTARGET_NVPTX_ALTERNATE_GCC_HOST_COMPILER}" CACHE FILEPATH "" FORCE)
 endif()
 
 get_filename_component(devicertl_base_directory
   ${CMAKE_CURRENT_SOURCE_DIR}
   DIRECTORY)
+set(devicertl_common_directory
+  ${devicertl_base_directory}/common)
+set(devicertl_nvptx_directory
+  ${devicertl_base_directory}/nvptx)
 
 if(LIBOMPTARGET_DEP_CUDA_FOUND)
   libomptarget_say("Building CUDA offloading device RTL.")
 
   # We really don't have any host code, so we don't need to care about
   # propagating host flags.
   set(CUDA_PROPAGATE_HOST_FLAGS OFF)
 
   set(cuda_src_files
-      src/cancel.cu
-      src/critical.cu
+      ${devicertl_common_directory}/src/cancel.cu
+      ${devicertl_common_directory}/src/critical.cu
       src/data_sharing.cu
       src/libcall.cu
-      src/loop.cu
+      ${devicertl_common_directory}/src/loop.cu
       src/omptarget-nvptx.cu
       src/parallel.cu
       src/reduction.cu
       src/support.cu
       src/sync.cu
-      src/task.cu
+      ${devicertl_common_directory}/src/task.cu
   )
 
   set(omp_data_objects src/omp_data.cu)
 
   # Get the compute capability the user requested or use SM_35 by default.
   # SM_35 is what clang uses by default.
   set(default_capabilities 35)
   if (DEFINED LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITY)
if(LIBOMPTARGET_DEP_CUDA_FOUND)
   if(${LIBOMPTARGET_NVPTX_DEBUG})
     set(CUDA_DEBUG -DOMPTARGET_NVPTX_DEBUG=-1 -g --ptxas-options=-v)
   endif()
 
   # NVPTX runtime library has to be statically linked. Dynamic linking is not
   # yet supported by the CUDA toolchain on the device.
   set(BUILD_SHARED_LIBS OFF)
   set(CUDA_SEPARABLE_COMPILATION ON)
-  list(APPEND CUDA_NVCC_FLAGS -I${devicertl_base_directory})
+  list(APPEND CUDA_NVCC_FLAGS -I${devicertl_base_directory}
+                              -I${devicertl_nvptx_directory}/src)
   cuda_add_library(omptarget-nvptx STATIC ${cuda_src_files} ${omp_data_objects}
       OPTIONS ${CUDA_ARCH} ${CUDA_DEBUG})
 
   # Install device RTL under the lib destination folder.
   install(TARGETS omptarget-nvptx ARCHIVE DESTINATION "${OPENMP_INSTALL_LIBDIR}")
 
   target_link_libraries(omptarget-nvptx ${CUDA_LIBRARIES})
 
if(LIBOMPTARGET_DEP_CUDA_FOUND)
   if (${LIBOMPTARGET_NVPTX_ENABLE_BCLIB})
     if (NOT ${LIBOMPTARGET_NVPTX_BCLIB_SUPPORTED})
       libomptarget_error_say("Cannot build CUDA LLVM bitcode offloading device RTL!")
     endif()
     libomptarget_say("Building CUDA LLVM bitcode offloading device RTL.")
 
     # Set flags for LLVM Bitcode compilation.
     set(bc_flags ${LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER_FLAGS}
-                 -I${devicertl_base_directory})
+                 -I${devicertl_base_directory}
+                 -I${devicertl_nvptx_directory}/src)
+
     if(${LIBOMPTARGET_NVPTX_DEBUG})
       set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=-1)
     else()
       set(bc_flags ${bc_flags} -DOMPTARGET_NVPTX_DEBUG=0)
     endif()
 
     # CUDA 9 header files use the nv_weak attribute which clang is not yet prepared
     # to handle. Therefore, we use 'weak' instead. We are compiling only for the

openmp/libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.h

 
 // std includes
 #include <inttypes.h>
 #include <math.h>
 #include <stdlib.h>
 
 // local includes
 #include "target_impl.h"
-#include "debug.h"     // debug
+#include "common/debug.h"     // debug
 #include "interface.h" // interfaces with omp, compiler, and user
-#include "state-queue.h"
+#include "common/state-queue.h"
 #include "support.h"
 
 #define OMPTARGET_NVPTX_VERSION 1.1
 
 // used by the library for the interface with the app
 #define DISPATCH_FINISHED 0
 #define DISPATCH_NOTFINISHED 1
 

openmp/libomptarget/deviceRTLs/nvptx/src/support.cu

 //===--------- support.cu - NVPTX OpenMP support functions ------- CUDA -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // Wrapper implementation to some functions natively supported by the GPU.
 //
 //===----------------------------------------------------------------------===//
 
 #include "support.h"
-#include "debug.h"
+#include "common/debug.h"
 #include "omptarget-nvptx.h"
 
 ////////////////////////////////////////////////////////////////////////////////
 // Execution Parameters
 ////////////////////////////////////////////////////////////////////////////////
 
 DEVICE void setExecutionParameters(ExecutionMode EMode, RuntimeMode RMode) {
   execution_param = EMode;