[PATCH] D61399: [OpenMP][Clang] Support for target math functions

This revision was automatically updated to reflect the committed changes. Closed by commit rC360265: [OpenMP][Clang] Support for target math functions (authored by gbercea, committed by ). Changed prior to commit: https://reviews.llvm.org/D61399?vs=198664=198677#toc Repository: rC Clang

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea updated this revision to Diff 198664. gtbercea added a comment. - Eliminate declarations of functions not needed for math function resolution. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 Files:

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea updated this revision to Diff 198578. gtbercea added a comment. - Fix move to openmp_wrapper folder. Fix header ordering problem. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 Files: lib/Driver/ToolChain.cpp

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

JDevlieghere added inline comments. Comment at: lib/Headers/CMakeLists.txt:36 bmiintrin.h + openmp_wrappers/math.h + openmp_wrappers/cmath hfinkel wrote: > JDevlieghere wrote: > > This doesn't do what you think it would do. The files are copied into the >

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

hfinkel added inline comments. Comment at: lib/Headers/CMakeLists.txt:36 bmiintrin.h + openmp_wrappers/math.h + openmp_wrappers/cmath JDevlieghere wrote: > This doesn't do what you think it would do. The files are copied into the > root of the resource

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

JDevlieghere added a comment. I've reverted this in rL360192 because it breaks stage 2 builds on GreenDragon. Please see the commit message and the inline comment for more details. Comment at: lib/Headers/CMakeLists.txt:36 bmiintrin.h +

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

This revision was automatically updated to reflect the committed changes. Closed by commit rC360063: [OpenMP][Clang] Support for target math functions (authored by gbercea, committed by ). Changed prior to commit: https://reviews.llvm.org/D61399?vs=198311=198314#toc Repository: rC Clang

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea updated this revision to Diff 198311. gtbercea added a comment. - Address comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 Files: lib/Driver/ToolChains/Clang.cpp lib/Headers/CMakeLists.txt

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG with a nit. Comment at: lib/Headers/__clang_cuda_cmath.h:444 using ::roundf; +#ifndef _OPENMP using ::scalblnf; gtbercea wrote: > ABataev wrote: > >

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/Headers/__clang_cuda_cmath.h:444 using ::roundf; +#ifndef _OPENMP using ::scalblnf; ABataev wrote: > I see that the same guard is used >

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added inline comments. Comment at: lib/Headers/__clang_cuda_cmath.h:444 using ::roundf; +#ifndef _OPENMP using ::scalblnf; I see that the same guard is used `lib/Headers/__clang_cuda_device_functions.h`, but for different set of functions. Is this

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea updated this revision to Diff 198301. gtbercea added a comment. - Address comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 Files: lib/Driver/ToolChains/Clang.cpp lib/Headers/CMakeLists.txt

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added inline comments. Comment at: lib/Headers/__clang_cuda_cmath.h:54 __DEVICE__ float fmod(float __x, float __y) { return ::fmodf(__x, __y); } +#ifndef _OPENMP __DEVICE__ int fpclassify(float __x) { gtbercea wrote: > ABataev wrote: > > Why we have

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea marked 2 inline comments as done. gtbercea added inline comments. Comment at: lib/Headers/__clang_cuda_cmath.h:54 __DEVICE__ float fmod(float __x, float __y) { return ::fmodf(__x, __y); } +#ifndef _OPENMP __DEVICE__ int fpclassify(float __x) { ABataev

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added inline comments. Comment at: lib/Headers/__clang_cuda_cmath.h:54 __DEVICE__ float fmod(float __x, float __y) { return ::fmodf(__x, __y); } +#ifndef _OPENMP __DEVICE__ int fpclassify(float __x) { Why we have this guard here? It does not work for

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

jdoerfert added a comment. Alexey, is this is good to go now? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea updated this revision to Diff 198108. gtbercea added a comment. - Add new tests. Add stub headers. - Remove old tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 Files: lib/Driver/ToolChains/Clang.cpp

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea updated this revision to Diff 198105. gtbercea added a comment. - Add driver test. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 Files: lib/Driver/ToolChains/Clang.cpp lib/Headers/CMakeLists.txt

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

hfinkel added a comment. > It is up to you. I don't have strong objections if you think this will work > as required. Just the tests must be fixed, especially codegen tests. Thanks, Alexey. I think this will work as required, and then we'll be able to update it when we get declare variant.

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added a comment. In D61399#1490189 , @hfinkel wrote: > > Still, I think we need to prvide the default implementation of those > > non-standard functions (they can be very simple, maybe reporting error is > > going to be enough), which can be

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

hfinkel added a comment. > Still, I think we need to prvide the default implementation of those > non-standard functions (they can be very simple, maybe reporting error is > going to be enough), which can be overriden by user. I appreciate your motivation, and I agree with you to some extent.

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea added a comment. In D61399#1489762 , @ABataev wrote: > In D61399#1489757 , @gtbercea wrote: > > > @ABataev this patch works for both C and C++ and for both math.h and cmath > > headers. > > > Did you test

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added a comment. In D61399#1489757 , @gtbercea wrote: > @ABataev this patch works for both C and C++ and for both math.h and cmath > headers. Did you test it for the builtins? like `pow`, `powf` and `powl`? How are the builtins resolved with

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea added a comment. @ABataev this patch works for both C and C++ and for both math.h and cmath headers. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 ___ cfe-commits

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added a comment. In D61399#1488897 , @hfinkel wrote: > In D61399#1488366 , @ABataev wrote: > > > In D61399#1488329 , @hfinkel wrote: > > > > > In D61399#1488309

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea updated this revision to Diff 197911. gtbercea added a comment. - Address comments. - Add math and cmath inclusion tests. - Add driver test. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 Files:

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

hfinkel added a comment. In D61399#1488366 , @ABataev wrote: > In D61399#1488329 , @hfinkel wrote: > > > In D61399#1488309 , @ABataev wrote: > > > > > In D61399#1488299

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added a comment. In D61399#1488329 , @hfinkel wrote: > In D61399#1488309 , @ABataev wrote: > > > In D61399#1488299 , @hfinkel wrote: > > > > > In D61399#1488262

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

hfinkel added a comment. In D61399#1488309 , @ABataev wrote: > In D61399#1488299 , @hfinkel wrote: > > > In D61399#1488262 , @ABataev wrote: > > > > > I don't like this

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added a comment. In D61399#1488299 , @hfinkel wrote: > In D61399#1488262 , @ABataev wrote: > > > I don't like this implementation. Seems to me, it breaks one of the OpenMP > > standard requirements: the

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

hfinkel added a comment. In D61399#1488262 , @ABataev wrote: > I don't like this implementation. Seems to me, it breaks one of the OpenMP > standard requirements: the program can be compiled without openmp support. I > assume, that with this includes

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added a comment. Moreover, I think this will cause troubles even in simple cases. Assume we have `target if(cond)` construct. In this case we will need to compile the target region for both, the device and the host. If the target region uses some device-specific math functions, it will

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

ABataev added a comment. I don't like this implementation. Seems to me, it breaks one of the OpenMP standard requirements: the program can be compiled without openmp support. I assume, that with this includes the program won't be able to be compiled without OpenMP support anymore because it

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

hfinkel accepted this revision. hfinkel added inline comments. This revision is now accepted and ready to land. Comment at: lib/Driver/ToolChains/Clang.cpp:1163 + llvm::sys::path::append(P, "openmp_wrappers"); + CmdArgs.push_back("-internal-isystem"); +

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea updated this revision to Diff 197798. gtbercea added a comment. - Clean-up. Add header. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 Files: lib/Driver/ToolChains/Clang.cpp lib/Headers/CMakeLists.txt

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

jdoerfert added inline comments. Comment at: lib/Headers/openmp_wrappers/__clang_openmp_math.h:29 + +#define __forceinline__ __attribute__((always_inline)) + I think this is a leftover we forgot to remove. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

jdoerfert added a comment. For the record, this is an implementation of the scheme proposed in https://reviews.llvm.org/D60907#1484756. There are drawbacks, see the TODO, but it will give most people a short term solution until we get OpenMP 5.0 variants. Finally, there is a remote chance this

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea updated this revision to Diff 197638. gtbercea edited the summary of this revision. gtbercea added a comment. - Minor fixes. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61399/new/ https://reviews.llvm.org/D61399 Files:

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

gtbercea created this revision. gtbercea added reviewers: hfinkel, caomhin, ABataev, tra. Herald added subscribers: cfe-commits, guansong, mgorny. Herald added a project: clang. In this patch we propose a temporary solution to resolving math functions for the NVPTX toolchain, temporary until