[libc-commits] [PATCH] D81134: [libc] Add implementation of few floating point manipulation functions.

[Anthony Steinhauser via Phabricator via libc-commits]

asteinhauser accepted this revision.
asteinhauser added a comment.
This revision is now accepted and ready to land.

Nice work. I would just make sure that the tests cover all control-flow paths.



================
Comment at: libc/test/src/math/frexp_test.cpp:100
+  EXPECT_EQ(exponent, 7);
+}
+
----------------
I would add one test with a different base than 0.5/-0.5 (e.g. 0.75, input value 24). You can also reduce the amount of 0.5 tests if you want to. The same with the float version.


================
Comment at: libc/test/src/math/logb_test.cpp:67
+  EXPECT_EQ(valueAsBits(5.0), valueAsBits(__llvm_libc::logb(-32.0)));
+}
+
----------------
I would add one more test of a value that is not 2^(integer).


================
Comment at: libc/test/src/math/modf_test.cpp:85
+  EXPECT_EQ(valueAsBits(integral), valueAsBits(-12345.0));
+}
+
----------------
Wouldn't it be useful to have also one test where the fractional part is non-zero (e.g. 0.5)? The same also for modff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81134