[libc-commits] [PATCH] D127046: [libc][math] fmod/fmodf implementation.

[Kirill Okhotnikov via Phabricator via libc-commits]

orex marked 2 inline comments as done.
orex added inline comments.


================
Comment at: libc/src/__support/FPUtil/generic/FMod.h:16
+
+#include <algorithm>
+#include <cerrno>
----------------
orex wrote:
> lntue wrote:
> > I'm not sure that we can include that many C++ standard headers in here,  as it might introduce circular dependency among the libraries.  @sivachandra should have known more about these than me.
> All std includes were removed except cerrno. 
cerrno excluded.


================
Comment at: libc/src/__support/FPUtil/generic/FMod.h:82
+  static std::optional<T> PreCheck(T x, T y) {
+    if (unlikely(y == 0 || std::isnan(y) || !std::isfinite(x))) {
+      force_eval<T>((x * y) / (y * x));
----------------
lntue wrote:
> I don't think you would want to use `std::isnan` or `std::isfinite` in here.  As you can imagine, `FPUtil` functions would be the ones to provide the *backbone* of `libc`, and hence `std::` functions, so if FPUtil functions depending on `std::` math functions, it would likely create circular dependency when building or linking.  So it's best to reuse or reimplement those simple math functions in the `FPUtil / FPBits` themselves.
Implemented using builtin functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127046