Skip to content

Conversation

@Artem-B
Copy link
Member

@Artem-B Artem-B commented May 8, 2025

No description provided.

@Artem-B Artem-B requested a review from ldionne May 8, 2025 21:45
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels May 8, 2025
@llvmbot
Copy link
Member

llvmbot commented May 8, 2025

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-clang

Author: Artem Belevich (Artem-B)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/139164.diff

1 Files Affected:

  • (modified) clang/lib/Headers/cuda_wrappers/cmath (+2-2)
diff --git a/clang/lib/Headers/cuda_wrappers/cmath b/clang/lib/Headers/cuda_wrappers/cmath
index 7deca678bf252..3e8820a173087 100644
--- a/clang/lib/Headers/cuda_wrappers/cmath
+++ b/clang/lib/Headers/cuda_wrappers/cmath
@@ -63,9 +63,9 @@ __constexpr_fmax(long double __x, long double __y) _NOEXCEPT {
 
 template <class _Tp, class _Up, __enable_if_t<is_arithmetic<_Tp>::value && is_arithmetic<_Up>::value, int> = 0>
 __attribute__((device))
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 typename __promote<_Tp, _Up>::type
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __promote_t<_Tp, _Up>
 __constexpr_fmax(_Tp __x, _Up __y) _NOEXCEPT {
-  using __result_type = typename __promote<_Tp, _Up>::type;
+  using __result_type = __promote_t<_Tp, _Up>;
   return std::__constexpr_fmax(static_cast<__result_type>(__x), static_cast<__result_type>(__y));
 }
 #endif // _LIBCPP_STD_VER < 14

@Artem-B Artem-B requested a review from jhuber6 May 12, 2025 18:14
Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now this checks for libc++ less than 14. Is that still relevant following that change?

@Artem-B
Copy link
Member Author

Artem-B commented May 12, 2025

@jhuber6 @ldionne One concern I have for this change is that it will break folks who will use older libc++ with the new Clang + wrapper headers.

Is older libc++ expected to work with non-matching clang version? If the expectation is that libc++ and clang are from the same version, then adapting the wrappers to match the current libc++ is sufficient, and there's no need to provide extra tweaks for backward compatibility.

I guess I can mitigate it to some degree by keeping the old variant of the change with #if _LIBCPP_VERSION < 210000, but I wonder if I need to do that at all.

@Artem-B
Copy link
Member Author

Artem-B commented May 12, 2025

Right now this checks for libc++ less than 14. Is that still relevant following that change?

That's a very good point. Looks like those __constexpr_fmin/fmax are gone now and we do not heed GPU-side wrappers for them any more.

libc++ no longer has them, so there's nothing to wrap.
@Artem-B Artem-B changed the title [CUDA] fix wrapper cmath header to match #136101 [CUDA] Remove obsolete GPU-side __constexpr_* wrappers. May 12, 2025
@Artem-B
Copy link
Member Author

Artem-B commented May 12, 2025

No wrappers -- no problems. :-)

@Artem-B Artem-B merged commit 8d7b35e into llvm:main May 12, 2025
9 of 10 checks passed
@Artem-B Artem-B deleted the cuda-promote branch May 12, 2025 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants