Add ULP function to MPFRNumber class to test correctly rounded functions.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
[libc] Add ULP function to MPFRNumber class to test correctly rounded functions such as SQRT, FMA.
The functionality LGTM. May be a few more comments would have helped me read this faster, but never mind for now. I am want to spend a little more time the layering of FPUtils, MPFRWrapper and UnitTest. Will come back soon with more comments.
We currently have UnitTest depend on FPUtil. I think that an creates unnecessary dependency for tests which do not need FPUtil for testing. So, a plan could be:
Instead of EXPECT_EQ and friends for floating point numbers, we will decouple UnitTest from FPUtil using matchers implemented in FPUtil. Say, EXPECT_FP_MATCH etc.
WDYT? If you concur, then you need not do the work. I will do it, and you can absorb those changes into this patch.
libc/utils/MPFRWrapper/MPFRUtils.cpp | ||
---|---|---|
176 | Few comments about documentation.
| |
219 | Instead of non-member functions, can we replace the existing methods with a template method: template <typename T> T as() const; We will still need specializations like below though. | |
235 | In what circumstances would a rounded MPFR result help understand a test failure? | |
238 | Is plural errors correct here? |
libc/utils/MPFRWrapper/MPFRUtils.cpp | ||
---|---|---|
235 | When the ULP error is exactly 0.5, the ULP value itself is not enough to check if the rounding is correct. This should never happen with SQRT function, but other one like HYPOT or FMA might be able to hit exactly 0.5 ULP. |
Correct the eps when the input is a power of 2, and add a check for correct rounding to even when it is a tie.
libc/utils/MPFRWrapper/MPFRUtils.cpp | ||
---|---|---|
180 | May be reword it as: We expect that this value and the value to be compared (the |input| argument) are reasonably close ... |
clang-tidy: warning: invalid case style for parameter 'op' [readability-identifier-naming]
not useful
clang-tidy: warning: invalid case style for parameter 'input' [readability-identifier-naming]
not useful
clang-tidy: warning: invalid case style for parameter 'libcOutput' [readability-identifier-naming]
not useful
clang-tidy: warning: invalid case style for parameter 't' [readability-identifier-naming]
not useful