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

Fri Jul 9 12:10:42 PDT 2021

estewart08 added inline comments.

================
Comment at: clang/test/Headers/Inputs/include/cstdlib:29
 float fabs(float __x) { return __builtin_fabs(__x); }
+#endif

----------------
JonChesterfield wrote:
> jdoerfert wrote:
> > That seems to be fundamentally broken then, but let's see, maybe it will somehow work anyway.
> I thought fabs was in math, not stdlib. Not sure what this file is doing but the functions above are inline and fabs isn't
I am afraid this is just a workaround to get openmp_device_math_isnan.cpp to pass for AMDGCN. This stems from not having an #ifndef OPENMP_AMDGCN in __clang_hip_cmath.h where 'using ::fabs' is present. Currently, OPENMP_AMDGCN uses all of the overloaded functions created by the HIP macros where NVPTX does not use the CUDA overloads. This may be a new topic of discussion.

https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/__clang_cuda_cmath.h#L191

By using this ifndef, it seems NVPTX looses quite a few overloaded functions. Are these meant to eventually be present in openmp_wrappers/cmath? Not sure what issues @jdoerfert ran into with D75788.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104904