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

[Tue Ly via Phabricator via libc-commits]

lntue marked 3 inline comments as done.
lntue 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>
----------------
sivachandra wrote:
> 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.
I updated the comments to reflect the expectations.


================
Comment at: libc/utils/MPFRWrapper/MPFRUtils.cpp:230
+  __llvm_libc::fputil::testing::describeValue(
+      "   MPFR rounded: ", toFloatingPoint<T>(mpfrResult), OS);
+  OS << '\n';
----------------
lntue wrote:
> sivachandra wrote:
> > In what circumstances would a rounded MPFR result help understand a test failure?
> 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.
I updated the test to reflect the case when there is a tie in rounded, i.e. ULP error == 0.5, we expect it to be rounded to even


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84725