[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

tagged request changes because I think we should ifdef around complex before (or while) landing this, as defining `__CUDA__`, even transiently, is a user hostile thing to do from amdgpu openmp

It is *really* ugly that we have cuda and hip implementations of cmath. Opening them in diff it looks very likely that the hip one was create by copying and pasting the cuda one then hacking on it a bit. This means we have openmp specific fixes already done in the cuda one and VS2019 workarounds in the hip one. It also means there are a bunch of differences that might be important or might be spurious, like whether a function calls ::scalbln or std::scalbln. This is particularly frustrating because we should be able isolate essentially all the differences between __nv and __ocml functions in math.h.



================
Comment at: clang/lib/Headers/openmp_wrappers/cmath:30
 
 #pragma omp begin declare variant match(                                       \
     device = {arch(nvptx, nvptx64)}, implementation = {extension(match_any, allow_templates)})
----------------
this declare variant will not match amdgcn


================
Comment at: clang/lib/Headers/openmp_wrappers/cmath:43
 
 __DEVICE__ float acosh(float __x) { return ::acoshf(__x); }
 __DEVICE__ float asinh(float __x) { return ::asinhf(__x); }
----------------
which means that amdgcn is not going to pick up any of these overloads, but that looks like it's actually OK because clang_hip_cmath does define them (I think, there are a lot of macros involved)


================
Comment at: clang/lib/Headers/openmp_wrappers/math.h:41
 #pragma omp begin declare variant match(                                       \
     device = {arch(nvptx, nvptx64)}, implementation = {extension(match_any)})
 
----------------
@jdoerfert do you know why we have match_any here? wondering if the amdgcn variant below should have the same


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104904