[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

This revision was automatically updated to reflect the committed changes. Closed by commit rGd999cbc98832: [OpenMP] Initial support for std::complex in target regions (authored by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D80897?vs=276053=276584#toc Repository: rG LLVM

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

tra accepted this revision. tra added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80897/new/ https://reviews.llvm.org/D80897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert updated this revision to Diff 276053. jdoerfert marked an inline comment as done. jdoerfert added a comment. Addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80897/new/ https://reviews.llvm.org/D80897 Files:

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert added a comment. @JonChesterfield @hfinkel @tra ping I would really like to land this before the release branches off to allow people to use complex in target regions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80897/new/

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. I think this change is good. The library story is a bit difficult, but fundamentally openmp needs a shim of some sort to map target math functions onto the libm of the

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert added a comment. @tra After chatting with @hfinkel I know now why we don't see the calls in the libc++ case. libc++ implements std::complex without `_Complex` types, stdlib++ does. If the user uses `_Complex` directly we need these functions for sure as the standard defines them (I

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/lib/Headers/__clang_cuda_complex_builtins.h:42 +#define _LOGBf _LOGBd +#else +#define _ISNANd isnan This will actually not work right now as we do not overload

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert marked an inline comment as done. jdoerfert added a comment. I tried to determine why we don't emit such calls for c++11 and stdc++ but I was not successful :( Tracking back from the emission lead to the generic expression codegen without any (obvious) check of the runtime library or

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

tra added a comment. In D80897#2066952 , @jdoerfert wrote: > In D80897#2066723 , @tra wrote: > > > Hmm. I'm pretty sure tensorflow is using std::complex for various types. > > I'm surprised that we haven't seen

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

arsenm added inline comments. Comment at: clang/lib/Headers/__clang_cuda_complex_builtins.h:136-137 + __d = _COPYSIGNf(_ISINFf(__d) ? 1 : 0, __d); + if (_ISNANf(__a)) +__a = _COPYSIGNf(0, __a); + if (_ISNANf(__b)) Why does this try to

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert marked 2 inline comments as done. jdoerfert added a comment. In D80897#2066723 , @tra wrote: > Hmm. I'm pretty sure tensorflow is using std::complex for various types. I'm > surprised that we haven't seen these functions missing. Which

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

tra added a comment. Hmm. I'm pretty sure tensorflow is using std::complex for various types. I'm surprised that we haven't seen these functions missing. Plain CUDA (e.g. https://godbolt.org/z/Us6oXC) code appears to have no references to `__mul*` or `__div*`, at least for optimized builds, but

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert updated this revision to Diff 267531. jdoerfert added a comment. Fix tests, add C support Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80897/new/ https://reviews.llvm.org/D80897 Files: clang/lib/Headers/CMakeLists.txt

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert created this revision. jdoerfert added reviewers: tra, hfinkel, ABataev, JonChesterfield. Herald added subscribers: sstefan1, guansong, bollu, yaxunl, mgorny. Herald added a project: clang. This simply follows the scheme we have for other wrappers. It resolves the current link problem,