[libc-commits] [PATCH] D153682: [libc][math] Clean up exhaustive tests implementations.

[Tue Ly via Phabricator via libc-commits]

lntue added inline comments.


================
Comment at: libc/test/src/math/exhaustive/exhaustive_test.h:35
+
+template <typename Op>
 struct LlvmLibcExhaustiveTest : public __llvm_libc::testing::Test {
----------------
sivachandra wrote:
> Can this be more specific like:
> 
> ```
> template <typename FloatType>
> using FuncType = FloatType(FloatType);
> 
> template <typename FloatType, __llvm_libc::testing::mpfr::Operation Op, FuncType<FloatType> Func>
> struct LlvmLibcExhaustiveMathTest ... {
>   ...
>   static constexpr FuncType<FloatType> *FUNC = Func;
> 
>   uint64_t check(...) {
>     bool correct = TEST_MPFR_MATCH_ROUNDING_SILENTLY(Op, x, FUNC(x), 0.5, rounding);
>   }
>   ...
> }
> ```
> 
> For `erff`, you can do:
> 
> ```
> using LlvmLibcErffExhaustiveTest : publicLlvmLibcExhaustiveTest<float, mpfr::Operation::ErfOp, __llvm_libc::erff>;
> ```
I've separated them into `Checker` class to provide the `check` method, and the canonical `UnaryOpChecker::check` method.  Other examples of `Checker` are in `hypotf_test` and `sincosf_test`.

Then the `LlvmLibcUnaryOpExhaustiveMathTest` will take 3 template parameters as you suggested.


================
Comment at: libc/test/src/math/exhaustive/exhaustive_test.h:44
+  // Check in a range, return the number of failures.
+  virtual uint64_t check(UIntType start, UIntType stop,
+                         mpfr::RoundingMode rounding) {
----------------
sivachandra wrote:
> Why is this `virtual`?
Originally, it is virtual so that different tests such as `hypotf` or `sincosf` can override the `check` method.  I refactored the classes so that the `check` method does not need to be `virtual`.


================
Comment at: libc/test/src/math/exhaustive/exhaustive_test.h:55
+      bool correct = TEST_MPFR_MATCH_ROUNDING_SILENTLY(
+          Op::Operation, x, Op::func(x), 0.5, rounding);
+      failed += (!correct);
----------------
sivachandra wrote:
> Do we tolerate errors?
We do not tolerate errors, but providing the number of failed inputs is very useful when choosing the polynomial approximation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153682/new/

https://reviews.llvm.org/D153682