-
Notifications
You must be signed in to change notification settings - Fork 15k
[libc++] Use libm implementations in exp for complex numbers #165457
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
base: main
Are you sure you want to change the base?
[libc++] Use libm implementations in exp for complex numbers #165457
Conversation
Replace the custom implementation with calls to compiler builtins, which delegate to libm implementations. This also improves accuracy, as demonstrated by the added test.
|
@llvm/pr-subscribers-libcxx Author: Aleksei Nurmukhametov (nurmukhametov) ChangesReplace the custom implementation with calls to compiler builtins, which delegate to libm implementations. This also improves accuracy, as demonstrated by the added test. Full diff: https://github.com/llvm/llvm-project/pull/165457.diff 2 Files Affected:
diff --git a/libcxx/include/complex b/libcxx/include/complex
index d8ec3d95c10ed..0ae2087d492f2 100644
--- a/libcxx/include/complex
+++ b/libcxx/include/complex
@@ -1074,24 +1074,13 @@ _LIBCPP_HIDE_FROM_ABI complex<_Tp> sqrt(const complex<_Tp>& __x) {
// exp
+_LIBCPP_HIDE_FROM_ABI inline _Complex float __cexp(_Complex float __v) { return __builtin_cexpf(__v); }
+_LIBCPP_HIDE_FROM_ABI inline _Complex double __cexp(_Complex double __v) { return __builtin_cexp(__v); }
+_LIBCPP_HIDE_FROM_ABI inline _Complex long double __cexp(_Complex long double __v) { return __builtin_cexpl(__v); }
+
template <class _Tp>
_LIBCPP_HIDE_FROM_ABI complex<_Tp> exp(const complex<_Tp>& __x) {
- _Tp __i = __x.imag();
- if (__i == 0) {
- return complex<_Tp>(std::exp(__x.real()), std::copysign(_Tp(0), __x.imag()));
- }
- if (std::isinf(__x.real())) {
- if (__x.real() < _Tp(0)) {
- if (!std::isfinite(__i))
- __i = _Tp(1);
- } else if (__i == 0 || !std::isfinite(__i)) {
- if (std::isinf(__i))
- __i = _Tp(NAN);
- return complex<_Tp>(__x.real(), __i);
- }
- }
- _Tp __e = std::exp(__x.real());
- return complex<_Tp>(__e * std::cos(__i), __e * std::sin(__i));
+ return complex<_Tp>(__from_builtin_tag(), std::__cexp(__x.__builtin()));
}
// pow
diff --git a/libcxx/test/libcxx/numerics/complex.number/exp.pass.cpp b/libcxx/test/libcxx/numerics/complex.number/exp.pass.cpp
new file mode 100644
index 0000000000000..ccf03cccd0a35
--- /dev/null
+++ b/libcxx/test/libcxx/numerics/complex.number/exp.pass.cpp
@@ -0,0 +1,40 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <complex>
+
+// template<class T>
+// complex<T>
+// exp(const complex<T>& x);
+
+#include <complex>
+#include <cassert>
+#include <cmath>
+
+#include "test_macros.h"
+
+template <class T>
+void test_overflow_case() {
+ typedef std::complex<T> C;
+
+ // In this case, the overflow of exp(real_part) is compensated when
+ // sin(imag_part) is close to zero, resulting in a finite imaginary part.
+ C z(T(90.0238094), T(5.900613e-39));
+ C result = std::exp(z);
+
+ assert(std::isinf(result.real()));
+ assert(result.real() > 0);
+
+ assert(std::isfinite(result.imag()));
+ assert(std::abs(result.imag() - T(7.3746)) < T(1.0));
+}
+
+int main(int, char**) {
+ test_overflow_case<float>();
+ return 0;
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| # Check for the existence of <complex.h> complex math functions. | ||
| # This is necessary even though libcxx uses the builtin versions | ||
| # of these functions, because if the builtin cannot be used, a reference | ||
| # to the library function is emitted. Using compiler builtins for these | ||
| # functions requires corresponding C99 library functions to be present. | ||
|
|
||
| cmake_push_check_state() | ||
| list(APPEND CMAKE_REQUIRED_LIBRARIES m) | ||
| check_c_source_compiles(" | ||
| #include <complex.h> | ||
| typedef __complex__ float float_type; | ||
| typedef __complex__ double double_type; | ||
| typedef __complex__ long double ld_type; | ||
| volatile float_type tmpf; | ||
| volatile double_type tmpd; | ||
| volatile ld_type tmpld; | ||
| int main(void) { | ||
| tmpf = cexpf(tmpf); | ||
| tmpd = cexp(tmpd); | ||
| tmpld = cexpl(tmpld); | ||
| return 0; | ||
| } | ||
| " C_SUPPORTS_C99_COMPLEX) | ||
| cmake_pop_check_state() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally try to avoid these kinds of shenanigans. Why is this required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required when libm is missing these functions. If they are missing, we can fall back to the old implementation, which is not using builtin_cexp functions. At least one build configuration encounters this issue: Android 5.0 x86.
Replace the custom implementation with calls to compiler builtins, which delegate to libm implementations. This also improves accuracy, as demonstrated by the added test.