[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
@@ -70,6 +70,9 @@ __DEVICE__ double floor(double); __DEVICE__ float floor(float); __DEVICE__ double fma(double, double, double); __DEVICE__ float fma(float, float, float); +#ifdef _MSC_VER +__DEVICE__ long double fma(long double, long double, long double); Artem-B wrote: CUDA does not provide GPU-side `fma` for long double. This declaration probably warrants a comment about this, because whoever ends up using it on a GPU will be surprised when their code compiles, but fails somewhere in ptxas, which will look like a compiler crash. https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
https://github.com/Artem-B edited https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
@@ -70,6 +70,9 @@ __DEVICE__ double floor(double);
__DEVICE__ float floor(float);
__DEVICE__ double fma(double, double, double);
__DEVICE__ float fma(float, float, float);
+#ifdef _MSC_VER
+__DEVICE__ long double fma(long double, long double, long double);
emankov wrote:
That's right, CUDA has provided `long double fma` since 10.2 but for `HOST`
only:
```cpp
inline _LIBCUDACXX_INLINE_VISIBILITY long double fma(long double __lcpp_x, long
double __lcpp_y, long double __lcpp_z) _NOEXCEPT {return ::fmal(__lcpp_x,
__lcpp_y, __lcpp_z);}
```
If a declaration of `device long double fma` is needed in order to parse it
only and with disallowing its usage on GPU, then it is better to add it to
`clang/lib/Headers/cuda_wrappers/cmath` under `_LIBCPP_STD_VER` and
`_LIBCPP_HIDE_FROM_ABI`; and it would need a corresponding `GPU` test, of
course.
https://github.com/llvm/llvm-project/pull/73756
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
@@ -70,6 +70,9 @@ __DEVICE__ double floor(double); __DEVICE__ float floor(float); __DEVICE__ double fma(double, double, double); __DEVICE__ float fma(float, float, float); +#ifdef _MSC_VER +__DEVICE__ long double fma(long double, long double, long double); Artem-B wrote: We already have a handful of MSVC-specific no-implementation functions declared here, so one more is OK. I just want to document it better. You may want to add a macro with a descriptive name. (E.g. `CUDA_ALLOW_LONG_DOUBLE_MATH_FUNCTION_DECLS`) and have it defined for MSVC, along with the comment why it's needed. Then replace the existing `#if _MSC_VER` with it. https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
@@ -70,6 +70,9 @@ __DEVICE__ double floor(double); __DEVICE__ float floor(float); __DEVICE__ double fma(double, double, double); __DEVICE__ float fma(float, float, float); +#ifdef _MSC_VER +__DEVICE__ long double fma(long double, long double, long double); emankov wrote: Yes, we have. But how do we prevent them from running on GPU? https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
@@ -70,6 +70,9 @@ __DEVICE__ double floor(double); __DEVICE__ float floor(float); __DEVICE__ double fma(double, double, double); __DEVICE__ float fma(float, float, float); +#ifdef _MSC_VER +__DEVICE__ long double fma(long double, long double, long double); Artem-B wrote: Given that there's no implementation for these functions, the reference will remain unresolved and GPU-side executable linking will fail. https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
blinkfrog wrote: I would like to share some thoughts regarding the proposed fix involving the #ifdef _MSC_VER check in the LLVM PR for the fma function ambiguity issue. I am sorry if I say something stupid. In the typical workflow with AdaptiveCpp and CUDA backend on Windows, MSVC is used only for compiling LLVM and Clang. However, the actual compilation of SYCL programs targeting the CUDA backend is performed using Clang with the AdaptiveCpp plugin. Given this workflow, the #ifdef _MSC_VER check may not be effective because _MSC_VER is specific to the MSVC compiler and is not defined when using Clang, even on Windows. Consequently, this check might not activate during our compilation process, and the ambiguity issue with the `fma` function may persist. A more suitable approach might be to use a different check that can effectively detect the Windows platform in the Clang environment, such as _WIN32, ensuring that the fix is applied in the relevant scenarios. Probably, better would be to check `_MSVC_STL_VERSION `, but this probably won't work, as standard libraries, where it is defined, are included after LLVM headers I think. I wanted to bring this to your attention to ensure that the proposed solution addresses the issue in the context of workflows similar to ours. Thank you for considering this point. https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
@@ -70,6 +70,9 @@ __DEVICE__ double floor(double); __DEVICE__ float floor(float); __DEVICE__ double fma(double, double, double); __DEVICE__ float fma(float, float, float); +#ifdef _MSC_VER +__DEVICE__ long double fma(long double, long double, long double); emankov wrote: That is it, it is only a compile-time story, and the situation when the successfully compiled code "fails somewhere in ptxas" on runtime would never happen till CUDA implementation of `long double fma` appears, or am I missing something? https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
@@ -70,6 +70,9 @@ __DEVICE__ double floor(double); __DEVICE__ float floor(float); __DEVICE__ double fma(double, double, double); __DEVICE__ float fma(float, float, float); +#ifdef _MSC_VER +__DEVICE__ long double fma(long double, long double, long double); Artem-B wrote: Correct. If someone ends up using fma(long double) in the GPU-side code, we will fail at build time, until the implementation is actually available. Failure in ptxas is not the best way to diagnose it, but it's better than failing to compile valid code because of a CUDA compatibility quirk in a standard library. https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
Artem-B wrote: I'm not familiar enough with MSVC. @rnk -- what's the best way to check for compilation with microsoft's stardard C++ library? https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
rnk wrote: > @rnk -- what's the best way to check for compilation with microsoft's > stardard C++ library? If Clang is compiling with the MSVC STL headers, it should be defining `_MSC_VER`, usually `-fms-compatibilty` has to be enabled to compile MSVC STL headers. However, I searched, and it looks like the MSVC STL defines [this macro](https://github.com/microsoft/STL/blob/main/stl/inc/yvals_core.h#L883C9-L883C27), _MSVC_STL_VERSION . I don't know when they started defining that, but it has the right meaning. https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
emankov wrote: Hello, @fodinabor and @blinkfrog > the #ifdef _MSC_VER check may not be effective because _MSC_VER is specific > to the MSVC compiler and is not defined when using Clang, even on Windows Does it mean that this fix doesn't solve https://github.com/AdaptiveCpp/AdaptiveCpp/issues/1256? Or, is [blinkfrog](https://github.com/blinkfrog) saying above about yet other cases? https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
https://github.com/fodinabor created https://github.com/llvm/llvm-project/pull/73756 As per https://github.com/AdaptiveCpp/AdaptiveCpp/issues/1256 - we are missing the `fma` long double variant for Cpp20 compact with MS-STL. See also #49853. -> I'm wondering if we should include this for all long double variants as defined in https://github.com/microsoft/STL/blob/730af178f7c79bd75d414627f969550775e14994/stl/inc/cmath#L291 ? CC @blinkfrog >From 25b80809e1875778cc968342da181c6a3b1a8304 Mon Sep 17 00:00:00 2001 From: fodinabor <5982050+fodina...@users.noreply.github.com> Date: Wed, 29 Nov 2023 07:49:09 +0100 Subject: [PATCH] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. --- clang/lib/Headers/__clang_cuda_math_forward_declares.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/lib/Headers/__clang_cuda_math_forward_declares.h b/clang/lib/Headers/__clang_cuda_math_forward_declares.h index c0f1f47cc99302a..2d1bdd0f9bb0f41 100644 --- a/clang/lib/Headers/__clang_cuda_math_forward_declares.h +++ b/clang/lib/Headers/__clang_cuda_math_forward_declares.h @@ -70,6 +70,9 @@ __DEVICE__ double floor(double); __DEVICE__ float floor(float); __DEVICE__ double fma(double, double, double); __DEVICE__ float fma(float, float, float); +#ifdef _MSC_VER +__DEVICE__ long double fma(long double, long double, long double); +#endif __DEVICE__ double fmax(double, double); __DEVICE__ float fmax(float, float); __DEVICE__ double fmin(double, double); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
llvmbot wrote: @llvm/pr-subscribers-backend-x86 Author: None (fodinabor) Changes As per https://github.com/AdaptiveCpp/AdaptiveCpp/issues/1256 - we are missing the `fma` long double variant for Cpp20 compact with MS-STL. See also #49853. -> I'm wondering if we should include this for all long double variants as defined in https://github.com/microsoft/STL/blob/730af178f7c79bd75d414627f969550775e14994/stl/inc/cmath#L291 ? CC @blinkfrog --- Full diff: https://github.com/llvm/llvm-project/pull/73756.diff 1 Files Affected: - (modified) clang/lib/Headers/__clang_cuda_math_forward_declares.h (+3) ``diff diff --git a/clang/lib/Headers/__clang_cuda_math_forward_declares.h b/clang/lib/Headers/__clang_cuda_math_forward_declares.h index c0f1f47cc99302a..2d1bdd0f9bb0f41 100644 --- a/clang/lib/Headers/__clang_cuda_math_forward_declares.h +++ b/clang/lib/Headers/__clang_cuda_math_forward_declares.h @@ -70,6 +70,9 @@ __DEVICE__ double floor(double); __DEVICE__ float floor(float); __DEVICE__ double fma(double, double, double); __DEVICE__ float fma(float, float, float); +#ifdef _MSC_VER +__DEVICE__ long double fma(long double, long double, long double); +#endif __DEVICE__ double fmax(double, double); __DEVICE__ float fmax(float, float); __DEVICE__ double fmin(double, double); `` https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA][Win32] Add `fma(long double,..)` to math forward declares. (PR #73756)
https://github.com/fodinabor edited https://github.com/llvm/llvm-project/pull/73756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
