Skip to content

Commit 2811b7d

Browse files
hctimcopybara-github
authored andcommitted
Revert "[libc++][math] Fix undue overflowing of std::hypot(x,y,z) (#93350)"
This reverts commit 9628777479a970db5d0c2d0b456dac6633864760. More details in llvm/llvm-project#93350, but this broke the PowerPC sanitizer bots. NOKEYCHECK=True GitOrigin-RevId: 1031335f2ee1879737576fde3a3425ce0046e773
1 parent ee093ca commit 2811b7d

File tree

8 files changed

+65
-197
lines changed

8 files changed

+65
-197
lines changed

include/__math/hypot.h

-89
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,10 @@
1515
#include <__type_traits/is_same.h>
1616
#include <__type_traits/promote.h>
1717

18-
#if _LIBCPP_STD_VER >= 17
19-
# include <__algorithm/max.h>
20-
# include <__math/abs.h>
21-
# include <__math/roots.h>
22-
# include <__utility/pair.h>
23-
# include <limits>
24-
#endif
25-
2618
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
2719
# pragma GCC system_header
2820
#endif
2921

30-
_LIBCPP_PUSH_MACROS
31-
#include <__undef_macros>
32-
3322
_LIBCPP_BEGIN_NAMESPACE_STD
3423

3524
namespace __math {
@@ -52,86 +41,8 @@ inline _LIBCPP_HIDE_FROM_ABI typename __promote<_A1, _A2>::type hypot(_A1 __x, _
5241
return __math::hypot((__result_type)__x, (__result_type)__y);
5342
}
5443

55-
#if _LIBCPP_STD_VER >= 17
56-
// Factors needed to determine if over-/underflow might happen for `std::hypot(x,y,z)`.
57-
// returns [overflow_threshold, overflow_scale]
58-
template <class _Real>
59-
_LIBCPP_HIDE_FROM_ABI std::pair<_Real, _Real> __hypot_factors() {
60-
static_assert(std::numeric_limits<_Real>::is_iec559);
61-
62-
if constexpr (std::is_same_v<_Real, float>) {
63-
static_assert(-125 == std::numeric_limits<_Real>::min_exponent);
64-
static_assert(+128 == std::numeric_limits<_Real>::max_exponent);
65-
return {0x1.0p+62f, 0x1.0p-70f};
66-
} else if constexpr (std::is_same_v<_Real, double>) {
67-
static_assert(-1021 == std::numeric_limits<_Real>::min_exponent);
68-
static_assert(+1024 == std::numeric_limits<_Real>::max_exponent);
69-
return {0x1.0p+510, 0x1.0p-600};
70-
} else { // long double
71-
static_assert(std::is_same_v<_Real, long double>);
72-
73-
// preprocessor guard necessary, otherwise literals (e.g. `0x1.0p+8'190l`) throw warnings even when shielded by `if
74-
// constexpr`
75-
# if __DBL_MAX_EXP__ == __LDBL_MAX_EXP__
76-
static_assert(sizeof(_Real) == sizeof(double));
77-
return static_cast<std::pair<_Real, _Real>>(__math::__hypot_factors<double>());
78-
# else
79-
static_assert(sizeof(_Real) > sizeof(double));
80-
static_assert(-16381 == std::numeric_limits<_Real>::min_exponent);
81-
static_assert(+16384 == std::numeric_limits<_Real>::max_exponent);
82-
return {0x1.0p+8190l, 0x1.0p-9000l};
83-
# endif
84-
}
85-
}
86-
87-
// Computes the three-dimensional hypotenuse: `std::hypot(x,y,z)`.
88-
// The naive implementation might over-/underflow which is why this implementation is more involved:
89-
// If the square of an argument might run into issues, we scale the arguments appropriately.
90-
// See https://github.com/llvm/llvm-project/issues/92782 for a detailed discussion and summary.
91-
template <class _Real>
92-
_LIBCPP_HIDE_FROM_ABI _Real __hypot(_Real __x, _Real __y, _Real __z) {
93-
const _Real __max_abs = std::max(__math::fabs(__x), std::max(__math::fabs(__y), __math::fabs(__z)));
94-
const auto [__overflow_threshold, __overflow_scale] = __math::__hypot_factors<_Real>();
95-
_Real __scale;
96-
if (__max_abs > __overflow_threshold) { // x*x + y*y + z*z might overflow
97-
__scale = __overflow_scale;
98-
__x *= __scale;
99-
__y *= __scale;
100-
__z *= __scale;
101-
} else if (__max_abs < 1 / __overflow_threshold) { // x*x + y*y + z*z might underflow
102-
__scale = 1 / __overflow_scale;
103-
__x *= __scale;
104-
__y *= __scale;
105-
__z *= __scale;
106-
} else
107-
__scale = 1;
108-
return __math::sqrt(__x * __x + __y * __y + __z * __z) / __scale;
109-
}
110-
111-
inline _LIBCPP_HIDE_FROM_ABI float hypot(float __x, float __y, float __z) { return __math::__hypot(__x, __y, __z); }
112-
113-
inline _LIBCPP_HIDE_FROM_ABI double hypot(double __x, double __y, double __z) { return __math::__hypot(__x, __y, __z); }
114-
115-
inline _LIBCPP_HIDE_FROM_ABI long double hypot(long double __x, long double __y, long double __z) {
116-
return __math::__hypot(__x, __y, __z);
117-
}
118-
119-
template <class _A1,
120-
class _A2,
121-
class _A3,
122-
std::enable_if_t< is_arithmetic_v<_A1> && is_arithmetic_v<_A2> && is_arithmetic_v<_A3>, int> = 0 >
123-
_LIBCPP_HIDE_FROM_ABI typename __promote<_A1, _A2, _A3>::type hypot(_A1 __x, _A2 __y, _A3 __z) _NOEXCEPT {
124-
using __result_type = typename __promote<_A1, _A2, _A3>::type;
125-
static_assert(!(
126-
std::is_same_v<_A1, __result_type> && std::is_same_v<_A2, __result_type> && std::is_same_v<_A3, __result_type>));
127-
return __math::__hypot(
128-
static_cast<__result_type>(__x), static_cast<__result_type>(__y), static_cast<__result_type>(__z));
129-
}
130-
#endif
131-
13244
} // namespace __math
13345

13446
_LIBCPP_END_NAMESPACE_STD
135-
_LIBCPP_POP_MACROS
13647

13748
#endif // _LIBCPP___MATH_HYPOT_H

include/cmath

+24-1
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ constexpr long double lerp(long double a, long double b, long double t) noexcept
313313
*/
314314

315315
#include <__config>
316-
#include <__math/hypot.h>
317316
#include <__type_traits/enable_if.h>
318317
#include <__type_traits/is_arithmetic.h>
319318
#include <__type_traits/is_constant_evaluated.h>
@@ -554,6 +553,30 @@ using ::scalbnl _LIBCPP_USING_IF_EXISTS;
554553
using ::tgammal _LIBCPP_USING_IF_EXISTS;
555554
using ::truncl _LIBCPP_USING_IF_EXISTS;
556555

556+
#if _LIBCPP_STD_VER >= 17
557+
inline _LIBCPP_HIDE_FROM_ABI float hypot(float __x, float __y, float __z) {
558+
return sqrt(__x * __x + __y * __y + __z * __z);
559+
}
560+
inline _LIBCPP_HIDE_FROM_ABI double hypot(double __x, double __y, double __z) {
561+
return sqrt(__x * __x + __y * __y + __z * __z);
562+
}
563+
inline _LIBCPP_HIDE_FROM_ABI long double hypot(long double __x, long double __y, long double __z) {
564+
return sqrt(__x * __x + __y * __y + __z * __z);
565+
}
566+
567+
template <class _A1, class _A2, class _A3>
568+
inline _LIBCPP_HIDE_FROM_ABI
569+
typename enable_if_t< is_arithmetic<_A1>::value && is_arithmetic<_A2>::value && is_arithmetic<_A3>::value,
570+
__promote<_A1, _A2, _A3> >::type
571+
hypot(_A1 __lcpp_x, _A2 __lcpp_y, _A3 __lcpp_z) _NOEXCEPT {
572+
typedef typename __promote<_A1, _A2, _A3>::type __result_type;
573+
static_assert(
574+
!(is_same<_A1, __result_type>::value && is_same<_A2, __result_type>::value && is_same<_A3, __result_type>::value),
575+
"");
576+
return std::hypot((__result_type)__lcpp_x, (__result_type)__lcpp_y, (__result_type)__lcpp_z);
577+
}
578+
#endif
579+
557580
template <class _A1, __enable_if_t<is_floating_point<_A1>::value, int> = 0>
558581
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool __constexpr_isnan(_A1 __lcpp_x) _NOEXCEPT {
559582
#if __has_builtin(__builtin_isnan)

test/libcxx/transitive_includes/cxx17.csv

-3
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,6 @@ chrono type_traits
130130
chrono vector
131131
chrono version
132132
cinttypes cstdint
133-
cmath cstddef
134-
cmath cstdint
135-
cmath initializer_list
136133
cmath limits
137134
cmath type_traits
138135
cmath version

test/libcxx/transitive_includes/cxx20.csv

-3
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,6 @@ chrono type_traits
135135
chrono vector
136136
chrono version
137137
cinttypes cstdint
138-
cmath cstddef
139-
cmath cstdint
140-
cmath initializer_list
141138
cmath limits
142139
cmath type_traits
143140
cmath version

test/libcxx/transitive_includes/cxx23.csv

-3
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,6 @@ chrono string_view
8383
chrono vector
8484
chrono version
8585
cinttypes cstdint
86-
cmath cstddef
87-
cmath cstdint
88-
cmath initializer_list
8986
cmath limits
9087
cmath version
9188
codecvt cctype

test/libcxx/transitive_includes/cxx26.csv

-3
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,6 @@ chrono string_view
8383
chrono vector
8484
chrono version
8585
cinttypes cstdint
86-
cmath cstddef
87-
cmath cstdint
88-
cmath initializer_list
8986
cmath limits
9087
cmath version
9188
codecvt cctype

test/std/numerics/c.math/cmath.pass.cpp

+16-75
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,14 @@
1212

1313
// <cmath>
1414

15-
#include <array>
1615
#include <cmath>
1716
#include <limits>
1817
#include <type_traits>
1918
#include <cassert>
2019

21-
#include "fp_compare.h"
2220
#include "test_macros.h"
2321
#include "hexfloat.h"
2422
#include "truncate_fp.h"
25-
#include "type_algorithms.h"
2623

2724
// convertible to int/float/double/etc
2825
template <class T, int N=0>
@@ -1116,56 +1113,6 @@ void test_fmin()
11161113
assert(std::fmin(1,0) == 0);
11171114
}
11181115

1119-
#if TEST_STD_VER >= 17
1120-
struct TestHypot3 {
1121-
template <class Real>
1122-
void operator()() const {
1123-
const auto check = [](Real elem, Real abs_tol) {
1124-
assert(std::isfinite(std::hypot(elem, Real(0), Real(0))));
1125-
assert(fptest_close(std::hypot(elem, Real(0), Real(0)), elem, abs_tol));
1126-
assert(std::isfinite(std::hypot(elem, elem, Real(0))));
1127-
assert(fptest_close(std::hypot(elem, elem, Real(0)), std::sqrt(Real(2)) * elem, abs_tol));
1128-
assert(std::isfinite(std::hypot(elem, elem, elem)));
1129-
assert(fptest_close(std::hypot(elem, elem, elem), std::sqrt(Real(3)) * elem, abs_tol));
1130-
};
1131-
1132-
{ // check for overflow
1133-
const auto [elem, abs_tol] = []() -> std::array<Real, 2> {
1134-
if constexpr (std::is_same_v<Real, float>)
1135-
return {1e20f, 1e16f};
1136-
else if constexpr (std::is_same_v<Real, double>)
1137-
return {1e300, 1e287};
1138-
else { // long double
1139-
# if __DBL_MAX_EXP__ == __LDBL_MAX_EXP__
1140-
return {1e300l, 1e287l}; // 64-bit
1141-
# else
1142-
return {1e4000l, 1e3985l}; // 80- or 128-bit
1143-
# endif
1144-
}
1145-
}();
1146-
check(elem, abs_tol);
1147-
}
1148-
1149-
{ // check for underflow
1150-
const auto [elem, abs_tol] = []() -> std::array<Real, 2> {
1151-
if constexpr (std::is_same_v<Real, float>)
1152-
return {1e-20f, 1e-24f};
1153-
else if constexpr (std::is_same_v<Real, double>)
1154-
return {1e-287, 1e-300};
1155-
else { // long double
1156-
# if __DBL_MAX_EXP__ == __LDBL_MAX_EXP__
1157-
return {1e-287l, 1e-300l}; // 64-bit
1158-
# else
1159-
return {1e-3985l, 1e-4000l}; // 80- or 128-bit
1160-
# endif
1161-
}
1162-
}();
1163-
check(elem, abs_tol);
1164-
}
1165-
}
1166-
};
1167-
#endif
1168-
11691116
void test_hypot()
11701117
{
11711118
static_assert((std::is_same<decltype(std::hypot((float)0, (float)0)), float>::value), "");
@@ -1188,31 +1135,25 @@ void test_hypot()
11881135
static_assert((std::is_same<decltype(hypot(Ambiguous(), Ambiguous())), Ambiguous>::value), "");
11891136
assert(std::hypot(3,4) == 5);
11901137

1191-
#if TEST_STD_VER >= 17
1192-
// clang-format off
1193-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (float)0, (float)0)), float>));
1194-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (bool)0, (float)0)), double>));
1195-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (unsigned short)0, (double)0)), double>));
1196-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (int)0, (long double)0)), long double>));
1197-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (double)0, (long)0)), double>));
1198-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (long double)0, (unsigned long)0)), long double>));
1199-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (int)0, (long long)0)), double>));
1200-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (int)0, (unsigned long long)0)), double>));
1201-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (double)0, (double)0)), double>));
1202-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (long double)0, (long double)0)), long double>));
1203-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (float)0, (double)0)), double>));
1204-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (float)0, (long double)0)), long double>));
1205-
static_assert((std::is_same_v<decltype(std::hypot((float)0, (double)0, (long double)0)), long double>));
1206-
static_assert((std::is_same_v<decltype(std::hypot((int)0, (int)0, (int)0)), double>));
1207-
static_assert((std::is_same_v<decltype(hypot(Ambiguous(), Ambiguous(), Ambiguous())), Ambiguous>));
1208-
// clang-format on
1138+
#if TEST_STD_VER > 14
1139+
static_assert((std::is_same<decltype(std::hypot((float)0, (float)0, (float)0)), float>::value), "");
1140+
static_assert((std::is_same<decltype(std::hypot((float)0, (bool)0, (float)0)), double>::value), "");
1141+
static_assert((std::is_same<decltype(std::hypot((float)0, (unsigned short)0, (double)0)), double>::value), "");
1142+
static_assert((std::is_same<decltype(std::hypot((float)0, (int)0, (long double)0)), long double>::value), "");
1143+
static_assert((std::is_same<decltype(std::hypot((float)0, (double)0, (long)0)), double>::value), "");
1144+
static_assert((std::is_same<decltype(std::hypot((float)0, (long double)0, (unsigned long)0)), long double>::value), "");
1145+
static_assert((std::is_same<decltype(std::hypot((float)0, (int)0, (long long)0)), double>::value), "");
1146+
static_assert((std::is_same<decltype(std::hypot((float)0, (int)0, (unsigned long long)0)), double>::value), "");
1147+
static_assert((std::is_same<decltype(std::hypot((float)0, (double)0, (double)0)), double>::value), "");
1148+
static_assert((std::is_same<decltype(std::hypot((float)0, (long double)0, (long double)0)), long double>::value), "");
1149+
static_assert((std::is_same<decltype(std::hypot((float)0, (float)0, (double)0)), double>::value), "");
1150+
static_assert((std::is_same<decltype(std::hypot((float)0, (float)0, (long double)0)), long double>::value), "");
1151+
static_assert((std::is_same<decltype(std::hypot((float)0, (double)0, (long double)0)), long double>::value), "");
1152+
static_assert((std::is_same<decltype(std::hypot((int)0, (int)0, (int)0)), double>::value), "");
1153+
static_assert((std::is_same<decltype(hypot(Ambiguous(), Ambiguous(), Ambiguous())), Ambiguous>::value), "");
12091154

12101155
assert(std::hypot(2,3,6) == 7);
12111156
assert(std::hypot(1,4,8) == 9);
1212-
1213-
// Check for undue over-/underflows of intermediate results.
1214-
// See discussion at https://github.com/llvm/llvm-project/issues/92782.
1215-
types::for_each(types::floating_point_types(), TestHypot3());
12161157
#endif
12171158
}
12181159

test/support/fp_compare.h

+25-20
Original file line numberDiff line numberDiff line change
@@ -9,34 +9,39 @@
99
#ifndef SUPPORT_FP_COMPARE_H
1010
#define SUPPORT_FP_COMPARE_H
1111

12-
#include <cmath> // for std::abs
13-
#include <algorithm> // for std::max
12+
#include <cmath> // for std::abs
13+
#include <algorithm> // for std::max
1414
#include <cassert>
15-
#include <__config>
1615

1716
// See https://www.boost.org/doc/libs/1_70_0/libs/test/doc/html/boost_test/testing_tools/extended_comparison/floating_point/floating_points_comparison_theory.html
1817

19-
template <typename T>
20-
bool fptest_close(T val, T expected, T eps) {
21-
_LIBCPP_CONSTEXPR T zero = T(0);
22-
assert(eps >= zero);
18+
template<typename T>
19+
bool fptest_close(T val, T expected, T eps)
20+
{
21+
constexpr T zero = T(0);
22+
assert(eps >= zero);
2323

24-
// Handle the zero cases
25-
if (eps == zero)
26-
return val == expected;
27-
if (val == zero)
28-
return std::abs(expected) <= eps;
29-
if (expected == zero)
30-
return std::abs(val) <= eps;
24+
// Handle the zero cases
25+
if (eps == zero) return val == expected;
26+
if (val == zero) return std::abs(expected) <= eps;
27+
if (expected == zero) return std::abs(val) <= eps;
3128

32-
return std::abs(val - expected) < eps && std::abs(val - expected) / std::abs(val) < eps;
29+
return std::abs(val - expected) < eps
30+
&& std::abs(val - expected)/std::abs(val) < eps;
3331
}
3432

35-
template <typename T>
36-
bool fptest_close_pct(T val, T expected, T percent) {
37-
assert(percent >= T(0));
38-
T eps = (percent / T(100)) * std::max(std::abs(val), std::abs(expected));
39-
return fptest_close(val, expected, eps);
33+
template<typename T>
34+
bool fptest_close_pct(T val, T expected, T percent)
35+
{
36+
constexpr T zero = T(0);
37+
assert(percent >= zero);
38+
39+
// Handle the zero cases
40+
if (percent == zero) return val == expected;
41+
T eps = (percent / T(100)) * std::max(std::abs(val), std::abs(expected));
42+
43+
return fptest_close(val, expected, eps);
4044
}
4145

46+
4247
#endif // SUPPORT_FP_COMPARE_H

0 commit comments

Comments
 (0)