[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 3 inline comments as done.
jdoerfert added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:1621
   case BuiltinType::LongDouble:
-if (getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice)
   return AuxTarget->getLongDoubleFormat();

These AuxTarget changes can be split off. I observed a segfault while testing 
stuff.



Comment at: clang/lib/Headers/CMakeLists.txt:146
   openmp_wrappers/new
 )
 

the math_declares above will provide all `C` math functions so we do not need 
`math.h` anymore here.



Comment at: clang/lib/Headers/__clang_cuda_cmath.h:306
 
+#ifndef _OPENMP
 // We need to define these overloads in exactly the namespace our standard

when this is included with OpenMP it is actually following the system `` 
and these using declarations are neither needed nor helpful (afaict)



Comment at: clang/lib/Headers/__clang_cuda_math_forward_declares.h:42-43
 __DEVICE__ long long abs(long long);
-__DEVICE__ double abs(double);
-__DEVICE__ float abs(float);
-#endif

tra wrote:
> Shouldn't float and double abs variants be preserved?
yes, thx! I make so many errors modifying these files, it is ridicules...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788



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

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 250035.
jdoerfert marked an inline comment as done.
jdoerfert added a comment.

Adjust to new scheme, tested locally with some math functions, seems to work


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Headers/openmp_wrappers/math.h
  clang/test/Headers/nvptx_device_cmath_functions.c
  clang/test/OpenMP/target_nvptx_math_complex.c
  clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
  clang/test/OpenMP/target_nvptx_math_sin.c

Index: clang/test/OpenMP/target_nvptx_math_sin.c
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_sin.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -internal-isystem %S -verify -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 -include openmp_wrappers/__clang_openmp_math_declares.h -verify -fopenmp -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -aux-triple powerpc64le-unknown-unknown -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+
+double math(float f, double d, long double ld) {
+  double r = 0;
+  r += sin(f);
+  r += sin(d);
+  return r;
+}
+
+long double foo(float f, double d, long double ld) {
+  double r = ld;
+  r += math(f, d, ld);
+#pragma omp target map(r)
+  { r += math(f, d, ld); }
+  return r;
+}
Index: clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+// TODO: How to include a "mock systme cmath" here for testing?
+
+int main() {
+  double a(0);
+  return (std::fpclassify(a) != FP_ZERO);
+}
Index: clang/test/OpenMP/target_nvptx_math_complex.c
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_complex.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-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
+// expected-no-diagnostics
+
+// CHECK-DAG: call { float, float } @__divsc3(
+// CHECK-DAG: call { float, float } @__mulsc3(
+void test_scmplx(float _Complex a) {
+#pragma omp target
+  {
+(void)(a * (a / a));
+  }
+}
+
+
+// CHECK-DAG: call { double, double } @__divdc3(
+// CHECK-DAG: call { double, double } @__muldc3(
+void test_dcmplx(double _Complex a) {
+#pragma omp target
+  {
+(void)(a * (a / a));
+  }
+}
Index: clang/test/Headers/nvptx_device_cmath_functions.c
===
--- clang/test/Headers/nvptx_device_cmath_functions.c
+++ clang/test/Headers/nvptx_device_cmath_functions.c
@@ -4,7 +4,7 @@
 // REQUIRES: nvptx-registered-target
 
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -include cmath -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_math_declares.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -include cmath -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 -check-prefix CHECK-YES %s
+// RUN: %clang_cc1 -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 -check-prefix CHECK-YES %s
 
 #include 
 
Index: clang/lib/Headers/openmp_wrappers/math.h
===
--- clang/lib/Headers/openmp_wrappers/math.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*===- math.h - Alternative math.h header --===
- *
- 

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Couple of nits below. LGTM for CUDA headers otherwise.




Comment at: clang/lib/Headers/__clang_cuda_math_forward_declares.h:42-43
 __DEVICE__ long long abs(long long);
-__DEVICE__ double abs(double);
-__DEVICE__ float abs(float);
-#endif

Shouldn't float and double abs variants be preserved?



Comment at: clang/lib/Headers/cuda_wrappers/new:107
+#pragma omp end declare variant
+#endif
+

You may want to push/pop `__DEVICE__` macro here and in other headers where 
it's not done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788



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

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 248937.
jdoerfert added a comment.

Add complex test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_libdevice_declares.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Headers/cuda_wrappers/new
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Headers/openmp_wrappers/math.h
  clang/test/OpenMP/target_nvptx_math_complex.c
  clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
  clang/test/OpenMP/target_nvptx_math_sin.c

Index: clang/test/OpenMP/target_nvptx_math_sin.c
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_sin.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+// TODO: How to include a "mock systme cmath" here for testing?
+
+double math(short s, int i, float f, double d, double ld) {
+  double r = 0;
+  r += sin(s);
+  r += sin(i);
+  r += sin(f);
+  r += sin(d);
+  return r;
+}
+
+long double foo(short s, int i, float f, double d, long double ld) {
+  double r = sin(ld);
+  r += math(s, i, f, d, ld);
+#pragma omp target map(r)
+  { r += math(s, i, f, d, ld); }
+  return r;
+}
Index: clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+// TODO: How to include a "mock systme cmath" here for testing?
+
+int main() {
+  double a(0);
+  return (std::fpclassify(a) != FP_ZERO);
+}
Index: clang/test/OpenMP/target_nvptx_math_complex.c
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_complex.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-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
+// expected-no-diagnostics
+
+// CHECK-DAG: call { float, float } @__divsc3(
+// CHECK-DAG: call { float, float } @__mulsc3(
+void test_scmplx(float _Complex a) {
+#pragma omp target
+  {
+(void)(a * (a / a));
+  }
+}
+
+
+// CHECK-DAG: call { double, double } @__divdc3(
+// CHECK-DAG: call { double, double } @__muldc3(
+void test_dcmplx(double _Complex a) {
+#pragma omp target
+  {
+(void)(a * (a / a));
+  }
+}
Index: clang/lib/Headers/openmp_wrappers/math.h
===
--- clang/lib/Headers/openmp_wrappers/math.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*===- math.h - Alternative math.h header --===
- *
- * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
- * See https://llvm.org/LICENSE.txt for license information.
- * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
- *
- *===---===
- */
-
-#include <__clang_openmp_math.h>
-
-#ifndef __CLANG_NO_HOST_MATH__
-#include_next 
-#else
-#undef __CLANG_NO_HOST_MATH__
-#endif
-
Index: clang/lib/Headers/openmp_wrappers/cmath
===
--- clang/lib/Headers/openmp_wrappers/cmath
+++ /dev/null
@@ -1,16 +0,0 @@
-/*===-- cmath - Alternative cmath header ---===
- *
- * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
- * See https://llvm.org/LICENSE.txt for license information.
- * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
- *
- *===---===
- */
-
-#include <__clang_openmp_math.h>
-
-#ifndef __CLANG_NO_HOST_MATH__
-#include_next 
-#else
-#undef __CLANG_NO_HOST_MATH__
-#endif
Index: clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
===
--- clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
+++ /dev/null
@@ -1,33 +0,0 

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked an inline comment as done.
jdoerfert added a comment.

In D75788#1910743 , @JonChesterfield 
wrote:

> That's less invasive than I feared. Nicely done.


We need to run some more tests to make sure it works as expected but I hope we 
can completely piggy back on the underlying "language" support.

> It may worth keeping the openmp header wrapper to do architecture dispatch. 
> Something like:

We can do that or adjust the pipeline based on the target, either is fine with 
me.




Comment at: clang/lib/Headers/cuda_wrappers/new:36
 
+#ifdef _OPENMP
+#define __DEVICE__

JonChesterfield wrote:
> macros look off here - should it be `#define DEVICE`, or the following uses 
> `__DEVICE__`?
Yes.

Furthermore I think I want to introduce the effect of `__device__` as an 
attribute, basically `match(device={arch(nvptx)}` on a single function. That 
would make the declare variant go away and allow us to piggy back on the 
`__DEVICE__` directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788



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

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

That's less invasive than I feared. Nicely done.

It may worth keeping the openmp header wrapper to do architecture dispatch. 
Something like:

  #ifndef __CLANG_OPENMP_MATH_DECLARES_H__
  #define __CLANG_OPENMP_MATH_DECLARES_H__
  
  #ifndef _OPENMP
  #error "This file is for OpenMP compilation only."
  #endif
  
  #if defined(__AMDGCN__)
  #pragma omp begin declare variant match(device = {arch(amdgcn)})
  #include "equivalent_header.h"
  #pragma omp end declare variant
  #endif // __AMDGCN__
  
  #if defined(__NVPTX__)
  #define __CUDA__
  #pragma omp begin declare variant match(device = {arch(nvptx)})
  
  #if defined(__cplusplus)
  #include <__clang_cuda_math_forward_declares.h>
  #endif
  
  /// Include declarations for libdevice functions.
  #include <__clang_cuda_libdevice_declares.h>
  /// Provide definitions for these functions.
  #include <__clang_cuda_device_functions.h>
  
  #pragma omp end declare variant
  #undef __CUDA__
  #endif // __NVPTX__
  
  
  #endif // __CLANG_OPENMP_MATH_DECLARES_H__




Comment at: clang/lib/Headers/cuda_wrappers/new:36
 
+#ifdef _OPENMP
+#define __DEVICE__

macros look off here - should it be `#define DEVICE`, or the following uses 
`__DEVICE__`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788



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

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, 
gtbercea, grokos, sdmitriev, JonChesterfield, hfinkel, fghanim, aaron.ballman.
Herald added subscribers: guansong, bollu, mgorny.
Herald added a project: clang.

This is a WIP patch to show one way of implementing math support (and
support for `new`, `algorithm`, ...) in OpenMP target regions by piggy
backing on the CUDA implementation. All CUDA "code" should be in
`omp begin/end declare variant match(device={arch(nvptx)})` at the end
of the day while the "host" code is not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75788

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_libdevice_declares.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Headers/cuda_wrappers/new
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Headers/openmp_wrappers/math.h
  clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
  clang/test/OpenMP/target_nvptx_math_sin.c

Index: clang/test/OpenMP/target_nvptx_math_sin.c
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_sin.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+// TODO: How to include a "mock systme cmath" here for testing?
+
+double math(short s, int i, float f, double d, double ld) {
+  double r = 0;
+  r += sin(s);
+  r += sin(i);
+  r += sin(f);
+  r += sin(d);
+  return r;
+}
+
+long double foo(short s, int i, float f, double d, long double ld) {
+  double r = sin(ld);
+  r += math(s, i, f, d, ld);
+#pragma omp target map(r)
+  { r += math(s, i, f, d, ld); }
+  return r;
+}
Index: clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+// TODO: How to include a "mock systme cmath" here for testing?
+
+int main() {
+  double a(0);
+  return (std::fpclassify(a) != FP_ZERO);
+}
Index: clang/lib/Headers/openmp_wrappers/math.h
===
--- clang/lib/Headers/openmp_wrappers/math.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*===- math.h - Alternative math.h header --===
- *
- * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
- * See https://llvm.org/LICENSE.txt for license information.
- * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
- *
- *===---===
- */
-
-#include <__clang_openmp_math.h>
-
-#ifndef __CLANG_NO_HOST_MATH__
-#include_next 
-#else
-#undef __CLANG_NO_HOST_MATH__
-#endif
-
Index: clang/lib/Headers/openmp_wrappers/cmath
===
--- clang/lib/Headers/openmp_wrappers/cmath
+++ /dev/null
@@ -1,16 +0,0 @@
-/*===-- cmath - Alternative cmath header ---===
- *
- * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
- * See https://llvm.org/LICENSE.txt for license information.
- * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
- *
- *===---===
- */
-
-#include <__clang_openmp_math.h>
-
-#ifndef __CLANG_NO_HOST_MATH__
-#include_next 
-#else
-#undef __CLANG_NO_HOST_MATH__
-#endif
Index: clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
===
--- clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*=== __clang_openmp_math_declares.h - OpenMP math declares ===
- *
- * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
- * See https://llvm.org/LICENSE.txt for license information.
- * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
- *
- *===---===
- */
-
-#ifndef __CLANG_OPENMP_MATH_DECLARES_H__
-#define __CLANG_OPENMP_MATH_DECLARES_H__
-
-#ifndef _OPENMP
-#error "This file is for OpenMP compilation only."
-#endif
-
-#if defined(__NVPTX__) && defined(_OPENMP)
-
-#define __CUDA__