[PATCH] D77774: [OpenMP] Allow to go first in C++-mode in target regions

2020-04-09 Thread Johannes Doerfert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG17d83342235f: [OpenMP] Allow math.h to go first in 
C++-mode in target regions (authored by jdoerfert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D4

Files:
  clang/lib/Headers/openmp_wrappers/math.h
  clang/test/Headers/Inputs/include/math.h
  clang/test/Headers/nvptx_device_math_sincos.cpp


Index: clang/test/Headers/nvptx_device_math_sincos.cpp
===
--- clang/test/Headers/nvptx_device_math_sincos.cpp
+++ clang/test/Headers/nvptx_device_math_sincos.cpp
@@ -1,8 +1,13 @@
 // REQUIRES: nvptx-registered-target
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -fopenmp -triple 
powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc 
%s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers 
-include __clang_openmp_device_functions.h -internal-isystem %S/Inputs/include 
-fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers  
   -include __clang_openmp_device_functions.h -internal-isystem 
%S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple 
powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers 
-DCMATH -include __clang_openmp_device_functions.h -internal-isystem 
%S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple 
powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
 
+#ifdef CMATH
 #include 
+#else
+#include 
+#endif
 
 // 4 calls to sincos(f), all translated to __nv_sincos calls:
 
Index: clang/test/Headers/Inputs/include/math.h
===
--- clang/test/Headers/Inputs/include/math.h
+++ clang/test/Headers/Inputs/include/math.h
@@ -197,3 +197,7 @@
  * math functions.
  */
 #define HUGE_VAL (__builtin_huge_val())
+
+#ifdef __cplusplus
+#include 
+#endif
Index: clang/lib/Headers/openmp_wrappers/math.h
===
--- clang/lib/Headers/openmp_wrappers/math.h
+++ clang/lib/Headers/openmp_wrappers/math.h
@@ -7,6 +7,19 @@
  *===---===
  */
 
+// If we are in C++ mode and include  (not ) first, we still 
need
+// to make sure  is read first. The problem otherwise is that we haven't
+// seen the declarations of the math.h functions when the system math.h 
includes
+// our cmath overlay. However, our cmath overlay, or better the underlying
+// overlay, e.g. CUDA, uses the math.h functions. Since we haven't declared 
them
+// yet we get errors. CUDA avoids this by eagerly declaring all math functions
+// (in the __device__ space) but we cannot do this. Instead we break the
+// dependence by forcing cmath to go first. While our cmath will in turn 
include
+// this file, the cmath guards will prevent recursion.
+#ifdef __cplusplus
+#include 
+#endif
+
 #ifndef __CLANG_OPENMP_MATH_H__
 #define __CLANG_OPENMP_MATH_H__
 


Index: clang/test/Headers/nvptx_device_math_sincos.cpp
===
--- clang/test/Headers/nvptx_device_math_sincos.cpp
+++ clang/test/Headers/nvptx_device_math_sincos.cpp
@@ -1,8 +1,13 @@
 // REQUIRES: nvptx-registered-target
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -DCMATH -include __clang_openmp_device_functions.h -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda 

[PATCH] D77774: [OpenMP] Allow to go first in C++-mode in target regions

2020-04-09 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

The cmath/math.h story makes me sad, but this is a good workaround. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D4



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


[PATCH] D77774: [OpenMP] Allow to go first in C++-mode in target regions

2020-04-08 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert created this revision.
jdoerfert added a reviewer: hfinkel.
Herald added subscribers: guansong, bollu, yaxunl.
Herald added a project: clang.

If we are in C++ mode and include  (not ) first, we still
need to make sure  is read first. The problem otherwise is that
we haven't seen the declarations of the math.h functions when the system
math.h includes our cmath overlay. However, our cmath overlay, or better
the underlying overlay, e.g. CUDA, uses the math.h functions. Since we
haven't declared them yet we get errors. CUDA avoids this by eagerly
declaring all math functions (in the __device__ space) but we cannot do
this. Instead we break the dependence by forcing cmath to go first.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D4

Files:
  clang/lib/Headers/openmp_wrappers/math.h
  clang/test/Headers/Inputs/include/math.h
  clang/test/Headers/nvptx_device_math_sincos.cpp


Index: clang/test/Headers/nvptx_device_math_sincos.cpp
===
--- clang/test/Headers/nvptx_device_math_sincos.cpp
+++ clang/test/Headers/nvptx_device_math_sincos.cpp
@@ -1,8 +1,13 @@
 // REQUIRES: nvptx-registered-target
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -fopenmp -triple 
powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc 
%s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers 
-include __clang_openmp_device_functions.h -internal-isystem %S/Inputs/include 
-fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers  
   -include __clang_openmp_device_functions.h -internal-isystem 
%S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple 
powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers 
-DCMATH -include __clang_openmp_device_functions.h -internal-isystem 
%S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple 
powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
 
+#ifdef CMATH
 #include 
+#else
+#include 
+#endif
 
 // 4 calls to sincos(f), all translated to __nv_sincos calls:
 
Index: clang/test/Headers/Inputs/include/math.h
===
--- clang/test/Headers/Inputs/include/math.h
+++ clang/test/Headers/Inputs/include/math.h
@@ -197,3 +197,7 @@
  * math functions.
  */
 #define HUGE_VAL (__builtin_huge_val())
+
+#ifdef __cplusplus
+#include 
+#endif
Index: clang/lib/Headers/openmp_wrappers/math.h
===
--- clang/lib/Headers/openmp_wrappers/math.h
+++ clang/lib/Headers/openmp_wrappers/math.h
@@ -7,6 +7,19 @@
  *===---===
  */
 
+// If we are in C++ mode and include  (not ) first, we still 
need
+// to make sure  is read first. The problem otherwise is that we haven't
+// seen the declarations of the math.h functions when the system math.h 
includes
+// our cmath overlay. However, our cmath overlay, or better the underlying
+// overlay, e.g. CUDA, uses the math.h functions. Since we haven't declared 
them
+// yet we get errors. CUDA avoids this by eagerly declaring all math functions
+// (in the __device__ space) but we cannot do this. Instead we break the
+// dependence by forcing cmath to go first. While our cmath will in turn 
include
+// this file, the cmath guards will prevent recursion.
+#ifdef __cplusplus
+#include 
+#endif
+
 #ifndef __CLANG_OPENMP_MATH_H__
 #define __CLANG_OPENMP_MATH_H__
 


Index: clang/test/Headers/nvptx_device_math_sincos.cpp
===
--- clang/test/Headers/nvptx_device_math_sincos.cpp
+++ clang/test/Headers/nvptx_device_math_sincos.cpp
@@ -1,8 +1,13 @@
 // REQUIRES: nvptx-registered-target
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h