[libc-commits] [PATCH] D94018: [libc] Add implementation of fmaf.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/math/fmaf.cpp:22
+  fputil::FPBits<double> bit_sum(static_cast<double>(bit_prod) +
+                                 static_cast<double>(bitz));
+
----------------
Nit: Can we use `z` directly?


================
Comment at: libc/src/math/fmaf.cpp:24
+
+  if (!(bit_sum.isInfOrNaN() || bit_sum.isZero())) {
+    // Dekker's 2Sum: find t such that s - t = prod + z exactly
----------------
Can you add comments explaining why this correction is required?


================
Comment at: libc/utils/FPUtil/FPBits.h:89
+                                  (cpp::IsIntegral<XType>::Value &&
+                                   (sizeof(XType) == sizeof(UIntType))),
+                              int> = 0>
----------------
This will not be true for `long double` on i386. Will replacing it with an inequality be OK?

```
sizeof(XType) <= sizeof(UIntType)
```


================
Comment at: libc/utils/MPFRWrapper/MPFRUtils.cpp:39
+template <typename T> struct Precision {
+  static constexpr unsigned int value = 128;
+};
----------------
Is this used anywhere?


================
Comment at: libc/utils/MPFRWrapper/MPFRUtils.cpp:42
+
+template <> struct Precision<float> {
+  static constexpr unsigned int value = 24;
----------------
Does it make sense to put these values in FPBits?


================
Comment at: libc/utils/MPFRWrapper/MPFRUtils.cpp:365
+  constexpr unsigned int prec = Precision<InputType>::value;
+  MPFRNumber inputX(x, prec), inputY(y, prec), inputZ(z, prec);
+  switch (op) {
----------------
Can you add a comment explaining why we can't use a single large precision for all types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94018