[libc-commits] [PATCH] D76825: [libc] Move implementations of cosf, sinf, sincosf to src/math directory.

[Alex Brachet via Phabricator via libc-commits]

abrachet added inline comments.


================
Comment at: libc/src/math/CMakeLists.txt:9
+  DEPENDS
+    __errno_location
+)
----------------
Will this properly propagate to targets which depend on math_utils?


================
Comment at: libc/src/math/cosf.cpp:19-21
+// Fast cosf implementation.  Worst-case ULP is 0.5607, maximum relative
+// error is 0.5303 * 2^-23.  A single-step range reduction is used for
+// small values.  Large inputs have their range reduced using fast integer
----------------
Maybe we could remove the two spaces after the period. This isn't common in the rest of our comments


================
Comment at: libc/src/math/math_utils.h:1
+#ifndef LLVM_LIBC_SRC_MATH_MATH_UTILS_H
+#define LLVM_LIBC_SRC_MATH_MATH_UTILS_H
----------------
Should this file have a comment header?


================
Comment at: libc/src/math/sincosf.h:1
+//===------------------- Implementation header for sinf ------------------===//
+//
----------------
sinf -> sincosf


================
Comment at: libc/src/math/sincosf_utils.h:33-36
+extern const sincos_t __sincosf_table[2];
+
+// Table with 4/PI to 192 bit precision.
+extern const uint32_t __inv_pio4[];
----------------
We can put the data from sincosf_data and just make them constexpr perhaps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76825