[Openmp-commits] [PATCH] D121467: [Libomptarget] Create device math wrappers

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield requested changes to this revision.
JonChesterfield added a comment.
This revision now requires changes to proceed.

OK, I think I follow.

What we have is:
1/ Long list of libm symbols as standardised with some ad hoc extra ones
2/ Header file mapping libm symbols onto cuda functions and intrinsics
3/ Header file mapping libm symbols onto hip (well, ocml) functions and intrinsics

What we want is:
Applications #include math.h and stuff works. Maybe an extra #include math_gpu.h containing the ad hoc extra ones.
Optimisations in LLVM that are tied to the C symbol name work

Thus, we instantiate a bitcode library that maps libm + some extra symbols onto the cuda/ocml library by reusing the 'header' file with some macro hackery to make it do the right thing across libraries.

We might want to split OpenMPMath.h into standard and extensions, because cuda put the extensions in the global namespace and some applications written in openmp are going to define the same symbols.

Strategy looks sound to me. It's unfortunate that the symbol remap tables are written as C++ header files but at least they already exist. Copy&pasting them into openmp is going to be a maintenance disaster almost immediately, I think we need to add some macros to the headers in clang and include them via CMakeLists setting the include path appropriately.

The review of adding those macros can include a link to this diff to show that said macros are the lesser of two evils.

AMD have already done something rather like this for Fortran (which cannot use the C++ headers), see https://github.com/RadeonOpenCompute/llvm-project/blob/amd-stg-open/openmp/libomptarget/libm/src/libm.c with corresponding macros added to the clang headers.

I'm therefore marking this 'requested changes', with the proviso that the change I'm requesting is that we add more ugliness to the clang headers to make this work out OK.

We might want to put a file called math.h in the override directory and not #include_next the system math.h on the gpu.



================
Comment at: openmp/libomptarget/DeviceLib/src/OpenMPMath.cpp:14
+extern "C" {
+
+__DEVICE__ int __omp_abs(int __a) { return abs(__a); }
----------------
This is quite code generator / xmacro friendly. Should be able to have something closer to

#define M(ARITY, SYMBOL)...

M(abs)
M(fabs)
M(acos)
M(cos)
...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121467/new/

https://reviews.llvm.org/D121467