[libc-commits] [PATCH] D91591: [libc] Add implementations of ldexp[f|l].

[Tue Ly via Phabricator via libc-commits]

lntue added inline comments.


================
Comment at: libc/test/src/math/LdExpTest.h:116
+  using LdExpTest = LdExpTestTemplate<T>;                                      \
+  TEST_F(LdExpTest, SpecialNumbers) { testSpecialNumbers(&func); }             \
+  TEST_F(LdExpTest, PowersOfTwo) { testPowersOfTwo(&func); }                   \
----------------
Isn't it going to show (LdExpTest, SpecialNumbers) for all float, double, and long double tests?
Maybe append ##T to LdExpTest to distinguish between different data types?


================
Comment at: libc/utils/FPUtil/NormalFloat.h:122
+          if (result.mantissa & 0x1)
+            result.mantissa += 1;
+        }
----------------
Can you test the case (S, E, M) = (0, 0x1, 0b11....1) and exp = -1?  The output should be (0, 0x1, 0b00..0).


================
Comment at: libc/utils/FPUtil/NormalFloat.h:233
+        if (result.mantissa & 0x1)
+          result.mantissa += 1;
+      }
----------------
Can you test with similar example as the above comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91591