JonChesterfield added inline comments.

================
Comment at: clang/test/Headers/Inputs/include/cstdlib:29
 float fabs(float __x) { return __builtin_fabs(__x); }
+#endif
 
----------------
jdoerfert wrote:
> estewart08 wrote:
> > 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.
> > 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.
> 
> Can you provide an example that shows how we "loose" something? So an input 
> and command line that should work but doesn't, or that should be compiled to 
> something else. That would help me a lot.
TLDR, I think nvptx works here, but it's hard to be certain. I've put a few 
minutes into looking for something that doesn't work, then much longer trying 
to trace where the various functions come from, and have concluded that the hip 
cmath header diverging from the cuda cmath header is the root cause.

The list of functions near the top of `__clang_cuda_cmath.h` is a subset of 
libm, e.g.
```
__DEVICE__ float acos(float __x) { return ::acosf(__x); }
but no acosh
```
Later on in the file are:
```
__CUDA_CLANG_FN_INTEGER_OVERLOAD_1(double, acosh)
```
but these are guarded by `#ifndef __OPENMP_NVPTX__`, which suggests they are 
not included when using the header from openmp.

However, openmp_wrappers/cmath does include `__DEVICE__ float acosh(float __x) 
{ return ::acoshf(__x); }` under the comment
`// Overloads not provided by the CUDA wrappers by by the CUDA system headers`

Finally there are some functions that are not in either list, such as 
fma(float,float,float), but which are nevertheless resolved, at a guess in a 
glibc header.

My current theory is that nvptx gets the set of functions right through a 
combination of cuda headers, clang cuda headers, clang openmp headers, system 
headers. At least, the half dozen I've tried work, and iirc it passes the OvO 
suite which I believe calls all of them. Wimplicit-float-conversion complains 
about a few but that seems minor.

Further, I think hip does not get this right, because the hip cmath header has 
diverged from the cuda one, and the amdgpu openmp implementation that tries to 
use the hip headers does not pass the OvO suite without some hacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104904

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to