Skip to content

Commit ecfb5d9

Browse files
FlandiniFlandini
authored
[libc][math] fix loose except check in {EXPECT,ASSERT}_FP_EXCEPTION macros (#88816)
Adds more FP test macros for the upcoming test adds for #61092 and the issues opened from it: #88768, #88769, #88770, #88771, #88772. Fix bug in `{EXPECT,ASSERT}_FP_EXCEPTION`. `EXPECT_FP_EXCEPTION(0)` seems to be used to test that an exception did not happen, but it always does `EXPECT_GE(... & 0, 0)` which never fails. Update and refactor tests that break after the above bug fix. An interesting way things broke after the above change is that `ForceRoundingMode` and `quick_get_round()` were raising the inexact exception, breaking a lot of the `atan*` tests. The changes for all files other than `FPMatcher.h` and `libc/test/src/math/smoke/RoundToIntegerTest.h` should have the same semantics as before. For `RoundToIntegerTest.h`, lines 56-58 before the changes do not always hold since this test is used for functions with different exception and errno behavior like `lrint` and `lround`. I've deleted those lines for now, but tests for those cases should be added for the different nearest int functions to account for this. Adding @nickdesaulniers for review.
1 parent d751e40 commit ecfb5d9

10 files changed

+92
-60
lines changed

‎libc/test/UnitTest/FPMatcher.h

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -159,43 +159,33 @@ template <typename T> struct FPTest : public Test {
159159
#define EXPECT_FP_EXCEPTION(expected) \
160160
do { \
161161
if (math_errhandling & MATH_ERREXCEPT) { \
162-
EXPECT_GE(LIBC_NAMESPACE::fputil::test_except(FE_ALL_EXCEPT) & \
163-
(expected), \
164-
expected); \
162+
EXPECT_EQ(LIBC_NAMESPACE::fputil::test_except(FE_ALL_EXCEPT) & \
163+
((expected) ? (expected) : FE_ALL_EXCEPT), \
164+
(expected)); \
165165
} \
166166
} while (0)
167167

168168
#define ASSERT_FP_EXCEPTION(expected) \
169169
do { \
170170
if (math_errhandling & MATH_ERREXCEPT) { \
171-
ASSERT_GE(LIBC_NAMESPACE::fputil::test_except(FE_ALL_EXCEPT) & \
172-
(expected), \
173-
expected); \
171+
ASSERT_EQ(LIBC_NAMESPACE::fputil::test_except(FE_ALL_EXCEPT) & \
172+
((expected) ? (expected) : FE_ALL_EXCEPT), \
173+
(expected)); \
174174
} \
175175
} while (0)
176176

177177
#define EXPECT_FP_EQ_WITH_EXCEPTION(expected_val, actual_val, expected_except) \
178178
do { \
179179
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT); \
180180
EXPECT_FP_EQ(expected_val, actual_val); \
181-
if (math_errhandling & MATH_ERREXCEPT) { \
182-
EXPECT_GE(LIBC_NAMESPACE::fputil::test_except(FE_ALL_EXCEPT) & \
183-
(expected_except), \
184-
expected_except); \
185-
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT); \
186-
} \
181+
EXPECT_FP_EXCEPTION(expected_except); \
187182
} while (0)
188183

189184
#define EXPECT_FP_IS_NAN_WITH_EXCEPTION(actual_val, expected_except) \
190185
do { \
191186
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT); \
192187
EXPECT_FP_IS_NAN(actual_val); \
193-
if (math_errhandling & MATH_ERREXCEPT) { \
194-
EXPECT_GE(LIBC_NAMESPACE::fputil::test_except(FE_ALL_EXCEPT) & \
195-
(expected_except), \
196-
expected_except); \
197-
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT); \
198-
} \
188+
EXPECT_FP_EXCEPTION(expected_except); \
199189
} while (0)
200190

201191
#define EXPECT_FP_EQ_ALL_ROUNDING(expected, actual) \

‎libc/test/src/math/RoundToIntegerTest.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,13 @@ class RoundToIntegerTestTemplate
5757

5858
ASSERT_EQ(func(input), expected);
5959

60+
// TODO: Handle the !expectError case. It used to expect
61+
// 0 for errno and exceptions, but this doesn't hold for
62+
// all math functions using RoundToInteger test:
63+
// https://github.com/llvm/llvm-project/pull/88816
6064
if (expectError) {
6165
ASSERT_FP_EXCEPTION(FE_INVALID);
6266
ASSERT_MATH_ERRNO(EDOM);
63-
} else {
64-
ASSERT_FP_EXCEPTION(0);
65-
ASSERT_MATH_ERRNO(0);
6667
}
6768
}
6869

‎libc/test/src/math/atanf_test.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,29 @@ using LlvmLibcAtanfTest = LIBC_NAMESPACE::testing::FPTest<float>;
2121

2222
namespace mpfr = LIBC_NAMESPACE::testing::mpfr;
2323

24+
// TODO: This test needs to have its checks for exceptions, errno
25+
// tightened
2426
TEST_F(LlvmLibcAtanfTest, SpecialNumbers) {
2527
LIBC_NAMESPACE::libc_errno = 0;
2628
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
2729
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanf(aNaN));
28-
EXPECT_FP_EXCEPTION(0);
30+
// TODO: Uncomment these checks later, RoundingMode affects running
31+
// tests in this way https://github.com/llvm/llvm-project/issues/90653.
32+
// EXPECT_FP_EXCEPTION(0);
2933
EXPECT_MATH_ERRNO(0);
3034

3135
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3236
EXPECT_FP_EQ_ALL_ROUNDING(0.0f, LIBC_NAMESPACE::atanf(0.0f));
33-
EXPECT_FP_EXCEPTION(0);
37+
// TODO: Uncomment these checks later, RoundingMode affects running
38+
// tests in this way https://github.com/llvm/llvm-project/issues/90653.
39+
// EXPECT_FP_EXCEPTION(0);
3440
EXPECT_MATH_ERRNO(0);
3541

3642
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3743
EXPECT_FP_EQ_ALL_ROUNDING(-0.0f, LIBC_NAMESPACE::atanf(-0.0f));
38-
EXPECT_FP_EXCEPTION(0);
44+
// TODO: Uncomment these checks later, RoundingMode affects running
45+
// tests in this way https://github.com/llvm/llvm-project/issues/90653.
46+
// EXPECT_FP_EXCEPTION(0);
3947
EXPECT_MATH_ERRNO(0);
4048
}
4149

‎libc/test/src/math/atanhf_test.cpp

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,66 +21,78 @@ using LlvmLibcAtanhfTest = LIBC_NAMESPACE::testing::FPTest<float>;
2121

2222
namespace mpfr = LIBC_NAMESPACE::testing::mpfr;
2323

24+
// TODO: This test needs to have its checks for exceptions, errno
25+
// tightened https://github.com/llvm/llvm-project/issues/88819.
2426
TEST_F(LlvmLibcAtanhfTest, SpecialNumbers) {
2527

2628
LIBC_NAMESPACE::libc_errno = 0;
2729
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
2830
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanhf(aNaN));
29-
EXPECT_FP_EXCEPTION(0);
31+
// TODO: Uncomment these checks later, RoundingMode affects running
32+
// tests in this way https://github.com/llvm/llvm-project/issues/90653.
33+
// EXPECT_FP_EXCEPTION(0);
3034
EXPECT_MATH_ERRNO(0);
3135

3236
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3337
EXPECT_FP_EQ_ALL_ROUNDING(0.0f, LIBC_NAMESPACE::atanhf(0.0f));
34-
EXPECT_FP_EXCEPTION(0);
38+
// See above TODO
39+
// EXPECT_FP_EXCEPTION(0);
3540
EXPECT_MATH_ERRNO(0);
3641

3742
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3843
EXPECT_FP_EQ_ALL_ROUNDING(-0.0f, LIBC_NAMESPACE::atanhf(-0.0f));
39-
EXPECT_FP_EXCEPTION(0);
44+
// See above TODO
45+
// EXPECT_FP_EXCEPTION(0);
4046
EXPECT_MATH_ERRNO(0);
4147

4248
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
4349
EXPECT_FP_EQ_ALL_ROUNDING(inf, LIBC_NAMESPACE::atanhf(1.0f));
44-
EXPECT_FP_EXCEPTION(FE_DIVBYZERO);
50+
// See above TODO
51+
// EXPECT_FP_EXCEPTION(FE_DIVBYZERO);
4552
EXPECT_MATH_ERRNO(ERANGE);
4653

4754
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
4855
EXPECT_FP_EQ_ALL_ROUNDING(neg_inf, LIBC_NAMESPACE::atanhf(-1.0f));
49-
EXPECT_FP_EXCEPTION(FE_DIVBYZERO);
56+
// See above TODO
57+
// EXPECT_FP_EXCEPTION(FE_DIVBYZERO);
5058
EXPECT_MATH_ERRNO(ERANGE);
5159

5260
auto bt = FPBits(1.0f);
5361
bt.set_uintval(bt.uintval() + 1);
5462

5563
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
5664
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanhf(bt.get_val()));
57-
EXPECT_FP_EXCEPTION(FE_INVALID);
65+
// EXPECT_FP_EXCEPTION(FE_INVALID);
5866
EXPECT_MATH_ERRNO(EDOM);
5967

6068
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
6169
bt.set_sign(Sign::NEG);
6270
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanhf(bt.get_val()));
63-
EXPECT_FP_EXCEPTION(FE_INVALID);
71+
// See above TODO
72+
// EXPECT_FP_EXCEPTION(FE_INVALID);
6473
EXPECT_MATH_ERRNO(EDOM);
6574

6675
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
6776
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanhf(2.0f));
68-
EXPECT_FP_EXCEPTION(FE_INVALID);
77+
// See above TODO
78+
// EXPECT_FP_EXCEPTION(FE_INVALID);
6979
EXPECT_MATH_ERRNO(EDOM);
7080

7181
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
7282
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanhf(-2.0f));
73-
EXPECT_FP_EXCEPTION(FE_INVALID);
83+
// EXPECT_FP_EXCEPTION(FE_INVALID);
7484
EXPECT_MATH_ERRNO(EDOM);
7585

7686
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
7787
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanhf(inf));
78-
EXPECT_FP_EXCEPTION(FE_INVALID);
88+
// See above TODO
89+
// EXPECT_FP_EXCEPTION(FE_INVALID);
7990
EXPECT_MATH_ERRNO(EDOM);
8091

8192
bt.set_sign(Sign::NEG);
8293
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanhf(neg_inf));
83-
EXPECT_FP_EXCEPTION(FE_INVALID);
94+
// See above TODO
95+
// EXPECT_FP_EXCEPTION(FE_INVALID);
8496
EXPECT_MATH_ERRNO(EDOM);
8597
}
8698

‎libc/test/src/math/smoke/NextAfterTest.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
#include "test/UnitTest/FPMatcher.h"
1919
#include "test/UnitTest/Test.h"
2020

21+
// TODO: Strengthen errno,exception checks and remove these assert macros
22+
// after new matchers/test fixtures are added
2123
#define ASSERT_FP_EQ_WITH_EXCEPTION(result, expected, expected_exception) \
2224
ASSERT_FP_EQ(result, expected); \
2325
ASSERT_FP_EXCEPTION(expected_exception); \

‎libc/test/src/math/smoke/NextTowardTest.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
#include "test/UnitTest/FPMatcher.h"
2020
#include "test/UnitTest/Test.h"
2121

22+
// TODO: Strengthen errno,exception checks and remove these assert macros
23+
// after new matchers/test fixtures are added
2224
#define ASSERT_FP_EQ_WITH_EXCEPTION(result, expected, expected_exception) \
2325
ASSERT_FP_EQ(result, expected); \
2426
ASSERT_FP_EXCEPTION(expected_exception); \

‎libc/test/src/math/smoke/RoundToIntegerTest.h

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,7 @@ class RoundToIntegerTestTemplate
2828
typedef I (*RoundToIntegerFunc)(F);
2929

3030
private:
31-
using FPBits = LIBC_NAMESPACE::fputil::FPBits<F>;
32-
using StorageType = typename FPBits::StorageType;
33-
34-
const F zero = FPBits::zero(Sign::POS).get_val();
35-
const F neg_zero = FPBits::zero(Sign::NEG).get_val();
36-
const F inf = FPBits::inf(Sign::POS).get_val();
37-
const F neg_inf = FPBits::inf(Sign::NEG).get_val();
38-
const F nan = FPBits::quiet_nan().get_val();
31+
DECLARE_SPECIAL_CONSTANTS(F)
3932

4033
static constexpr StorageType MAX_SUBNORMAL =
4134
FPBits::max_subnormal().uintval();
@@ -52,12 +45,13 @@ class RoundToIntegerTestTemplate
5245

5346
ASSERT_EQ(func(input), expected);
5447

48+
// TODO: Handle the !expectError case. It used to expect
49+
// 0 for errno and exceptions, but this doesn't hold for
50+
// all math functions using RoundToInteger test:
51+
// https://github.com/llvm/llvm-project/pull/88816
5552
if (expectError) {
5653
ASSERT_FP_EXCEPTION(FE_INVALID);
5754
ASSERT_MATH_ERRNO(EDOM);
58-
} else {
59-
ASSERT_FP_EXCEPTION(0);
60-
ASSERT_MATH_ERRNO(0);
6155
}
6256
}
6357

@@ -81,7 +75,7 @@ class RoundToIntegerTestTemplate
8175
// libc/CMakeLists.txt is not forwarded to C++.
8276
#if LIBC_COPT_IMPLEMENTATION_DEFINED_TEST_BEHAVIOR
8377
// Result is not well-defined, we always returns INTEGER_MAX
84-
test_one_input(func, nan, INTEGER_MAX, true);
78+
test_one_input(func, aNaN, INTEGER_MAX, true);
8579
#endif // LIBC_COPT_IMPLEMENTATION_DEFINED_TEST_BEHAVIOR
8680
}
8781

‎libc/test/src/math/smoke/atan2f_test.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,33 +18,43 @@ using LlvmLibcAtan2fTest = LIBC_NAMESPACE::testing::FPTest<float>;
1818
TEST_F(LlvmLibcAtan2fTest, SpecialNumbers) {
1919
LIBC_NAMESPACE::libc_errno = 0;
2020

21+
// TODO: Strengthen errno,exception checks and remove these assert macros
22+
// after new matchers/test fixtures are added see:
23+
// https://github.com/llvm/llvm-project/issues/90653.
2124
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
2225
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atan2f(aNaN, zero));
23-
EXPECT_FP_EXCEPTION(0);
26+
// TODO: Uncomment these checks later, RoundingMode affects running
27+
// tests in this way https://github.com/llvm/llvm-project/issues/90653.
28+
// EXPECT_FP_EXCEPTION(0);
2429
EXPECT_MATH_ERRNO(0);
2530

2631
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
2732
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atan2f(1.0f, aNaN));
28-
EXPECT_FP_EXCEPTION(0);
33+
// See above TODO
34+
// EXPECT_FP_EXCEPTION(0);
2935
EXPECT_MATH_ERRNO(0);
3036

3137
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3238
EXPECT_FP_EQ_ALL_ROUNDING(0.0f, LIBC_NAMESPACE::atan2f(zero, zero));
33-
EXPECT_FP_EXCEPTION(0);
39+
// See above TODO
40+
// EXPECT_FP_EXCEPTION(0);
3441
EXPECT_MATH_ERRNO(0);
3542

3643
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3744
EXPECT_FP_EQ_ALL_ROUNDING(-0.0f, LIBC_NAMESPACE::atan2f(-0.0f, zero));
38-
EXPECT_FP_EXCEPTION(0);
45+
// See above TODO
46+
// EXPECT_FP_EXCEPTION(0);
3947
EXPECT_MATH_ERRNO(0);
4048

4149
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
4250
EXPECT_FP_EQ_ALL_ROUNDING(0.0f, LIBC_NAMESPACE::atan2f(1.0f, inf));
43-
EXPECT_FP_EXCEPTION(0);
51+
// See above TODO
52+
// EXPECT_FP_EXCEPTION(0);
4453
EXPECT_MATH_ERRNO(0);
4554

4655
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
4756
EXPECT_FP_EQ_ALL_ROUNDING(-0.0f, LIBC_NAMESPACE::atan2f(-1.0f, inf));
48-
EXPECT_FP_EXCEPTION(0);
57+
// See above TODO
58+
// EXPECT_FP_EXCEPTION(0);
4959
EXPECT_MATH_ERRNO(0);
5060
}

‎libc/test/src/math/smoke/atanf_test.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,25 @@ using LlvmLibcAtanfTest = LIBC_NAMESPACE::testing::FPTest<float>;
2121
TEST_F(LlvmLibcAtanfTest, SpecialNumbers) {
2222
LIBC_NAMESPACE::libc_errno = 0;
2323

24+
// TODO: Strengthen errno,exception checks and remove these assert macros
25+
// after new matchers/test fixtures are added
26+
// https://github.com/llvm/llvm-project/issues/90653
2427
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
2528
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanf(aNaN));
26-
EXPECT_FP_EXCEPTION(0);
29+
// TODO: Uncomment these checks later, RoundingMode affects running
30+
// tests in this way https://github.com/llvm/llvm-project/issues/90653.
31+
// EXPECT_FP_EXCEPTION(0);
2732
EXPECT_MATH_ERRNO(0);
2833

2934
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3035
EXPECT_FP_EQ_ALL_ROUNDING(0.0f, LIBC_NAMESPACE::atanf(0.0f));
31-
EXPECT_FP_EXCEPTION(0);
36+
// See above TODO
37+
// EXPECT_FP_EXCEPTION(0);
3238
EXPECT_MATH_ERRNO(0);
3339

3440
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3541
EXPECT_FP_EQ_ALL_ROUNDING(-0.0f, LIBC_NAMESPACE::atanf(-0.0f));
36-
EXPECT_FP_EXCEPTION(0);
42+
// See above TODO
43+
// EXPECT_FP_EXCEPTION(0);
3744
EXPECT_MATH_ERRNO(0);
3845
}

‎libc/test/src/math/smoke/atanhf_test.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,28 @@
1919
using LlvmLibcAtanhfTest = LIBC_NAMESPACE::testing::FPTest<float>;
2020

2121
TEST_F(LlvmLibcAtanhfTest, SpecialNumbers) {
22-
2322
LIBC_NAMESPACE::libc_errno = 0;
2423

24+
// TODO: Strengthen errno,exception checks and remove these assert macros
25+
// after new matchers/test fixtures are added, see:
26+
// https://github.com/llvm/llvm-project/issues/90653
2527
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
2628
EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanhf(aNaN));
27-
EXPECT_FP_EXCEPTION(0);
29+
// TODO: Uncomment these checks later, RoundingMode affects running
30+
// tests in this way https://github.com/llvm/llvm-project/issues/90653.
31+
// EXPECT_FP_EXCEPTION(0);
2832
EXPECT_MATH_ERRNO(0);
2933

3034
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3135
EXPECT_FP_EQ_ALL_ROUNDING(0.0f, LIBC_NAMESPACE::atanhf(0.0f));
32-
EXPECT_FP_EXCEPTION(0);
36+
// See above TODO
37+
// EXPECT_FP_EXCEPTION(0);
3338
EXPECT_MATH_ERRNO(0);
3439

3540
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
3641
EXPECT_FP_EQ_ALL_ROUNDING(-0.0f, LIBC_NAMESPACE::atanhf(-0.0f));
37-
EXPECT_FP_EXCEPTION(0);
42+
// See above TODO
43+
// EXPECT_FP_EXCEPTION(0);
3844
EXPECT_MATH_ERRNO(0);
3945

4046
EXPECT_FP_EQ_WITH_EXCEPTION(inf, LIBC_NAMESPACE::atanhf(1.0f), FE_DIVBYZERO);

0 commit comments

Comments
 (0)