[libcxx-commits] [PATCH] D70631: Microsoft's floating-point to_chars powered by Ryu and Ryu Printf

Mon Nov 25 00:24:04 PST 2019

ulfjack added inline comments.

================
Comment at: libcxx/CREDITS.TXT:16
+N: Ulf Adams
+D: Invented the Ryu and Ryu Printf algorithms used in floating-point to_chars.
+
----------------
Some of my comments are still alive, including some that don't make sense in this context. Maybe mention that I wrote the initial code?

================
Comment at: libcxx/include/xcharconv_ryu.h:973
+
+// This table is generated by PrintFloatLookupTable.
+inline constexpr int __FLOAT_POW5_INV_BITCOUNT = 59;
----------------
This should probably be adjusted?

================
Comment at: libcxx/include/xcharconv_ryu.h:975
+inline constexpr int __FLOAT_POW5_INV_BITCOUNT = 59;
+inline constexpr uint64_t __FLOAT_POW5_INV_SPLIT[31] = {
+  576460752303423489u, 461168601842738791u, 368934881474191033u, 295147905179352826u,
----------------
I have changed upstream to (optionally) make the 32-bit float code path to use the 64-bit double tables. Since everything is in one file here, that would allow you to remove these.

================
Comment at: libcxx/include/xcharconv_ryu.h:1025
+  _LIBCPP_ASSERT(__p < 32, "");
+  // return __builtin_ctz(__value) >= __p;
+  return (__value & ((1u << __p) - 1)) == 0;
----------------
Maybe remove this line? We can probably also remove it upstream or replace it with a comment that using builtin_ctz isn't faster.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70631