Skip to content

[CUDA] Remove obsolete GPU-side __constexpr_* wrappers. #139164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 12, 2025

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