[PATCH] D77240: [CUDA] Add missing cmath overloads

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert abandoned this revision.
jdoerfert added a comment.

In D77240#1957468 , @tra wrote:

> In D77240#1957386 , @jdoerfert wrote:
>
> > I just noticed those as well. I forgot to put the new definitions into the 
> > forward declare header. Will do it in a second. The OpenMP math overlay 
> > doesn't have one so I forgot :(
>
>
> I'm not sure how it's going to help. The problem is that the functions are 
> already defined by CUDA headers. Moving the duplicate definition to a 
> different header does not change that. 
>  Bottom line is that the already-existing definitions are not needed for 
> CUDA. If OpenMP needs them, then they should probably go into an 
> OpenMP-specific header.


I see. In my CUDA headers `float acos(float)` is conditionally defined as well 
but as a `__cudart_builtin__`.
So it seems we can redefine those but not the `float acosh(float)` which is a 
`__MATH_FUNCTIONS_DECL__` for some configurations.

I'll move the new functions into an OpenMP only header. I thought they might be 
needed here too but that was wrong. Thanks for spending the time on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77240



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

[PATCH] D77240: [CUDA] Add missing cmath overloads

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D77240#1957386 , @jdoerfert wrote:

> I just noticed those as well. I forgot to put the new definitions into the 
> forward declare header. Will do it in a second. The OpenMP math overlay 
> doesn't have one so I forgot :(


I'm not sure how it's going to help. The problem is that the functions are 
already defined by CUDA headers. Moving the duplicate definition to a different 
header does not change that. 
Bottom line is that the already-existing definitions are not needed for CUDA. 
If OpenMP needs them, then they should probably go into an OpenMP-specific 
header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77240



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

[PATCH] D77240: [CUDA] Add missing cmath overloads

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I just noticed those as well. I forgot to put the new definitions into the 
forward declare header. Will do it in a second. The OpenMP math overlay doesn't 
have one so I forgot :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77240



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

[PATCH] D77240: [CUDA] Add missing cmath overloads

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

We do have a problem. With your patch I see a lot of errors about function 
redefinitions conflicting with the ones in CUDA's `math_functions.hpp`:

E.g:

  In file included from :1:
  In file included from 
/usr/local/google/home/tra/work/llvm/build/release+assert+zapcc/lib/clang/11.0.0/include/__clang_cuda_runtime_wrapper.h:398:
  
/usr/local/google/home/tra/work/llvm/build/release+assert+zapcc/lib/clang/11.0.0/include/__clang_cuda_cmath.h:60:18:
 error: redefinition of 'acosh'
  __DEVICE__ float acosh(float __x) { return ::acoshf(__x); }
   ^
  
/usr/local/google/home/tra/local/cuda-10.0/include/crt/math_functions.hpp:624:31:
 note: previous definition is here
  __MATH_FUNCTIONS_DECL__ float acosh(float a)

Full list of conflicting functions in CUDA 9.0, 10.0, 10.1, 10.2

  redefinition of 'acosh'
  redefinition of 'asinh'
  redefinition of 'atanh'
  redefinition of 'cbrt'
  redefinition of 'erf'
  redefinition of 'erfc'
  redefinition of 'exp2'
  redefinition of 'expm1'
  redefinition of 'fdim'
  redefinition of 'hypot'
  redefinition of 'ilogb'
  redefinition of 'lgamma'
  redefinition of 'llrint'
  redefinition of 'llround'
  redefinition of 'log1p'
  redefinition of 'log2'
  redefinition of 'logb'
  redefinition of 'lrint'
  redefinition of 'lround'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77240



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

[PATCH] D77240: [CUDA] Add missing cmath overloads

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D77240#1955755 , @tra wrote:

> In D77240#1955724 , @jdoerfert wrote:
>
> > At least that one is defined in what is "now" `__clang_cuda_math.h`:
>
>
> Cool. We may be OK, but we still need to verify it. Math headers are rather 
> fragile and we need to make sure it all still works two different standard 
> libraries and all CUDA versions. 
>  Unfortunately my CUDA build bot has decided to die the day we've started 
> working from home, so it has to be done manually.
>  Let me try patching in your changes and try them with the CUDA versions I 
> have. Stay tuned.


Thank you very much! I (now) know what pain math headers are... These are 
almost the last infrastructure changes I need, there is a problem with the 
include path order on some system but for that I first need to write a patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77240



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

[PATCH] D77240: [CUDA] Add missing cmath overloads

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D77240#1955724 , @jdoerfert wrote:

> At least that one is defined in what is "now" `__clang_cuda_math.h`:


Cool. We may be OK, but we still need to verify it. Math headers are rather 
fragile and we need to make sure it all still works two different standard 
libraries and all CUDA versions. 
Unfortunately my CUDA build bot has decided to die the day we've started 
working from home, so it has to be done manually.
Let me try patching in your changes and try them with the CUDA versions I have. 
Stay tuned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77240



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

[PATCH] D77240: [CUDA] Add missing cmath overloads

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D77240#1955678 , @tra wrote:

> We'll need to make sure that all of these new functions are vailable in all 
> supported CUDA versions.
>  E.g. `acoshf` does not seem to be present in CUDA-9.


At least that one is defined in what is "now" `__clang_cuda_math.h`:

`__DEVICE__ float acoshf(float __a) { return __nv_acoshf(__a); } `


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77240



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

[PATCH] D77240: [CUDA] Add missing cmath overloads

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

We'll need to make sure that all of these new functions are vailable in all 
supported CUDA versions.
E.g. `acoshf` does not seem to be present in CUDA-9.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77240



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

[PATCH] D77240: [CUDA] Add missing cmath overloads

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added a reviewer: tra.
Herald added subscribers: bollu, yaxunl.
Herald added a project: clang.

Some function overloads for floats were missing, found by running a test
suite [0] with the OpenMP overlay.

[0] https://github.com/TApplencourt/OmpVal

Depends on D77239 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77240

Files:
  clang/lib/Headers/__clang_cuda_cmath.h


Index: clang/lib/Headers/__clang_cuda_cmath.h
===
--- clang/lib/Headers/__clang_cuda_cmath.h
+++ clang/lib/Headers/__clang_cuda_cmath.h
@@ -57,15 +57,24 @@
 __DEVICE__ const double abs(const double __x) { return ::fabs((double)__x); }
 #endif
 __DEVICE__ float acos(float __x) { return ::acosf(__x); }
+__DEVICE__ float acosh(float __x) { return ::acoshf(__x); }
 __DEVICE__ float asin(float __x) { return ::asinf(__x); }
+__DEVICE__ float asinh(float __x) { return ::asinhf(__x); }
 __DEVICE__ float atan(float __x) { return ::atanf(__x); }
+__DEVICE__ float atanh(float __x) { return ::atanhf(__x); }
 __DEVICE__ float atan2(float __x, float __y) { return ::atan2f(__x, __y); }
+__DEVICE__ float cbrt(float __x) { return ::cbrtf(__x); }
 __DEVICE__ float ceil(float __x) { return ::ceilf(__x); }
 __DEVICE__ float cos(float __x) { return ::cosf(__x); }
 __DEVICE__ float cosh(float __x) { return ::coshf(__x); }
+__DEVICE__ float erf(float __x) { return ::erff(__x); }
+__DEVICE__ float erfc(float __x) { return ::erfcf(__x); }
 __DEVICE__ float exp(float __x) { return ::expf(__x); }
+__DEVICE__ float exp2(float __x) { return ::exp2f(__x); }
+__DEVICE__ float expm1(float __x) { return ::expm1f(__x); }
 __DEVICE__ float fabs(float __x) __NOEXCEPT { return ::fabsf(__x); }
 __DEVICE__ float floor(float __x) { return ::floorf(__x); }
+__DEVICE__ float fdim(float __x, float __y) { return ::fdimf(__x, __y); }
 __DEVICE__ float fmod(float __x, float __y) { return ::fmodf(__x, __y); }
 // TODO: remove when variant is supported
 #ifndef _OPENMP
@@ -82,6 +91,8 @@
   return ::frexpf(__arg, __exp);
 }
 
+__DEVICE__ float hypot(float __x, float __y) { return ::hypotf(__x, __y); }
+__DEVICE__ int ilogb(float __x) { return ::ilogbf(__x); }
 // For inscrutable reasons, the CUDA headers define these functions for us on
 // Windows.
 #ifndef _MSC_VER
@@ -137,9 +148,18 @@
 __DEVICE__ float ldexp(float __arg, int __exp) {
   return ::ldexpf(__arg, __exp);
 }
+__DEVICE__ float lgamma(float __x) { return ::lgammaf(__x); }
+__DEVICE__ long long int llrint(float __x) { return ::llrintf(__x); }
+__DEVICE__ long long int llround(float __x) { return ::llroundf(__x); }
 __DEVICE__ float log(float __x) { return ::logf(__x); }
 __DEVICE__ float log10(float __x) { return ::log10f(__x); }
+__DEVICE__ float log1p(float __x) { return ::log1pf(__x); }
+__DEVICE__ float log2(float __x) { return ::log2f(__x); }
+__DEVICE__ float logb(float __x) { return ::logbf(__x); }
+__DEVICE__ long int lrint(float __x) { return ::lrintf(__x); }
+__DEVICE__ long int lround(float __x) { return ::lroundf(__x); }
 __DEVICE__ float modf(float __x, float *__iptr) { return ::modff(__x, __iptr); 
}
+__DEVICE__ float nextafter(float __x, float __y) { return ::nextafterf(__x, 
__y); }
 __DEVICE__ float pow(float __base, float __exp) {
   return ::powf(__base, __exp);
 }
@@ -149,6 +169,9 @@
 __DEVICE__ double pow(double __base, int __iexp) {
   return ::powi(__base, __iexp);
 }
+__DEVICE__ float remainder(float __x, float __y) { return ::remainderf(__x, 
__y); }
+__DEVICE__ float scalbln(float __x, long int __y) { return ::scalblnf(__x, 
__y); }
+__DEVICE__ float scalbn(float __x, int __y) { return ::scalbnf(__x, __y); }
 __DEVICE__ bool signbit(float __x) { return ::__signbitf(__x); }
 __DEVICE__ bool signbit(double __x) { return ::__signbitd(__x); }
 __DEVICE__ float sin(float __x) { return ::sinf(__x); }
@@ -156,6 +179,7 @@
 __DEVICE__ float sqrt(float __x) { return ::sqrtf(__x); }
 __DEVICE__ float tan(float __x) { return ::tanf(__x); }
 __DEVICE__ float tanh(float __x) { return ::tanhf(__x); }
+__DEVICE__ float tgamma(float __x) { return ::tgammaf(__x); }
 
 // Notably missing above is nexttoward.  We omit it because
 // libdevice doesn't provide an implementation, and we don't want to be in the


Index: clang/lib/Headers/__clang_cuda_cmath.h
===
--- clang/lib/Headers/__clang_cuda_cmath.h
+++ clang/lib/Headers/__clang_cuda_cmath.h
@@ -57,15 +57,24 @@
 __DEVICE__ const double abs(const double __x) { return ::fabs((double)__x); }
 #endif
 __DEVICE__ float acos(float __x) { return ::acosf(__x); }
+__DEVICE__ float acosh(float __x) { return ::acoshf(__x); }
 __DEVICE__ float asin(float __x) { return ::asinf(__x); }
+__DEVICE__ float asinh(float __x) { return ::asinhf(__x); }
 __DEVICE__ float atan(float __x) { return ::atanf(__x); }
+__DEVICE__ float atanh(float __x) { return ::atanhf(__x); }