[libc-commits] [PATCH] D84725: [libc] Add ULP function to MPFRNumber class to test correctly rounded functions such as SQRT, FMA.

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Thu Aug 13 00:06:57 PDT 2020

sivachandra added inline comments.

Comment at: libc/utils/MPFRWrapper/MPFRUtils.cpp:172
+  // ULP(mpfr_value, value) = abs(mpfr_value - value) / eps(value)
+  // ULP < 0.5 will imply that the value is correctly rounded.
+  template <typename T>
Few comments about documentation.

1. I think this function assumes that the exponent of `input` matches that of the MPFR number. If yes, document it here?
2. Explain why a `double` return value is good enough?
3. Lastly, is the name `ulp` of this method correct? I guess, because you defined ULP above as a ratio, the name `ulp` is correct and is OK as is.

Comment at: libc/utils/MPFRWrapper/MPFRUtils.cpp:200
+template <typename T> T toFloatingPoint(MPFRNumber mpfrInput);
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.

Comment at: libc/utils/MPFRWrapper/MPFRUtils.cpp:230
+  __llvm_libc::fputil::testing::describeValue(
+      "   MPFR rounded: ", toFloatingPoint<T>(mpfrResult), OS);
+  OS << '\n';
In what circumstances would a rounded MPFR result help understand a test failure?

Comment at: libc/utils/MPFRWrapper/MPFRUtils.cpp:233
+  if (useULP) {
+    OS << "     ULP errors: " << std::to_string(mpfrResult.ulp(matchValue))
+       << '\n';
Is plural `errors` correct here?

  rG LLVM Github Monorepo



More information about the libc-commits mailing list