[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-18 Thread Jon Chesterfield via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e649f8ef187: [openmp][nfc] Simplify macros guarding math 
complex headers (authored by JonChesterfield).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

Files:
  clang/lib/Headers/openmp_wrappers/complex
  clang/lib/Headers/openmp_wrappers/complex.h


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-18 Thread Joachim Meyer via Phabricator via cfe-commits 
fodinabor accepted this revision.
fodinabor added a comment.
This revision is now accepted and ready to land.

LGTM as well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-18 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

@fodinabor?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-16 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert accepted this revision.
jdoerfert added a comment.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-14 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added inline comments.



Comment at: clang/lib/Headers/openmp_wrappers/complex:21
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__

^ this header does not look for a macro called __CUDA__ or include any other 
headers so I believe dropping the macro can make no change to that header.

It might affect other things that happen to be included after this header, but 
iiuc cuda and openmp-nvptx both define `__CUDA__` anyway, so that could only 
break amdgpu applications that were erroneously looking for a cuda macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-14 Thread Pushpinder Singh via Phabricator via cfe-commits 
pdhaliwal added a comment.

Looks ok to me. Regression tests and runtime tests went fine. Tested a simple 
cuda and openmp kernel with `sin` function on sm_61, didn't see any issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-13 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

Cut down to only dropping the cuda define, which is sufficient to resolve 
D104904 . Haven't build/tested this diff yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-13 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield updated this revision to Diff 358250.
JonChesterfield added a comment.

- reduce patch to only dropping cuda define v2, now with missing save


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

Files:
  clang/lib/Headers/openmp_wrappers/complex
  clang/lib/Headers/openmp_wrappers/complex.h


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-13 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield updated this revision to Diff 358248.
JonChesterfield added a comment.

- reduce patch to only dropping cuda define


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

Files:
  clang/lib/Headers/__clang_cuda_complex_builtins.h
  clang/lib/Headers/openmp_wrappers/complex
  clang/lib/Headers/openmp_wrappers/complex.h


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
Index: clang/lib/Headers/__clang_cuda_complex_builtins.h
===
--- clang/lib/Headers/__clang_cuda_complex_builtins.h
+++ clang/lib/Headers/__clang_cuda_complex_builtins.h
@@ -16,7 +16,7 @@
 // to work with CUDA and OpenMP target offloading [in C and C++ mode].)
 
 #pragma push_macro("__DEVICE__")
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp declare target
 #define __DEVICE__ __attribute__((noinline, nothrow, cold, weak))
 #else
@@ -26,7 +26,7 @@
 // To make the algorithms available for C and C++ in CUDA and OpenMP we select
 // different but equivalent function versions. TODO: For OpenMP we currently
 // select the native builtins as the overload support for templates is lacking.
-#if !defined(__OPENMP_NVPTX__)
+#if !defined(_OPENMP)
 #define _ISNANd std::isnan
 #define _ISNANf std::isnan
 #define _ISINFd std::isinf
@@ -276,7 +276,7 @@
 #undef _fmaxd
 #undef _fmaxf
 
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp end declare target
 #endif
 


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
Index: clang/lib/Headers/__clang_cuda_complex_builtins.h
===
--- clang/lib/Headers/__clang_cuda_complex_builtins.h
+++ clang/lib/Headers/__clang_cuda_complex_builtins.h
@@ -16,7 +16,7 @@
 // to work with CUDA and OpenMP target offloading [in C and C++ mode].)
 
 #pragma push_macro("__DEVICE__")
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp declare target
 #define __DEVICE__ __attribute__((noinline, nothrow, cold, weak))
 #else
@@ -26,7 +26,7 @@
 // To make the algorithms available for C and C++ in CUDA and OpenMP we select
 // different but equivalent function versions. TODO: For OpenMP we currently
 // select the native builtins as the overload support for templates is lacking.
-#if !defined(__OPENMP_NVPTX__)
+#if !defined(_OPENMP)
 #define _ISNANd std::isnan
 #define _ISNANf std::isnan
 #define _ISINFd std::isinf
@@ -276,7 +276,7 @@
 #undef _fmaxd
 #undef _fmaxf
 
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp end declare target
 #endif
 
___
cfe-commits

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-08 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

I think the openmp_wrappers are only used when compiling device code, which 
would explain why setting a macro in one of them is a proxy for detecting 
compilation for the device.

Attempting to verify that, it looks like:
trunk-nvptx includes openmp_wrappers on device code only
trunk-amdgcn never includes openmp_wrappers
aomp-amdgcn includes openmp_wrappers on device code and cuda_wrappers on host 
code

in which case `#define __OPENMP_NVPTX` from an openmp_wrapper is equivalent to 
defining __OPENMP_NVPTX when compiling for the target and not for the host.

This seems fragile. How about we #define _OPENMP_HOST when compiling openmp for 
the host, and _OPENMP_TARGET when compiling openmp for the device? Do that from 
clang directly, not from a header which is only sometimes included. For one 
thing, we may want wrapper headers like these for the openmp host at some point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-02 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert requested changes to this revision.
jdoerfert added a comment.

We need a macro for OPENMP and one for OPENMP_OFFLOAD, we can use a single one 
for the latter and avoid _NVPTX, _AMDGCN, ... but we need both as described by 
@fodinabor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-02 Thread Joachim Meyer via Phabricator via cfe-commits 
fodinabor added a comment.

I added a pretty simple regression that should make testing this -x cuda 
-fopenmp issue simpler: https://reviews.llvm.org/D105322
I guess a similar test for -x hip -fopenmp could be added, but it hasn't been 
an issue so far as HIP and OpenMP AMDGCN seem to use the same builtins?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-01 Thread Joachim Meyer via Phabricator via cfe-commits 
fodinabor requested changes to this revision.
fodinabor added a comment.
This revision now requires changes to proceed.

citing from https://reviews.llvm.org/rG7f1e6fcff9427adfa8efa3bfeeeac801da788b87:

> Due to recent changes we cannot use OpenMP in CUDA files anymore (PR45533) as 
> the math handling of CUDA is different when _OPENMP is defined. We actually 
> want this different behavior only if we are offloading with OpenMP to NVIDIA, 
> thus generating NVPTX.

`_OPENMP` is defined even when only the CPU backend is targeted, when using 
`-fopenmp`. But then e.g. the OpenMP `__nv_isnand` variant is chosen for 
`_ISNANd` which is not defined if using CPU OpenMP and CUDA.

Applying this patch thus leads to this bunch of errors for `clang -x cuda 
-fopenmp /dev/null -o /dev/null --cuda-gpu-arch=sm_70`

  In file included from :1:
  In file included from 
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_runtime_wrapper.h:419:
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:98:7:
 error: no matching function for call to '__nv_isnand'
if (_ISNANd(__real__(z)) && _ISNANd(__imag__(z))) {
^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:66:17:
 note: expanded from macro '_ISNANd'
  #define _ISNANd __nv_isnand
  ^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_libdevice_declares.h:226:16:
 note: candidate function not viable: call to __device__ function from __host__ 
function
  __DEVICE__ int __nv_isnand(double __a);
 ^
  In file included from :1:
  In file included from 
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_runtime_wrapper.h:419:
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:98:31:
 error: no matching function for call to '__nv_isnand'
if (_ISNANd(__real__(z)) && _ISNANd(__imag__(z))) {
^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:66:17:
 note: expanded from macro '_ISNANd'
  #define _ISNANd __nv_isnand
  ^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_libdevice_declares.h:226:16:
 note: candidate function not viable: call to __device__ function from __host__ 
function
  __DEVICE__ int __nv_isnand(double __a);
 ^
  In file included from :1:
  In file included from 
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_runtime_wrapper.h:419:
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:100:9:
 error: no matching function for call to '__nv_isinfd'
  if (_ISINFd(__a) || _ISINFd(__b)) {
  ^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:68:17:
 note: expanded from macro '_ISINFd'
  #define _ISINFd __nv_isinfd
  ^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_libdevice_declares.h:224:16:
 note: candidate function not viable: call to __device__ function from __host__ 
function
  __DEVICE__ int __nv_isinfd(double __a);
 ^
  In file included from :1:
  In file included from 
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_runtime_wrapper.h:419:
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:100:25:
 error: no matching function for call to '__nv_isinfd'
  if (_ISINFd(__a) || _ISINFd(__b)) {
  ^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:68:17:
 note: expanded from macro '_ISINFd'
  #define _ISINFd __nv_isinfd
  ^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_libdevice_declares.h:224:16:
 note: candidate function not viable: call to __device__ function from __host__ 
function
  __DEVICE__ int __nv_isinfd(double __a);
 ^
  In file included from :1:
  In file included from 
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_runtime_wrapper.h:419:
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:101:24:
 error: no matching function for call to '__nv_isinfd'
__a = _COPYSIGNd(_ISINFd(__a) ? 1 : 0, __a);
 ^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_complex_builtins.h:68:17:
 note: expanded from macro '_ISINFd'
  #define _ISINFd __nv_isinfd
  ^~~
  
/home/joachim/Projekte/install/lib/clang/13.0.0/include/__clang_cuda_libdevice_declares.h:224:16:
 note: candidate function not viable: call to __device__ function from __host__ 
function
  __DEVICE__ int __nv_isinfd(double __a);
 ^
  In file included from :1:
  In file included from

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-01 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

That's interesting. I don't see how there is a semantic change here - _openmp 
is defined already and the builtins file ignores the cuda define - but I also 
haven't tried openmp+cuda in combination.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-01 Thread Joachim Meyer via Phabricator via cfe-commits 
fodinabor added a comment.

Looks pretty much like a revert of https://reviews.llvm.org/D90415 which was 
necessary to allow building with `-x cuda -fopenmp`.
Won't this break that again?

I fear there's no test covering that case and I either wasn't sure where to add 
such a test.. (also `-x hip -fopenmp`?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-01 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

Yeah, it probably should be. I should also check the blame list for the file to 
see who else should be on the reviewer list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-01 Thread Pushpinder Singh via Phabricator via cfe-commits 
pdhaliwal added a comment.

Should the name of file be changed as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-06-30 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a reviewer: ronlieb.
JonChesterfield added a comment.

this unblocks the hazard I am concerned about for D104904 
, namely it stops us defining `__CUDA__` when 
compiling amdgcn code that includes complex.h




Comment at: clang/lib/Headers/__clang_cuda_complex_builtins.h:21
 #pragma omp declare target
 #define __DEVICE__ __attribute__((noinline, nothrow, cold, weak))
 #else

bit weird that these are weak, but not changing that here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105221

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

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-06-30 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield created this revision.
JonChesterfield added reviewers: jdoerfert, tianshilei1992, pdhaliwal.
Herald added subscribers: guansong, kristof.beyls, tpr, yaxunl.
JonChesterfield requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

[openmp][nfc] Simplify macros guarding math complex headers

The __CUDA__ macro is already defined for openmp/nvptx and is not used by
__clang_cuda_complex_builtins.h, so dropping that macro slightly simplifies
nvptx and avoids defining it on amdgcn (where it is likely to be harmful).

The cuda_complex_builtins file has already been updated to work with amdgcn.
Using _OPENMP instead of __OPENMP_NVPTX__ there means one fewer macro to define
and it will work on amdgpu openmp when the rest of math is enabled.

Finally dropped a cplusplus test from a C++ header as compilation will have
failed on cmath earlier if it was included from C.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105221

Files:
  clang/lib/Headers/__clang_cuda_complex_builtins.h
  clang/lib/Headers/openmp_wrappers/complex
  clang/lib/Headers/openmp_wrappers/complex.h


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,10 +17,8 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
-#define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
-#undef __OPENMP_NVPTX__
+
 #endif
 
 // Grab the host header too.
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,18 +17,12 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
-#define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
-#undef __OPENMP_NVPTX__
 #endif
 
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +42,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
Index: clang/lib/Headers/__clang_cuda_complex_builtins.h
===
--- clang/lib/Headers/__clang_cuda_complex_builtins.h
+++ clang/lib/Headers/__clang_cuda_complex_builtins.h
@@ -16,7 +16,7 @@
 // to work with CUDA and OpenMP target offloading [in C and C++ mode].)
 
 #pragma push_macro("__DEVICE__")
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp declare target
 #define __DEVICE__ __attribute__((noinline, nothrow, cold, weak))
 #else
@@ -26,7 +26,7 @@
 // To make the algorithms available for C and C++ in CUDA and OpenMP we select
 // different but equivalent function versions. TODO: For OpenMP we currently
 // select the native builtins as the overload support for templates is lacking.
-#if !defined(__OPENMP_NVPTX__)
+#if !defined(_OPENMP)
 #define _ISNANd std::isnan
 #define _ISNANf std::isnan
 #define _ISINFd std::isinf
@@ -276,7 +276,7 @@
 #undef _fmaxd
 #undef _fmaxf
 
-#ifdef __OPENMP_NVPTX__
+#ifdef _OPENMP
 #pragma omp end declare target
 #endif
 


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,10 +17,8 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
-#define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
-#undef __OPENMP_NVPTX__
+
 #endif
 
 // Grab the host header too.
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,18 +17,12 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
-#define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
-#undef __OPENMP_NVPTX__
 #endif
 
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +42,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
Index: clang/lib/Headers/__clang_cuda_complex_builtins.h
===
--- clang/lib/Headers/__clang_cuda_complex_builtins.h
+++ clang/lib/Headers/__clang_cuda_complex_builtins.h
@@ -16,7 +16,7 @@
 // to work with CUDA and